mac: fix screen selection edge case

[makigumo]

For one of my monitors NSScreen.displayName was reporting nil. This lead to the following two issues.

• It was not possible to select the monitor by name, e.g. --screen-name or --fs-screen-name. This is addressed by an additional name check with the system provided localizedName.
• If no output monitor was specified, it would always default to this nil-monitor because there would be a match for screen.displayName == screenName thus returning this monitor, instead of returning nil (the default). This is addressed with a check for nil.

[makigumo]

#9697 may be related

[Akemi]

if we want to use the localizedName if the displayName is not available we also need to adjust the retrieval for the display-names property. since they should match. this part of the code https://github.com/mpv-player/mpv/blob/master/video/out/mac/common.swift#L642-L651

also localizedName is only available on macOS 10.15+, so it can't be used like this without a check.

maybe there is a bug in the retrieval of the display name in our custom code/extension https://github.com/mpv-player/mpv/blob/master/osdep/macos/swift_extensions.swift#L32-L62 that should be fixed instead.

ist this on an ARM mac?

[makigumo]

Ah I see. I wasn't aware localizedName was kind of new. Btw, I'm on Intel.

I agree its probably best to fix it in swift_extensions.swift

After digging a little deeper I found that the nil-monitor doesn't satisfy the last condition here: https://github.com/mpv-player/mpv/blob/5186651f304fb49867f2542d2eb149f8191cf0de/osdep/macos/swift_extensions.swift#L47

info[kDisplaySerialNumber] is nil but CGDisplaySerialNumber(displayID) is not 0. I'm not sure why that is. Maybe because it's an old DVI monitor.

[Akemi]

if the localizedName is the new official way to retrieve the display name now, we should replace all the usages of displayName with localizedName. i only added displayName since there wasn't a property to retrieve it before 10.15.

i think the best way would probably be, to move the displayName extension to https://github.com/mpv-player/mpv/blob/master/osdep/macos/swift_compat.swift, rename it to localizedName, make it conditional compile only for SDKs older than 10.15 (we should be done anyway) and then replace all the usages of displayName with localizedName. that way it can be removed easily at some point of time when we drop support for those older macOS versions.

[makigumo]

I have tried to follow your suggestion. I hope I didn't miss anything. Please have a look.

[Akemi]

you can completely remove the code from osdep/macos/swift_extensions.swift, the one for the displayName i mean.

also i think the macOS detection for macOS versions after 10.15 can be simplified. something like:

if int(version_parts.group(1)) >= 20:
    build_version = str(major-9) + '.' + str(minor)

[makigumo]

Thank you, somehow I missed to remove displayName.

[Akemi]

the compile time check won't work like that. mpv build with SDK 10.15 but min system set to 10.14 for example will fail, since the localizedName is not available. sry about not catching that earlier.

we probably need to keep the displayName and what you had there before, returning the localizedName inside the displayName. though addtional to the run time system check we also need the compile time check, but for availability.

[makigumo]

No problem. Next try.

[Akemi]

https://github.com/mpv-player/mpv/blob/5186651f304fb49867f2542d2eb149f8191cf0de/osdep/macos/swift_extensions.swift#L47

info[kDisplaySerialNumber] is nil but CGDisplaySerialNumber(displayID) is not 0. I'm not sure why that is. Maybe because it's an old DVI monitor.

would you mind telling what the values the keys in the dictionary contain and the CGDisplay* return values, eg

info[kDisplayVendorID]
CGDisplayVendorNumber(displayID)
info[kDisplayProductID]
CGDisplayModelNumber(displayID)
info[kDisplaySerialNumber]
CGDisplaySerialNumber(displayID))

i would like to fix the old case too if possible.

[makigumo]

No problem, here it is.

----
info[kDisplayProductName] = Optional({
    "en_US" = X223HQ;
})
--
info[kDisplayVendorID] = Optional(1138)
CGDisplayVendorNumber(displayID) = 1138
info[kDisplayProductID] = Optional(152)
CGDisplayModelNumber(displayID) = 152
info[kDisplaySerialNumber] = Optional(-1807666473)
info[kDisplaySerialNumber] as? UInt32 = nil
CGDisplaySerialNumber(displayID) = 2487300823

I would probably just remove the serial number check. As I would expect the monitor name to be the same for all models with a specific vendor and model number anyway.

[noma4i]

any updates? MPV suffer from this issue

[Akemi]

there are a few things i still need to think about and some possible changes. though i am currently short on time.

[makigumo]

I think I have found a fix for the serial number mismatch. Please have a look as I'm not that experienced with Swift.

[makigumo]

@Akemi Anything more required? Only cd9dbd41a479865d6d18a075a36b63918fe97a63 is really needed to solve the issue. And maybe ea78d494e7044587b9661bc94f7b5e80163c6e13 to be on the safer side. The other commits are more of a nice-to-have.

[makigumo]

Addressed https://github.com/mpv-player/mpv/pull/9885#issuecomment-1046307112, which I somehow totally missed. Sorry about that.

[Akemi]

as a heads up, i didn't forget about this and am trying to look at this soonish. sorry for the long wait.

[forthrin]

Applying only https://github.com/mpv-player/mpv/commit/ea78d494e7044587b9661bc94f7b5e80163c6e13 resolved this for me. When will it be merged into master? It's quite an important fix, since one can't watch movies in setups with external screens without it (ie. something as essential as a media center and a TV).

[Akemi]

it should probably also be mentioned in our https://github.com/mpv-player/mpv/blob/master/DOCS/interface-changes.rst file. since the output of VOCTRL_GET_DISPLAY_NAMES changed and scripts/properties etc depend on it.

[makigumo]

it should probably also be mentioned in our https://github.com/mpv-player/mpv/blob/master/DOCS/interface-changes.rst file. since the output of VOCTRL_GET_DISPLAY_NAMES changed and scripts/properties etc depend on it.

Not sure what if the provided info is sufficient. So please feel free to edit/correct this further.

[nightfriendly]

@makigumo @Akemi is there something we can do to help?

#9697 is the last bug that prevents me from using the newest mpv on MacOS (I'm still on 0.30...).

[Akemi]

not really. i am without a working mac still. so it's basically my fault for being stuck.

[nightfriendly]

Don't beat yourself too hard man, it's really not your fault if developing for MacOS is such a pain.

Is there any way we can help as a community? I am ready to donate. A former mplayer user, I have been using mpv since 2014. First on Linux, then on Mac, and I cannot imagine my life without it!

[forthrin]

@Akemi: What is the earliest Mac/macOS you can use?

[Akemi]

@Akemi: What is the earliest Mac/macOS you can use?

i am not sure how to answer or rather what the intention behind that question is. the first macOS/OSX i used was 10.5.

anyway i finally ordered a new mac now, since my backup computer broke too a few days ago.

[forthrin]

@Akemi: Maybe someone had a surplus Mac lying around. PS! It's not paranoia when they're really after you.

[Akemi]

ah i see, thanks. that would have been very kind of you.

[Akemi]

closing in favour of #12846

[makigumo]

EDIT: It's fine.

[Hamuko]

Partially works on my machine.

mpv --no-config --fs-screen=0 --fs launches directly to fullscreen on my primary display.

mpv --no-config --fs-screen=1 --fs launches on the secondary display, but not in fullscreen. Instead, mpv opens up behind Firefox, which is the only window I have open on the secondary screen, and an alert bong is heard. Selecting the window and hitting the fullscreen hotkey does make it fullscreen on the secondary display though.

Apple M1 Max Mac Studio, macOS 13.6, mpv v0.35.0-1399-ga54cc02341.