Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use the same default optical drive for everything #15889

Merged
merged 3 commits into from
Feb 17, 2025

Conversation

Dudemanguy
Copy link
Member

A trip down mplayer commits.

Copy link

github-actions bot commented Feb 17, 2025

Download the artifacts for this pull request:

Windows
macOS

Amusingly, this was not working correctly for at least a decade. For
all OSes, the default drive is the same except for macOS. It had
"/dev/diskN" as the default dvd drive. Back in the mplayer days, there
was actually dynamic compile time generation (only for mac) that
replaced the N with the actual number*. But this didn't actually work
for all stream types. It seems like it never worked for stream_dvdnav
and was only a stream_dvd (libdvdread) feature. And then later that
stream got removed completely. So at least for macOS users, they had a
fallback device of "/dev/diskN" for certain stream types and for a very
long time.

We can't really do better here, but we should at least pick a default
string that has a chance of working. So /dev/disk1 it is then. With
that, all of the devices become the same so we can combine it all and
use the same default optical drive for both CDs and DVDs. BD will be
added in the next commit. Also /dev/dvd is some antiquated thing that
doesn't really exist anymore. Linux systems will likely still have
/dev/cdrom which has a decent chance of being usable. /dev/dvd probably
won't be there.

*: f215e84
Unlike the other ones, stream_bluray had no default drive to choose as a
fallback so make it use DEFAULT_OPTICAL_DRIVE.
@Dudemanguy Dudemanguy force-pushed the default-optical-device branch from a7240c8 to b1ba230 Compare February 17, 2025 02:37
@kasper93
Copy link
Contributor

Needs docs update, specifically the dvd part

Specify the DVD device or .iso filename (default: ``/dev/dvd``). You can

@Dudemanguy
Copy link
Member Author

Dudemanguy commented Feb 17, 2025

Ugh, those docs aren't even right today. Who is compiling mpv on a non-Windows, non-darwin, non-linux, and non-BSD system?

Edit: added a doc commit for that.

Unless you were compiling on an obscure OS, the default drive location
was never /dev/dvd.
Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

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

Makes sense. Selecting D: on Windows and /dev/disk1 on macOS is kinda whatever. But it was like that anyway, so status quo is preserved.

@Dudemanguy Dudemanguy merged commit ae1ec5e into mpv-player:master Feb 17, 2025
27 checks passed
@Dudemanguy Dudemanguy deleted the default-optical-device branch February 17, 2025 21:15
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.

2 participants