Changes in Output settings take effect immediately #188
base: develop
Are you sure you want to change the base?
Conversation
src/mpc-hc/MainFrm.h
Outdated
@@ -387,11 +387,19 @@ class CMainFrame : public CFrameWnd, public CDropClient | |||
void StartWebServer(int nPort); | |||
void StopWebServer(); | |||
|
|||
int GetPlaybackMode() const { return m_iPlaybackMode; } | |||
bool IsPlaybackCaptureMode() const { return GetPlaybackMode() == PM_ANALOG_CAPTURE || GetPlaybackMode() == PM_DIGITAL_CAPTURE; } | |||
int GetPlaybackMode() const { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes seem unrelated. Please revert the unrelated changes and I'll have a look at astyle.
Please test with a live radio stream, or a raw audio file. In those cases duration is unknown, and seeking to last position may fail. Better restart from beginning if duration is unknown. |
I think forcefully resetting playback is little bit too much. Restarting playback can be done with single hotkey (CTRL+E by default) and user can do it once he finish changing settings. I think it would be annoying for me if playback reset occur automatically, especially that some files/steam can't be recovered in the same position. Also this patch is not complete, because if we want to reset playback there are many more options that should trigger this. Every other option on output page, subtitle renderer setting, all internal/external filters settings. Basically every change that can affect Directshow graph should reset playback with your approach. I don't like it. But if everyone thinks it is best we can think about merging this change. |
The VMR and EVR output settings already take effect immediately, same with the subtitle settings, so in a sense it's already a bit inconsistent. To me, it seems a bit surprising that changing the renderer, then clicking 'Apply' only applies the change on the next file loaded. If you, say, want to compare the same image with different renderer backends, this is cumbersome because you have to change the setting, close the settings, restart the file, and seek to the same point. |
I don't think users would change Output settings often in normal playback, so it's not that annoying. If someone tinkers with Output settings, they may try to see how different renderers behave, and having to restart playback for that every time may be actually more annoying. Maybe, if it's not too hard to implement, we could have a checkbox to disable/enable this behavior on the Output settings page. |
An extra option for this doesn't make sense. We should decide if we want to change the default behavior or not. Perhaps we should show a warning message an if the user confirms then restart playback or something like that. |
I wonder what would be a media stream that breaks this behaviour? As this is implemented, if the media has no duration, it simply "restarts" playback from the beginning, but generally if there is no duration, it's because it's a live stream (like internet radio), so even though we "restart" the playback, it's just continuing to stream it from the same position. |
Some streams that have duration are still not seekable. Anyway we can go with those changes, I still think we shouldn't restart playback, but I don't mind it really. I will take closer look soon. |
Updates the output settings page to automatically reload the current file at the same seek position after changing the video, audio, or subtitle renderers so the changes are visible immediately.