Skip to content

options: reset some options between files by default - #14151

Open
guidocella wants to merge 1 commit into
mpv-player:masterfrom
guidocella:reset-on-next-file
Open

options: reset some options between files by default#14151
guidocella wants to merge 1 commit into
mpv-player:masterfrom
guidocella:reset-on-next-file

Conversation

@guidocella

@guidocella guidocella commented May 15, 2024

Copy link
Copy Markdown
Contributor

Add some options to --reset-on-next-file's default value that you commonly don't want to keep between files. The catch is that if you specify options in the command line or mpv.conf --reset-on-next-file resets to those values, but the vast majority of uses for these options should be at runtime.

Fixes #17953.

@github-actions

github-actions Bot commented May 15, 2024

Copy link
Copy Markdown

Download the artifacts for this pull request:

Windows
macOS

@na-na-hi

na-na-hi commented May 15, 2024

Copy link
Copy Markdown
Contributor

that you commonly don't want to keep between files

This purely depends on the contents of the playlist in question. It's very often that all videos in the playlist come from closely-related sources. I don't want the crop and rotate settings to be reset for this reason. Switching to the next file and then switching it back also causes the settings to be reset.

Also it makes no sense to reset ab loop points without also resetting --ab-loop-count.

@guidocella
guidocella force-pushed the reset-on-next-file branch from 4879430 to 966a267 Compare May 15, 2024 17:05
@guidocella

Copy link
Copy Markdown
Contributor Author

I always wanted to reset rotate and crop to reset between files, it's hard to guess how common the use case you mentioned is. We can remove those and add ab-loop only if desired. Resetting ab-loop is also nice because it is not obvious how to add it to --reset-on-next-file (#10966). I added --ab-loop-count.

A tricky one is loop-file which ideally you could make it preserve numerical values between files without making it reset on next file if you change it at runtime; see #5943 #10966 #11291 https://www.reddit.com/r/mpv/comments/rcwnrw/looping_each_file_n_times_in_a_playlist/

and IRC discussions:

2023-09-18 17:47:53 ChH guido: kasper93: I didn't realize how weird loop-file is, I've always only used it with yes/no. I thought resetting it means that if mpv gets started without it and then set to 'yes' at runtime, then resetting it would mean it goes to 'no' when going to the next file.
2023-09-18 17:48:54 ChH imo that option should be changed so that it counts repeats (or seeks to the beginning, idc) for the current file instead of in total
...
2023-09-18 20:35:02 guido I just remembered that there was a lot of discussion about adding current-window-scale because option values shouldn't change on their own but actually loop-file already changes the value set by the user
2023-09-18 20:37:13 guido d07b7f0
2023-09-18 20:51:53 guido I guess there should be a remaining-loops property that decreases on each loop while --loop-file=N persists across files
2023-09-18 20:52:10 guido --loop-playlist=N also overwrites the value set by the user, but that doesn't cause issues
2023-09-18 20:56:01 ChH remaining-file-loops to make clear it doesn't have anything to do with loop-playlist
2023-09-18 20:56:23 guido right
2023-09-18 20:57:17 guido whether --loop-file=inf should reset on next file depends on individual preferences, but --loop-file=N should never reset

Add ab-loop options to --reset-on-next-file's default value because you
never want to preserve them between files. VLC and MPC-HC also reset
them.
@guidocella
guidocella force-pushed the reset-on-next-file branch from 966a267 to 83d583e Compare May 18, 2024 20:23
@guidocella

Copy link
Copy Markdown
Contributor Author

I made this ab loop only which should be uncontroversial.

@kasper93

Copy link
Copy Markdown
Member

Do we really need this change? Are we fixing anything? I can image ab-loop to be useful if you watch some series and want to skip recap/intro each time. Not sure how valid is that usecase, but maybe just don't introduce precedence of resetting anything.

@guidocella

Copy link
Copy Markdown
Contributor Author

That would be weird since episodes have different durations, but it's fine if the change is not worth it.

@guidocella guidocella closed this May 20, 2024
@guidocella
guidocella deleted the reset-on-next-file branch May 20, 2024 16:07
@guidocella
guidocella restored the reset-on-next-file branch May 21, 2026 09:45
@guidocella guidocella reopened this May 21, 2026
Comment thread options/options.c
Comment on lines +954 to +956
"ab-loop-a",
"ab-loop-b",
"ab-loop-count",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's so special about ab-loop? As opposed to dozen other properties that shouldn't be preserved between files? Do you plan to list them all here?

Also user can use file-local-options probably too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That it makes no sense to preserve loop points because after changing file they will refer to completely different scenes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as dozen different options, that are file specific.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how any other option is as file specific.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

All options in default --watch-later-options are pretty file specific. There is nothing that makes ab loop option different from others.

@guidocella guidocella May 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely wanted to preserve the options in --watch-later-options for different files except pan and crop, which autocrop.lua already sets locally. Never for ab loops.

@guidocella

Copy link
Copy Markdown
Contributor Author

This works too alternatively.

diff --git a/player/command.c b/player/command.c
index d6f7bfd50e..d78d564a7b 100644
--- a/player/command.c
+++ b/player/command.c
@@ -6974,10 +6974,10 @@ static void cmd_ab_loop(void *p)
 
     double now = get_current_time(mpctx);
     if (mpctx->opts->ab_loop[0] == MP_NOPTS_VALUE) {
-        mp_property_do("ab-loop-a", M_PROPERTY_SET, &now, mpctx);
+        mp_property_do("file-local-options/ab-loop-a", M_PROPERTY_SET, &now, mpctx);
         show_property_osd(mpctx, "ab-loop-a", cmd->on_osd);
     } else if (mpctx->opts->ab_loop[1] == MP_NOPTS_VALUE) {
-        mp_property_do("ab-loop-b", M_PROPERTY_SET, &now, mpctx);
+        mp_property_do("file-local-options/ab-loop-b", M_PROPERTY_SET, &now, mpctx);
         show_property_osd(mpctx, "ab-loop-b", cmd->on_osd);
     } else {
         now = MP_NOPTS_VALUE;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A/B Loop range should reset on new file

3 participants