abdallahmehiz / mpvKt

A media player for android, based on mpv-android and built with Jetpack Compose.
Apache License 2.0
408 stars 12 forks source link

[BUG] Subtite delay utility adds/subtracts from delay value #69

Closed 77-77SP closed 1 week ago

77-77SP commented 1 week ago

The subtitle delay utility should not take the delay value into calculation since it is adding/subtracting to/from it and this is not the expected behavior. While it works fine when the subtitle delay vaule is 0, any vaule other than that will mess up the delay.

abdallahmehiz commented 1 week ago

Can you elaborate with an example?

77-77SP commented 1 week ago

Can you elaborate with an example?

Of course, Consider the following example:

A user has loaded a video and entered the subtitle delay interface in order to fix the delay. He used the delay utility and managed to fix the delay. The delay value has changed from 0 ms to lets say, 5000 ms. Later on, the subtitle delay issue popped up again so the user tries to apply a new vaule using the utility. Lets say the new delay is 10000 ms. Instead of the utility applying the new delay vaule only, it adds both delays as the new vaule. So he ends up with 15000 ms as the new vaule instead of 10000 ms.

77-77SP commented 1 week ago

I should mention that this behavior also occurs with the audio delay utility.

abdallahmehiz commented 1 week ago

That's not the case from my testing though... and you can test it by enabling Page 1 statistics and checking the A-V delay on the audio section. Same logic applies to subtitles delay too, The value you see on the text fields is the value being passed to mpv...

abdallahmehiz commented 1 week ago

Or are you talking about the new VLC-like utility buttons? Those do add the old value with new recorded delay but that's intentional.

77-77SP commented 1 week ago

Or are you talking about the new VLC-like utility buttons? Those do add the old value with new recorded delay but that's intentional.

Yes, that's what I am talking about. May I ask why it is intentional? In VLC it is not for example.

abdallahmehiz commented 1 week ago

In VLC it is not for example.

It is though, subsequent presses on the utility buttons increments/decrements the delay rather than reset it in there, same way it's in mpvKt.

Yes, that's what I am talking about. May I ask why it is intentional?

That's because it relies on visual feedback, if the subs are already delayed by 1 second, and the user wants to delay them to 2.5 seconds, we'd want use the 1 second delay as our reference point cause that's what the user is seeing. If we reset it to 0 we basically reset the reference point. I don't know if this gets the point across.

77-77SP commented 1 week ago

In VLC it is not for example.

It is though, subsequent presses on the utility buttons increments/decrements the delay rather than reset it in there, same way it's in mpvKt.

Yes, that's what I am talking about. May I ask why it is intentional?

That's because it relies on visual feedback, if the subs are already delayed by 1 second, and the user wants to delay them to 2.5 seconds, we'd want use the 1 second delay as our reference point cause that's what the user is seeing. If we reset it to 0 we basically reset the reference point. I don't know if this gets the point across.

You are absolutely correct. Sorry for the misunderstanding and the you for your time.