clsid2 / mpc-hc

Media Player Classic
GNU General Public License v3.0
11.13k stars 492 forks source link

Multiple flushes of DirectShow chain on every seek #689

Closed chainikdn closed 3 years ago

chainikdn commented 3 years ago

So, here is the Avisynth Filter: https://github.com/CrendKing/avisynth_filter It reloads the Avisynth script on every CTransformInputPin::EndFlush() initiated by the upstream filter. For instance it reloads on every seek made by user. The problem is EndFlush() is called TWICE in a row on every seek when running in MPC-HC only. The problem doesn't occur in MPC-BE or PotPlayer with the same filter chain used (LAV splitter -> LAV decoder -> AVisynth Filter -> any renderer). More details there: https://github.com/CrendKing/avisynth_filter/issues/14#issuecomment-713463686

clsid2 commented 3 years ago

MPC-HC only calls SetPositions(). The flushing is all done by the filters.

CrendKing commented 3 years ago

I updated the call stack with function info, thanks to the pdb you uploaded. It looks like the MainFrm receives two WM_HSCROLL messages one immediate after another. The message itself is pretty much empty. I checked the state of the CPlayerSeekBar in both callback, and their states are also pretty much the same, only differing at those "hover point" related properties.

What's strange is, if I just set breakpoint in my filter, I can only catch one break. But if I change the breakpoint to no break and only print message to output window, I can see two messages. It is as if there are two threads both receiving the message, and the second thread can cancel the first thread if it has time to process some code, but can't if they happen exactly the same time. The only way I managed to get two breakpoint is to print some heavy messages at and entry of CMainFrame::WindowProc, which bogs down the responsiveness of Visual Studio significantly. I know there is one thread doing message handling though, so I'm not sure. It is just strange.

chainikdn commented 3 years ago

I just checked pdb too :) When clicking the seek bar SetPositions() is called twice with the exact same call stack. But seeking via Navigate -> Go to... works as expected - only one SetPositions() call. So I think the problem is in duplicated WM_HSCROLL messages.

if I just set breakpoint in my filter, I can only catch one break

I can see both

CrendKing commented 3 years ago

But seeking via Navigate -> Go to... works as expected - only one SetPositions() call

Can confirm. Also works if using keyboard shortcut to seek like 10 seconds forward/backward.

I also tested old MPC-HC version like 1.8.1 and the issue was present too. So if this is a bug for seekbar clicking, it has been hidden for years.

clsid2 commented 3 years ago

I will have a look later. But two messages does not necessarily mean it will also do two seeks, as it checks if the position has changed enough distance. This to prevent a huge number of seeks when for example dragging the seekbar.

@adipose, can you also check if you see any anomalies?

adipose commented 3 years ago

Well, I can see that it will send WM_HSCROLL when left button is pressed, and again when it is released. And also while dragging in between. It's not supposed to send the second one unless you were dragging in between, though.

adipose commented 3 years ago

https://github.com/clsid2/mpc-hc/pull/690/files

This should fix it for single clicks. What was happening was a call to SyncVideoToThumb() always on left button down, which immediately sends the hscroll. On lbutton up, it calls CheckScrollDistance to see if it needs another one, but the hover position is never updated unless there's actually scrolling, so it calls SyncVideoToThumb() again.

I updated lbutton down to just call the CheckScrollDistance as well, which will update the hover position to current position.

CrendKing commented 3 years ago

on left button down On lbutton up

Oh, that actually explained why if I set breakpoint only one break showed up. That's because there was only the button down event. The button up never registered since the Visual Studio window was brought to foreground.

adipose commented 3 years ago

Yeah debugging mouse events is kind of tricky without a second machine.

clsid2 commented 3 years ago

Hopefully fixed in 1.9.7.103

chainikdn commented 3 years ago

yep, thanks!