search

mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
26.69k stars 2.83k forks source link

win32: quantize taskbar playback position into uint8 range #14283

Closed kasper93 closed 4 weeks ago

github-actions[bot] commented 1 month ago

Download the artifacts for this pull request:

Windows * [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1560922724.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1560924017.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1560929772.zip)
macOS * [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1560920172.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1560921909.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1560919533.zip)
kasper93 commented 1 month ago

Can you check if this also fixes https://github.com/mpv-player/mpv/issues/5932?

Fixed by https://github.com/mpv-player/mpv/commit/d061f28397937f0eae8fc7ba3b13913bbf29a9b7, currently mpv correctly rounds playback position, so it reaches 100%. Side note, I don't think get_percent_pos is really useful. 1% increment can be like 144s for 4h media file...

na-na-hi commented 1 month ago

Side note, I don't think get_percent_pos is really useful.

It was used when percent-pos was an integer property, but I agree this function isn't useful anymore. I have removed all remaining uses of this function in https://github.com/mpv-player/mpv/pull/14285.

kasper93 commented 4 weeks ago

I was thinking to use AVRational for this, but in fact the progress doesn't have to be perfectly smooth and Windows also divide it into small amount of steps. On the same note, I change to uint8_t, because it should be good enough for everything.

Let me know if you think we should do better here.

na-na-hi commented 4 weeks ago

I think uint8 range is good enough for this. Even with Windows 10 taskbar showing labels the total width is larger than 256 pixels only at 1.75x scale and above. Without labels it shouldn't be a problem at any meaningful scale.

kasper93 commented 4 weeks ago

Can you check if this also fixes #5932?

Fixed by d061f28, currently mpv correctly rounds playback position, so it reaches 100%. Side note, I don't think get_percent_pos is really useful. 1% increment can be like 144s for 4h media file...

In fact about this one it works the same way as OSC. We don't go to the end on EOF. For short files, we would get for example 0.99*255 = 252.45 ~= 252. Which is kinda expected. I don't think there is anything to do in the toolbar code itself. Reported position when we are at EOF could be adjusted to be at the end of the file instead of beginning of last frame, but it is only matter of preference at this point.

And the original issue was indeed resolved by rounding, because it was never reaching anything more than 99%, always leaving a gap, while now it can only happen when there is legitimate offset from the end, same as in OSC.

In short, everything is correct.