CrendKing / avisynth_filter

DirectShow filters that put AviSynth and VapourSynth into video playing
MIT License
107 stars 8 forks source link

Everything restarted on every track switching #54

Closed chainikdn closed 3 years ago

chainikdn commented 3 years ago

Since 0.10 (?). i.e. the remote control's HWND is changed and SVP thinks the player was closed. Is it intendent behavior?

CrendKing commented 3 years ago

You mean https://github.com/CrendKing/avisynth_filter/commit/58e08a3fa157204a7238046867fa8e946e7ccd22#diff-aee96ecbc8c7163a95d56c6871f0cf43cdc6f0ed8a23875c952ac217abdf47cdR99? I think the reason is, if RC is started at Active(), it should be stopped somewhere, and Inactive() is just the perfect place. Otherwise, RC will never shutdown. Besides, how would RC be useful when the player is inactive? Also, from the filter's perspective, switching track is effectively the same as the user pressed the stop button then play button.

I guess if SVP relies heavily on knowing if the player is closed or not, using OS API to detect PID's existence would be more reliable?

chainikdn commented 3 years ago

how would RC be useful when the player is inactive?

it's not about that when you stop the RC and DestroyWindow() , SVP think that playback is over and clear everything just too much of work plus 20 lines in the log, basically for nothing RC must be stopped when player is closed OR the video is closed / changed, and it used to work this way before. and this is how ffdshow works.

from the filter's perspective, switching track is effectively the same as the user pressed the stop button then play button.

pressing stop and then play doesn't have to delete RC too. again, like with ffdshow. "File -> Close" and "Play -> Stop" are different actions, obviously

CrendKing commented 3 years ago

when player is closed

Like I said, easy to detect by PID.

video is closed

Like I said, switching track is effectively stopping video.

For example, a user could very much

  1. Play a video
  2. Stop the video, but leaving the player open for half an hour, doing something else
  3. Close the player process

You are suggesting, in this case, even the video is indeed stopped, we should leave the RC active for the whole time? Because there is no way to tell the difference between switching track and real stopping video.

The only ugly way is "solve" this is, after Inactive(), don't stop RC immediately. Instead, wait for like a minute or so. If still no Active() been called, then stop RC. But again, the whole process can be replicated in SVP side. You see the RC window destroy, don't immediately release all resource. Instead, wait for a minute and see if new RC is created.

video is changed

RC API provides the video path, so SVP could detect if the video is changed or not from their side and only release resource if it is changed.

chainikdn commented 3 years ago

switching track is effectively stopping video

again, what exactly do you mean by "stopping video"? if "File -> Close" will delete everything including RC and "Play -> Stop" won't - this looks good to me

even the video is indeed stopped, we should leave the RC active for the whole time?

so what? yes, "leave the RC active for the whole time". what is the problem?


the filter is alive - > RC is alive filter deleted -> RC deleted so simple...

CrendKing commented 3 years ago

I agree that remote control should only be disrupted at video change/close time, which is at the destructor of the last filter instance (i.e. "filter deleted"). And I think the same goes for the frame handler's worker thread. There is no reason to stop that thread and start a new one when no video is changed/closed.

Please test https://github.com/CrendKing/avisynth_filter/actions/runs/946247582 (https://github.com/CrendKing/avisynth_filter/commit/d593dea5c82413f3384618a46b68304ba5779778). If it works as expected, please close the issue.

chainikdn commented 3 years ago

now track switching takes ~10 secs with spamming log with

Rejecting source sample due to start time going backward

and the source frame number is not reset after switching. I'm not an expert but probably BeginFlush()/EndFlush is still needed in Inactive()

CrendKing commented 3 years ago

You are correct:

https://github.com/CrendKing/avisynth_filter/actions/runs/946529407 (https://github.com/CrendKing/avisynth_filter/commit/a1a6f692af5d0e4fda3eaa54f3af8954674fd902)

chainikdn commented 3 years ago

looks good, thanks