mpv
mpv copied to clipboard
mpv-player
build: dynamically generate mpv.desktop file protocols
Download the artifacts for this pull request:
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
I also removed the bluray:// protocol alias in stream_bluray because that name is already used by an ffmpeg protocol which is now supported with this change.
The bluray:// change looks questionable, I can't get it to do anything with a folder structure (have no physical media to test).
Would mean we're just breaking something for our users for no reason.
NINOMIYA YUI % find . -maxdepth 2
.
./BDMV
./BDMV/STREAM
./BDMV/MovieObject.bdmv
./BDMV/index.bdmv
./BDMV/CLIPINF
./BDMV/PLAYLIST
./BDMV/BACKUP
./CERTIFICATE
./CERTIFICATE/id.bdmv
./CERTIFICATE/BACKUP
NINOMIYA YUI % ~/mpv/build/mpv bd:// --bluray-device=.
[bd] List of available titles:
[bd] idx: 0 duration: 00:00:34 (playlist: 00000.mpls)
[bd] idx: 1 duration: 00:00:01 (playlist: 00002.mpls)
[bd] idx: 2 duration: 00:46:47 (playlist: 00003.mpls)
Exiting... (Quit)
NINOMIYA YUI % ~/mpv/build/mpv bluray:// --bluray-device=.
disc.c:437: error opening file BDMV/index.bdmv
disc.c:437: error opening file BDMV/BACKUP/index.bdmv
[ffmpeg] bluray: bd_open() failed
Failed to open bluray://.
Exiting... (Errors when loading file)
NINOMIYA YUI % ~/mpv/build/mpv bluray://.
disc.c:437: error opening file BDMV/index.bdmv
disc.c:437: error opening file BDMV/BACKUP/index.bdmv
[ffmpeg] bluray: bd_open() failed
Failed to open bluray://..
Exiting... (Errors when loading file)
NINOMIYA YUI % ~/mpv/build/mpv bluray:.
[file] Cannot open file 'bluray:.': No such file or directory
Failed to open bluray:..
Exiting... (Errors when loading file)
NINOMIYA YUI % ~/mpv/build/mpv bluray://.
disc.c:437: error opening file BDMV/index.bdmv
disc.c:437: error opening file BDMV/BACKUP/index.bdmv
[ffmpeg] bluray: bd_open() failed
Failed to open bluray://..
Exiting... (Errors when loading file)
It's also not really in scope of this PR so maybe keep the whilelist approach for now?
So this works but I still don't see the selling point:
NINOMIYA YUI % ~/mpv/build/mpv lavf://bluray:.
bluray.c:299: 00007.m2ts: no timestamp for SPN 0 (got 0). clip 188838000-315192228.
(+) Video --vid=1 (h264 1920x1080 29.970fps)
(+) Audio --aid=1 (pcm_bluray 2ch 48000Hz)
AO: [pipewire] 48000Hz stereo 2ch s32
Using hardware decoding (vaapi).
VO: [gpu-next] 1920x1080 vaapi[nv12]
The bluray:// change looks questionable
Well we have to do something with it since otherwise bluray:// will get listed twice (from mpv and from ffmpeg). As for the actual command, mpv bluray://dev/sr0 works for me.
Well we have to do something with it since otherwise
bluray://will get listed twice (from mpv and from ffmpeg).
Hence my suggestion to reintroduce the old whitelist of unsafe ffmpeg protocols.
Or just exclude bluray specificially if you want to keep the rest.
bluray:// was only an alias, I think we can afford changing it. Users should use bd:// primarily.
bluray://was only an alias
It's not a deprecated one. There are other documented non-deprecated aliases which are important parts of the API. This needs to be deprecated for at least 2 releases before it can be changed.
Hence my suggestion to reintroduce the old whitelist of unsafe ffmpeg protocols. Or just exclude bluray specificially if you want to keep the rest.
Not a fan of either of these tbh. One of the whole points of this PR is to get rid of the hardcoded ffmpeg lists (sans the safe whitelist). I don't want to reduce the discoverability of protocols. Adding logic to specficially exclude bluray feels arbitrary and again hurts discoverability.
We don't have version compare macros, but if we had we could deprecate bluray:// and #if its exclusion behind VERSION < 0.40, after that it would be removed, even if we forget.
Is all that effort for bluray:// even worth it? Who even watches discs with mpv?
I can't answer this question, but we cannot have this habit of breaking things, just because it is probably not used.
Adding logic to specficially exclude
blurayfeels arbitrary and again hurts discoverability.
But users don't need to discover the ffmpeg bluray protocol. mpv has its own.
I get the naming conflict but I don't see an actual concrete advantage of switching one of the so far supported ways of playing BD disks to a somewhat incompatible different demuxer.
Why 2?
DOCS/compatibility.rst:
Important/often used parts must be deprecated for at least 2 releases before they can be broken. There must be at least 1 release where the deprecated functionality still works, and a replacement is available (if technically reasonable).
Unless this bluray thing is really useless I'd suggest following this protocol.
I guess I misunderstood what 2 means. I agree it has to be deprecated for at least one full release cycle. So it is deprecated in stable and removed in next stable. So I guess that makes it 2.
Anyway, I would just add an exception for bluray:// for now and it can be reevaluated in another PR if we want to remove it.
Please list a single reason on why we want to map bluray:// to ffmpeg. Everyone seems to be beating around the bush.
So the problem is that we lookup stream_info_ffmpeg and stream_info_ffmpeg_unsafe first. As I understand in this case dvd:// have exactly same problem as bluray://, no?
Is it possible to put these dynamic protocols to the end of probing order? Every time ffmpeg adds a protocol there is a potential for name conflict with a mpv built-in one. This also means that whether the built-in implementation is selected depends on whether ffmpeg is built with that protocol or not, which is certainly undesirable.
Please list a single reason on why we want to map bluray:// to ffmpeg.
Because stream_bluray is not really maintained in mpv at all? It's not like this is some wonderful feature that is thoroughly tested and constantly used. In fairness, the same is probably true for ffmpeg but why go out of our way to hide it?
As I understand in this case dvd:// have exactly same problem as bluray://, no?
Yes, I forgot that this was recently added. I don't know what the best way to deal with this is unfortunately. It's probable that ffmpeg's dvd is better than mpv's already (another thing that nobody maintains) or will be soon.
Is it possible to put these dynamic protocols to the end of probing order?
Sure I moved it to the bottom of the stream list.
Because stream_bluray is not really maintained in mpv at all? It's not like this is some wonderful feature that is thoroughly tested and constantly used. In fairness, the same is probably true for ffmpeg but why go out of our way to hide it?
The fact it is not maintained, doesn't mean it doesn't work. We don't get any complaints about that, so for the small amount of people who use it it works ok. Also ffmpeg playlist selection is very simple, maybe mpv is simple too, but protocol like this is not a ffmpeg job. I know we don't support menus and stuff, but ffmpeg will always just select playlist and try to play it, while media player like mpv may do more with it. mpv is not ffplay. And more complex things like bluray support should be implemented in the player, maybe some day someone wants to improve it, maybe not, but the API boundary between mpv and ffmpeg would make it impossible to do proper work.
Yes, I forgot that this was recently added. I don't know what the best way to deal with this is unfortunately. It's probable that ffmpeg's dvd is better than mpv's already (another thing that nobody maintains) or will be soon.
Completely disagree, same reasons as above. I know dvd demuxer was heavily crippled by wm4 and now it is not doing more "advanced" stuff like menus. But ffmpeg demuxer is in it's infancy and doesn't even support seeking currently with certain discs. ffmpeg is not some golden standard, look at their ML sometime... And similar thing is with our Matroska demuxer, our is just better, and while I recently added some missing feature, it still supports more things than lavf.
AFAIK, ffmpeg alleged supports bluray angles and mpv doesn't (did not personally test it). Also #7111 exists and to my knowledge it is still completely valid. So I would consider it to be pretty broken. I know from personal experience that seeking on CDs is totally unreliable. I doubt that DVDs or BDs are better. And again ffmpeg support is probably subpar as well but I don't see a strong reason why we should believe ours is automatically better and should be encouraged.
And similar thing is with our Matroska demuxer, our is just better
No I don't think this is similar at all. I was going to bring it up as a counterargument actually. Our mkv demuxer is undisputedly better and works well. You can not say the same about disc playback in mpv. It's crap here.
Fair. But even though our code may be subpar, we shouldn't try to hide it. If it is broken it has to be deprecated and removed. As long as we support certain protocol, first-party mapping should prefer our code for better or for worse.
You can use ffmpeg://bluray:// or lavf://bluray:// if you want to bypass it.
I'm only suggesting that we show both in --list-protocols. Maybe document the difference if people feel strongly enough about it. The problem is the naming.
If the ffmpeg demuxers are better than what we have then it would be reasonable to plan to drop ours entirely. It just shouldn't be done half way and drive-by in this PR.
We should also think about compatibility for --bluray-device and --dvd-device or the protocol path/args.
So for this PR I propose
- moving the ffmpeg to probe last to prevent possible future name conflicts
- hiding the
dvdandblurayffmpeg protocols
I know dvd demuxer was heavily crippled by wm4 and now it is not doing more "advanced" stuff like menus.
mpv never supported DVD menues. What wm4 did was remove some disc-specific hacks that were needed to have seeking work correctly on DVD/BD because the extra code was bothering him I guess, and it's been broken since.
It just shouldn't be done half way and drive-by in this PR.
No one suggested that and this PR doesn't do that. I don't understand why "just list both" is apparently so controversial but whatever I will stop wasting my time on this and forcibly drop the ffmpeg listings. It's not like anyone even uses discs with mpv anyway.
Edit: and done.
mpv never supported DVD menues
It was supported in mplayer and was even rewritten once for mpv: 41fbcee, 0530447
mpv never supported DVD menues.
It did. But either way, got removed, because it interfered with internals. And that's fine, I guess if someone not want to maintain some parts of code. But it is still something that is doable in mpv codebase, but would be impossible to do in ffmpeg. ffmpeg is not really designed to support interactivity.