player: set playlist title to media title if not set already #10453
Conversation
christoph-heinrich
force-pushed
the
playlist_title
branch
from
098527b
to
871b24e
Compare
christoph-heinrich
changed the title
The playlist title only got set when it was specified in the playlist file. If there is a title after opening a file, that should also be reflected in the playlist. ref. mpv-player#4780
christoph-heinrich
force-pushed
the
playlist_title
branch
from
871b24e
to
9c94d33
Compare
|
Seems pretty good. The only nit I can find is that the |
|
I don't see a problem with |
|
If I set |
ytdl_hook always set force-media-title, making users unable to force a media-title via options. Now the title gets set as tag when possible, and multi-arc videos check if force-media-title is already set to avoid overwriting it. ref. mpv-player#10453 (comment)
ytdl_hook always set force-media-title, making users unable to force a media-title via options. To prevent that, check if force-media-title is already set to avoid overwriting it. ref. mpv-player#10453 (comment)
ytdl_hook always set force-media-title, making users unable to force a media-title via options. To prevent that, check if force-media-title is already set to avoid overwriting it. A better solution would be to use tags like is already done for some metadata, however that doesn't work when `all_formats=yes` is used. See cbb8f53 A comment was added to hint at why it isn't done via tags. ref. mpv-player#10453 (comment)
ytdl_hook always set force-media-title, making users unable to force a media-title via options. To prevent that, check if force-media-title is already set to avoid overwriting it. A better solution would be to use tags like is already done for some metadata, however that doesn't work when `all_formats=yes` is used. See cbb8f53 A comment was added to hint at why it isn't done via tags. ref. #10453 (comment)
| if (!name || !name[0]){ | ||
| name = pe->title; | ||
| } else if (!pe->title) { | ||
| pe->title = talloc_strdup(pe, name); |
There was a problem hiding this comment.
This is the property handler for media-title.
- why does this modify a playlist entry?
- how does this work if nobody touches
media-title?
There was a problem hiding this comment.
The way I understood it is that this handler runs every time the media-title might change because of
Line 3983 in c5d1230
media-title is filename if there is no other title available, and setting the playlist title to filename doesn't really make sense. The simplest way of setting it only when media-title is something other then the filename is in the media-title handler itself.
There was a problem hiding this comment.
If we want to set the playlist title to the media title in certain cases, it needs to be done in the media-title property handler not the playlist one since it would need to update itself anytime the media-title changes.
There was a problem hiding this comment.
The way I understood it is that this handler runs every time the media-title might change
No, it runs everytime one of the mp_property_actions happens on the property, such as getting, setting, incrementing it.
The code that causes MP_EVENT_METADATA_UPDATE to happen would be in player/loadfile.c
There was a problem hiding this comment.
You're right, I thought it was fine because the handler usually gets a ton of calls, but handler never gets called when running mpv --no-config --no-terminal --no-osc --title=foo <some file>.
Any suggestions on a better way of implementing that?
There was a problem hiding this comment.
Not sure, maybe in update_demuxer_properties or some other place that deals with playlist changes.
The title in the playlist should get set when we get a title and the playlist title isn't set already.
#8548 has essentially the same goal, but this has the advantage that
Somewhat closes #4780, but still only shows titles for files/urls that have been opened before. I don't think we should make a bunch of requests just to get the titles.