more playback speed changes #13614
more playback speed changes #13614
Conversation
|
Download the artifacts for this pull request: |
|
Can we just revert e3af545 for the time being and rethink this whole thing again? I'm not sure fixing such a minor issue is worth introducing hacks on top of hacks. @DanOscarsson said he had a better fix for it, so we could wait on that or at least wait for more eyes to look at these changes before merging them. edit: 3c37e18 specifically looks good to me though, I'm mostly talking about the av state reset change. |
|
Silently skipping around the playback position when changing speed is something I would consider pretty severe not minor. But then again since nobody ever noticed and this probably existed for the entire of mpv's history, so maybe it's a minor issue by that perspective. I don't claim to have the best or smartest solutions. But nobody else is interested in working on it so I do what I can even if it takes 10 iterations to get to a good state. Of course if Dan shows up and one ups me with the perfect solution, I will happily click merge. I'm not doubting him, but people have lives and supposedly said patches are pretty old so who knows how long it will take to get it ready. If he's ready by tomorrow or something then yeah sure, I'll close this. Commits 1 and 3 I'm fine with. 2 needs more work, but I would like to verify if it at least addresses the audio gap problem. |
Well I ended up doing this anyway in this patchset and completely rewrote it. |
Think I've mostly dealt with this now. Much happier with this.
Hopefully that would be PR now. Could you give it a try again? I think I've ironed everything out. |
|
I've tested this and it seems to be fine now, I can't reproduce the original issue and there are no obvious regressions. |
|
Made a small adjustment to deal with a fringe case I found. Hopefully, this should be better than the old code and definitely master, so I'll probably merge this later today if I or anyone else doesn't find anything else. I'll handle more issues if/when they inevitably come up. |
|
I got the b9ff661 build from the links above and tried it AV desync can still be quite high (~500ms), but it resyncs fairly quickly so it's much less noticeable in practice. The effect is fairly similar to using For higher speeds (above ~4x), I can manage to get an edge case where the A-V sync number gets "stuck", or in some cases keeps increasing indefinitely, by changing between 2, 4, 8x speeds randomly. It actually seems to trigger when you decrease the speed |
Dudemanguy
force-pushed
the
more-speed
branch
2 times, most recently
from
66c801e
to
9b28f75
Compare
It's a tradeoff. When you hit these super high speeds, you can't let the av adjustment go too much or else you'll skip frames around weirdly in a non-monotonic fashion. So that means higher desync on the immediate speed change. It's by no means perfect but for me the av desync message doesn't appear until around 10x speed which is well beyond the point of usuable audio anyways, so I figured it was not too important.
Thanks, I could hit this too. It's pretty easy to address and with the latest push it shouldn't runaway indefinitely anymore. Unfortunately in these weird edge cases, decreasing the speed from say like 16x to 8x will still result in frames skipping forward like before. It doesn't happen when resetting to a sane number around 1x though. |
With the latest build 2565bbd, this is further improved to the point of being subjectively unnoticable in all cases and only measurable by screen recording or other analysis. But the a/v desync still seems to adversely affect scripts that depend on firing at precise times; i.e. I seem to be receive some events ~85ms after they fire, which is enough to cause some issues. I'm trying to work around that by adjusting audio-buffer somehow.
This is also fixed on my end now. Hopefully you didn't have to put too many hacks in :P Since nobody else ever complained, maybe I'm just being too picky, or scripting too far out of scope of what mpv is designed to handle. But I think it's worth pursuing a more robust solution in the future. Shall I open an issue to track further progress on A/V sync + speed change bugs? |
What was the delay before in the old code (i.e. before e3af545)? I don't want to shoot you down but scripts are async/racy so depending on things happening at precise times is not easy.
Sure after this hopefully is to everyone's satisfaction we can drill down on some even more fringe cases. |
I had a silencedetect filter on the audio and I think I was receiving messages consistently ~30ms ahead of when they happen. Some things were also dependent on audio-buffer length, which was a hacky way for me to play around with that delay. The thing is, 25-50ms resolution is good enough for me at 1x speed at least for what I'm trying to do. But the problem at higher speeds like 4x is that you start to need an equivalent increase in time resolution to be able to do certain things. So instead of 50ms you now need 12.5ms resolution and your audio-buffer of 200ms now contains 800ms of the audio stream, so you start to notice timing issues if you need to change anything like speed, seeking, filters, etc.. Anyways, I'm not sure I really want to tackle this in my script right now. I may just reconsider doing things differently for now while waiting for a better fix. |
|
That makes sense. This change does increase the duration of the speed change since it tries to smooth out the sync over a long period of time. So there would be more time until whatever message you're waiting on is delivered. I'm not sure what the best approach for your usecase would be. Maybe new events, extending the script API, etc.? |
Yea, I will have to think about that. Right now, the A/V sync would probably help me personally the most, along with refreshing the A/V buffers quickly on filter and speed changes without disrupting playback or introducing gaps. I played with Another convenience may be a better version of |
This sounds doable. I dunno what a good name would be essentially it would just be passing back mpv's timer offset by whenever playback of the file starts. |
|
From what I can read I cannot say what parts my code will solve. Some of the problems sounds like they can be related to how video is synced to the audio. |
|
I can cherry-pick and push the first three commits from this for now (everyone is good with those) and then later when you're done we can compare approaches and maybe work out what's the best way to go. Edit: And done. |
This is basically just mostly ad hoc from looking at numbers. A large speed change is defined as a greater than 50% difference between the previous time frame value and the newly calculated one right after a speed change (arbitrary). When this is satisfied, there are two distinct possibilities: the time frame is either negative or positive. The negative case is actually surprisingly easy to solve. Negative time frame values are unacceptable since mpv is guaranteed to seek forward since the audio hasn't caught up yet. So just simply add a tiny negative value to mpctx->delay (to avoid AV from running away forever) and wait until the buffer goes positive again before returning back to normal. This prevents the frames from skipping forwards to weird places for at least normalish cases. The positive case is the tricky one. It has a bad tendency to lead to non-monotonic frame order (i.e. it can skip ahead, then go backwards, then back forwards again, etc.). This is because the initial frame after the speed change lingers on the screen for far too long which essentially causes havoc on the calculations and subsequent passes through the renderloop overcorrect in both directions until it settles on the "correct" frame and then proceeds normally. "Fix" this by basically doing some hacks. Since the source of the problem is mpctx->time_frame being too big, let's just arbitrarily reduce the value for a arbitrary amount of frames. Essentially what this does is smoothen out the change for a short period of time before we trust that the values are sane enough to allow the normal rendering to proceed. Up to 8x speed, this seems to work OK for me and the frames increas monotonically. This is probably about where the limit with this method is although going any higher will guarentee a/v desync anyways (you don't actually use speeds this stupid do you). The final thing here to consider is the display sync code path. It has similar problems, and the cause in this case is the calculated a_pos having a dramatic offset from the video pts which causes skipping frames when changing speed (mostly when you decreaase the speed though). In this case, what we do is simply hold the a_pos to the video pts until the a_pos catches back up to a reasonable difference. After that, allow the normal syncing to happen again.
If it wouldn't break much, perhaps this new property could replace Then you'd end up with |
|
@DanOscarsson Would you also be willing to share the original patchset (against the older mpv version)? |
|
My original patches are for the mpv code of before 2020. |
I just happen to use such an old version daily, so this isn't an issue for me. [In fact it'd be easier for me to cherrypick from the older patchset rather than trying to backport changes you've forward-ported to the newer version.]
This is interesting, how does it differ from |
|
Long ago I made patches for mplayer to adjust audio speed to video. |
As usual, for regressions I introduced.