Color Management (OS X): Use system configured display profile automatically

[UliZappe]

Note: I can argue for OS X only, but of course, the issue might be relevant for the other platforms, too.

Now that color management works correctly :-) , it would certainly make sense from a usage point of view to offer an option to automatically use the system configured ICC display profile automatically, instead of having to configure it explicitly for mpv.

I.e. something like :icc-profile=auto should automatically use whatever display profile the user has set in the OS X System Preferences.

I will provide the code to figure out the path of the system’s display profile below, but I have too little insight in mpv’s architecture to know how to integrate this functionality correctly.

I can see two major architectural questions:

1. Can the color management LUT table be changed during playback?

If yes, mpv could adjust its color if the user changes its display profile during playback. (It would register to get a notification whenever the user changes the display profile for a screen, which is easy to implement.)

This is probably not the most important feature, but it could be useful. For instance, I use two different display profiles for daylight and artificial light. If I start watching a movie during the day, pause it, and continue in the night, such intelligent adaptation would be useful. Again, not the most important feature, but one that would make the integration in the OS "perfect". (QuickTime Player Pro X does it.)

2. In a multi-display configuration, can the user change the display during playback?

If yes, then point 1.) – the possibility to change the display profile during playback – would basically become mandatory, since different displays will almost certainly have different display profiles. And if I understand the --screen and --fullscreen options correctly, this is indeed the case.

The specific API of the function that provides the path of the display profile will probably depend on the answer to these two questions.

If no change of the display profile during playback is required and only the display profile of the currently used display needs to be set at launch, the following function is sufficient:

 Boolean get_screen_profile(char *pathBuffer, size_t maxPathSize)
{
    CFUUIDRef displayUUID = CGDisplayCreateUUIDFromDisplayID(kCGDirectMainDisplay);
    if (CFGetTypeID(displayUUID) == CFNullGetTypeID()) {MP_ERR(s, "Cannot get display UUID.\n"); return false;}

    CFDictionaryRef displayInfo = ColorSyncDeviceCopyDeviceInfo(kColorSyncDisplayDeviceClass, displayUUID);
    if (!displayInfo) {MP_ERR(s, "Cannot get display info.\n"); CFRelease(displayUUID); return false;}
    CFRelease(displayUUID);
    CFDictionaryRef customProfileInfo = CFDictionaryGetValue(displayInfo, kColorSyncCustomProfiles);
    if (!customProfileInfo) {MP_ERR(s, "Cannot get display profile info.\n"); CFRelease(displayInfo); return false;}

    CFURLRef customProfileURLs[CFDictionaryGetCount(customProfileInfo)];
    CFDictionaryGetKeysAndValues(customProfileInfo, NULL, (const void **)customProfileURLs);
    if ((void*)customProfileURLs[0] == kCFNull) {MP_ERR(s, "Cannot get display profile URL.\n"); CFRelease(displayInfo); return false;}
    Boolean result = CFURLGetFileSystemRepresentation(customProfileURLs[0], true, (UInt8*)pathBuffer, maxPathSize);
    CFRelease(displayInfo);

    if (!result) MP_ERR(s, "Cannot get display profile path.\n");
    return result;
}

(Note that I’m unsure about the log levels you would consider adequate.)

You can directly insert this function as is in video/out/cocoa_common.m, or even in video/out/gl_lcms.c if you additionally add

#include <ApplicationServices/ApplicationServices.h>

You would call the function as follows:

char buffer[PATH_MAX];
Boolean result = get_screen_profile(buffer, PATH_MAX);

If successful, buffer will contain the path to the ICC display profile of the current display, which can then be used for the _loadfile() call in _mp_loadicc().

However, if the profile must be changeable during playback, you’ll probably need an API similar to this in video/out/cocoa_common.m (EDIT: updated code for retrieving the display ID):

static Boolean get_screen_profile(struct vo *vo, Boolean fullscreen, char *pathBuffer, size_t maxPathSize)
{
    struct vo_cocoa_state *s = vo->cocoa;
    NSScreen *screen = fullscreen? s->fs_screen : s->current_screen;
    const char *screenstring = fullscreen? "full screen " : "";

    CGDirectDisplayID displayID = (CGDirectDisplayID)[[[screen deviceDescription] valueForKey:@"NSScreenNumber"] unsignedLongValue];
    CFUUIDRef displayUUID = CGDisplayCreateUUIDFromDisplayID(displayID);
    if (CFGetTypeID(displayUUID) == CFNullGetTypeID()) {MP_ERR(s, "Cannot get %sdisplay UUID.\n", screenstring); return false;}

    CFDictionaryRef displayInfo = ColorSyncDeviceCopyDeviceInfo(kColorSyncDisplayDeviceClass, displayUUID);
    if (!displayInfo) {MP_ERR(s, "Cannot get %sdisplay info.\n", screenstring); CFRelease(displayUUID); return false;}
    CFRelease(displayUUID);
    CFDictionaryRef customProfileInfo = CFDictionaryGetValue(displayInfo, kColorSyncCustomProfiles);
    if (!customProfileInfo) {MP_ERR(s, "Cannot get %sdisplay profile info.\n", screenstring); CFRelease(displayInfo); return false;}

    CFURLRef customProfileURLs[CFDictionaryGetCount(customProfileInfo)];
    CFDictionaryGetKeysAndValues(customProfileInfo, NULL, (const void **)customProfileURLs);
    if ((void*)customProfileURLs[0] == kCFNull) {MP_ERR(s, "Cannot get %sdisplay profile URL.\n", screenstring); CFRelease(displayInfo); return false;}
    Boolean result = CFURLGetFileSystemRepresentation(customProfileURLs[0], true, (UInt8*)pathBuffer, maxPathSize);
    CFRelease(displayInfo);

    if (!result) MP_ERR(s, "Cannot get %sdisplay profile path.\n", screenstring);
    return result;
}

A pseudo code example to call this function would be:

void vo_cocoa_update_screen_profile_info(struct vo *vo)
{
    char buffer[PATH_MAX];
    if (get_screen_profile(vo, false, buffer, PATH_MAX)) printf("current screen profile: %s\n", buffer);
    if (get_screen_profile(vo, true, buffer, PATH_MAX)) printf("current fullscreen profile: %s\n", buffer);
}

This code example simply prints out the ICC profile paths for the screen and the fullscreen display. I’m not too sure about how mpv uses the _vo_cocoastate struct and about the meaning of its _fsscreen and _currentscreen members, and how an update of its values would be triggered during playback.

What do you think?

[pigoz]

I was working on this two days ago. Yesterday I got home really late because of RL issues and could not complete my code. I didn't know about ColorSyncDeviceCopyDeviceInfo so in my code I just iterated of all color profiles. I will probably have a working commit this evening or tomorrow.

[haasn]

Does anybody know what the situation is like on Linux? I don't know if X even has any sort of concept of display profiles, but surely even if it doesn't some standards must exist to combat this.

It would be a cool feature to get profile auto-detection for other platforms as well. I know Windows has a concept of display profiles, too.

Edit: This seems relevant: http://www.argyllcms.com/doc/ucmm.html

Looks like my system indeed has it set up already, due to the wonders of dispcalGUI. Do we already rely on XDG stuff?

Edit: This seems relevant, too: http://www.burtonini.com/computing/x-icc-profiles-spec-0.2.html

Less reliant on XDG, too.

[pigoz]

No idea, I discussed this a few days ago with wm4 and this will implemented as a VOCTL command where the backends can reply with a profile path.

The main problem is that changing the profile during playback, will probably require some sort of VO reinitialization (something that is not currently handled in mpv's architecture).

[haasn]

I think that is a separate issue and it would be nice to get profile autodetection working at all for now.

Edit: With argyll-dispwin, if you install the .icc profile with -I then the _ICC_PROFILE property of the root X window seems to hold the profile as intended, at least over here.

[ghost]

The main problem is that changing the profile during playback, will probably require some sort of VO reinitialization (something that is not currently handled in mpv's architecture).

I think switching between existing profiles wouldn't be too hard, but we need some kind of notification mechanism etc., so it would require figuring out how it should work. vo_opengl itself probably won't need a full reinit. It just has to recompute the lookup table, and then make gl_video.c to reinitialize somehow.

Another thing is that this would break the icc-cache. Maybe that's ok, since most will use that only when configuring the icc profile manually.

[ghost]

Edit: This seems relevant: http://www.argyllcms.com/doc/ucmm.html

Looks like my system indeed has it set up already, due to the wonders of dispcalGUI. Do we already rely on XDG stuff?

Edit: This seems relevant, too: http://www.burtonini.com/computing/x-icc-profiles-spec-0.2.html

Less reliant on XDG, too.

Both of these look pretty annoying. First we should find out which one is "more" standard.

PS: and of course, color management on Linux is a ghetto. Maybe there isn't even a standard behavior across the desktop environments.

Also, @giselher: do you know if Wayland has anything?

[UliZappe]

@wm4

I think switching between existing profiles wouldn't be too hard, but we need some kind of notification mechanism

In OS X, this would be NSScreenColorSpaceDidChangeNotification, but @pigoz is certainly aware of that.

If additional screens are added (e.g. a projector is connected to a laptop), OS X sends an NSApplicationDidChangeScreenParametersNotification; then, mpv would have to register for yet another NSScreenColorSpaceDidChangeNotification for this new screen.

Also useful might be [NScreen deepestScreen], which returns the display with the highest color quality (bit depth), so if e.g. a laptop display (usually lower bit depths and dithering) and a projector are in mirroring mode, the "deepest screen" would be the projector, and its color profile should be used.

[ghost]

Well, I mean mpv-internally. Basically an annoying design decision how to propagate the system notification.

[UliZappe]

I found an improvement for the code of the second function mentioned above:

 static Boolean get_screen_profile(struct vo *vo, Boolean fullscreen, char *pathBuffer, size_t maxPathSize)

I overlooked the direct way to get the display ID from an NSScreen, so I used an awkward piece of code which retrieves the display ID from the frame of the NSScreen:

CGDirectDisplayID displayID;
uint32_t count;
CGError error =  CGGetDisplaysWithRect(NSRectToCGRect([screen frame]), 1, &displayID, &count);
if (error) {MP_ERR(s, "Cannot get %sdisplay ID.\n", screenstring); return false;}

The correct replacement for this code is:

 CGDirectDisplayID displayID = (CGDirectDisplayID)[[[screen deviceDescription] valueForKey:@"NSScreenNumber"] unsignedLongValue];

I will update the original code above accordingly. Only the new version should be used.

[pigoz]

Sorry, I was really busy with RL stuff the past week. I have started working on it on my commute.

Why do you get the first element of the custom profiles dictionary values array? Is that always guaranteed to be the profile currently in use?

Apart from that, your code looks ok and I'm integrating it into mpv. On a first commit I will probably ignore the issues with regard to monitor switching and just use the current monitor. This is not only a color management issue, since it's also a problem with monitors with different pixel density (retina vs normal), color depth, etc.

[UliZappe]

Why do you get the first element of the custom profiles dictionary values array? Is that always guaranteed to be the profile currently in use?

It seems so. Unfortunately, there’s no documentation apart from the header files, which don’t say anything about that.

But in my tests, this has always worked. In fact, I don’t even think you can set more than 1 "custom" (= current) profile for a monitor (per user, but we only care about the current user). The usual GUI way, via System Preferences > Displays > Color, always replaces the previous custom profile by the new one (if you select the default profile from Apple, then this profile also becomes the current "custom" profile), and when you use ColorSync Utility > Devices to set the current profile, the Open panel with which you specify the path to the new custom profile only lets you select 1 profile which, again, replaces the old custom profile. This differs from printers, where (depending on the printer driver) you might have a custom profile for RGB printing, one for CMYK, one for gray etc.

[UliZappe]

I just checked again and I found one exception:

If you select the monitor default profile in ColorSync Utility > Devices, then (contrary to doing so in System Preferences) the "custom profile" becomes actually undefined and the code must fall back to the default profile instead. This is probably the state after a fresh OS X installation, so it certainly matters. I will update my code accordingly tonight.

[pigoz]

I think its just a matter of querying for kColorSyncFactoryProfiles as well. If that's the case I already am making the needed code.

[UliZappe]

True, but currently I am testing this and get strange behavior. I’ll try a bit more.

[UliZappe]

OK, I rewrote the whole thing. It’s a bit convoluted, but it’s now hopefully as "ColorSync official" as it gets. (It also removes the arbitrary selection of the first returned custom profile that you asked about.)

If you came to basically the same result, then obviously you need not care. :-)

static Boolean get_screen_profile(struct vo *vo, Boolean fullscreen, char *pathBuffer, size_t maxPathSize)
{
    Boolean result;
    Boolean useOnlyEntry = false;
    CFTypeRef valueArray[1];
    CFURLRef profileURL;

    struct vo_cocoa_state *s = vo->cocoa;
    NSScreen *screen = fullscreen? s->fs_screen : s->current_screen;
    const char *screenstring = fullscreen? "full screen " : "";

    CGDirectDisplayID displayID = (CGDirectDisplayID)[[[screen deviceDescription] valueForKey:@"NSScreenNumber"] unsignedLongValue];

    CFUUIDRef displayUUID = CGDisplayCreateUUIDFromDisplayID(displayID); // for the current display, use kCGDirectMainDisplay instead of displayID
    if (CFGetTypeID(displayUUID) == CFNullGetTypeID()) {MP_ERR(s, "Cannot get %sdisplay UUID.\n", screenstring); return false;}

    CFDictionaryRef deviceInfo = ColorSyncDeviceCopyDeviceInfo(kColorSyncDisplayDeviceClass, displayUUID);
    if (!deviceInfo) {MP_ERR(s, "Cannot get %sdisplay info.\n", screenstring); CFRelease(displayUUID); return false;}
    CFRelease(displayUUID);

    CFDictionaryRef factoryInfo = CFDictionaryGetValue(deviceInfo, kColorSyncFactoryProfiles);
    if (!factoryInfo) {MP_ERR(s, "Cannot get %sdisplay factory settings.\n", screenstring); CFRelease(deviceInfo); return false;}
    CFStringRef defaultProfileID = CFDictionaryGetValue(factoryInfo, kColorSyncDeviceDefaultProfileID);
    if (!defaultProfileID)  useOnlyEntry = true;        // if no profile ID is specified, the profile dictionaries are guaranteed to contain only one entry, which we will use

    CFDictionaryRef profileInfo = CFDictionaryGetValue(deviceInfo, kColorSyncCustomProfiles);
    if (profileInfo)
    {
        if (useOnlyEntry)
        {
            if (CFDictionaryGetCount(profileInfo) != 1) {MP_ERR(s, "Inconsistent %sdisplay profile setting.\n", screenstring); CFRelease(deviceInfo); return false;}
            CFDictionaryGetKeysAndValues(profileInfo, NULL, (const void **)valueArray);
            // If profileURL is kCFNull or NULL, the profile URL could not be retrieved although a custom profile was specified.
            // This points to a configuration error, so we should not fall back to the factory profile, but return an error instead.
            if ((void*)valueArray[0] == kCFNull) {MP_ERR(s, "Cannot get %sdisplay profile URL.\n", screenstring); CFRelease(deviceInfo); return false;}
            profileURL = valueArray[0];
        }
        else profileURL = CFDictionaryGetValue(profileInfo, defaultProfileID);
        if (!profileURL) {MP_ERR(s, "Cannot get %sdisplay profile URL.\n", screenstring); CFRelease(deviceInfo); return false;}
    }
    else    // No custom profile specified; try factory profile for the device
    {
        CFDictionaryRef defaultProfileInfo;

        if (useOnlyEntry)
        {
            if (CFDictionaryGetCount(factoryInfo) != 1) {MP_ERR(s, "Inconsistent %sdisplay factory profile setting.\n", screenstring); CFRelease(deviceInfo); return false;}
            CFDictionaryGetKeysAndValues(factoryInfo, NULL, (const void **)valueArray);
            if ((void*)valueArray[0] == kCFNull) {MP_ERR(s, "Cannot get %sdisplay profile info.\n", screenstring); CFRelease(deviceInfo); return false;}
            defaultProfileInfo = valueArray[0];
        }           
        else defaultProfileInfo = CFDictionaryGetValue(factoryInfo, defaultProfileID);
        if (!defaultProfileInfo) {MP_ERR(s, "Cannot get %sdisplay profile info.\n", screenstring); CFRelease(deviceInfo); return false;}
        profileURL = CFDictionaryGetValue(defaultProfileInfo, kColorSyncDeviceProfileURL);
        if (!profileURL) {MP_ERR(s, "Cannot get %sdisplay factory profile URL.\n", screenstring); CFRelease(deviceInfo); return false;}
    }

    result = CFURLGetFileSystemRepresentation(profileURL, true, (UInt8*)pathBuffer, maxPathSize);
    CFRelease(deviceInfo);
    if (!result) MP_ERR(s, "Cannot get %sdisplay profile path.\n", screenstring);
    return result;
}

[UliZappe]

it’s now hopefully as "ColorSync official" as it gets

Well, it still isn’t, as I just realized that kColorSyncDeviceDefaultProfileID is optional if only one factory profile exists (which will probably always be the case for displays). Apple does specify it, but we cannot rely on that (if e.g. some third-party profiling software modifies this setting). Sigh – this makes the whole thing even more complex, as we’ll have to write code to use the first = only value of the dictionaries in case kColorSyncDeviceDefaultProfileID is undefined. I’ll try to supplement the above code with that.

[UliZappe]

I updated the code. As long as I didn’t make some stupid copy&paste error, it should work.

[UliZappe]

As long as I didn’t make some stupid copy&paste error

I did make two, but they are fixed now.

[UliZappe]

Just for the record, let me clear up Apple’s terminology.

Device profiles can be factory, default and current.

For instance, my HP color laser printer has an RGB, a CMYK and a Gray mode, and comes with factory profiles for all modes. By default, it expects CMYK data. I built additional custom profiles for the CMYK and RGB modes which I configured instead of the factory profiles. The result of all this is as follows:

Profile	Factory	Default	Current
HPProfileCMYK	yes	no	no
HPProfileRGB	yes	no	no
HPProfileGray	yes	no	yes
myProfileCMYK	no	yes	yes
myProfileRGB	no	no	yes

Note that there are several current profiles; one for each mode (CMYK, RGB and Gray). However, there’s only one default profile, which is the current profile of the default mode (CMYK in this example), so to speak. So it’s the default profile we’re looking for, not a current one, which might be counter-intuitive for our use case. I hope this makes clear why the code above looks for kColorSyncDeviceDefaultProfileID.

[pigoz]

Great stuff. I've fallen into the trap of thinking Current meant Default. Thanks for the new info.

[UliZappe]

I wrote:

it’s now hopefully as "ColorSync official" as it gets

Well, it still isn’t, as I just realized that kColorSyncDeviceDefaultProfileID is optional if only one factory profile exists (which will probably always be the case for displays).

Hmmm – after a good night’s sleep, and rereading the sparse documentation in the ColorSync header files, I wonder if I overreacted here. Strictly speaking, kColorSyncDeviceDefaultProfileID is marked optional when registering the factory profiles for a device (ColorSyncRegisterDevice), but is unconditionally listed within ColorSyncDeviceCopyDeviceInfo which we use.

It could well be that if you don’t specify kColorSyncDeviceDefaultProfileID when registering the factory defaults for a device because you only specify one factory profile, OS X will automatically generate it so that it’s always present in ColorSyncDeviceCopyDeviceInfo. If so, the last update to the code would have been unnecessary.

I will test tonight what happens if I register a device without specifying kColorSyncDeviceDefaultProfileID and then call ColorSyncDeviceCopyDeviceInfo. If kColorSyncDeviceDefaultProfileID exists, then we could go back to the slightly simpler variant (which unfortunately I overwrote …) before the last update.

Sorry for the confusion. The documentation is … well … suboptimal. ;-)

[UliZappe]

I tested it, and indeed kColorSyncDeviceDefaultProfileID is defined in ColorSyncDeviceCopyDeviceInfo even if it was not specified while registering the device with ColorSyncRegisterDevice. So the latest addition to the code was unnecessary.

So here is the previous version, which hopefully will be the final one. Again, sorry for the confusion.

static Boolean get_screen_profile(struct vo *vo, Boolean fullscreen, char *pathBuffer, size_t maxPathSize)
{
    CFURLRef profileURL;

    struct vo_cocoa_state *s = vo->cocoa;
    NSScreen *screen = fullscreen? s->fs_screen : s->current_screen;
    const char *screenstring = fullscreen? "full screen " : "";

    CGDirectDisplayID displayID = (CGDirectDisplayID)[[[screen deviceDescription] valueForKey:@"NSScreenNumber"] unsignedLongValue];

    CFUUIDRef displayUUID = CGDisplayCreateUUIDFromDisplayID(displayID); // for the current display, use kCGDirectMainDisplay instead of displayID
    if (CFGetTypeID(displayUUID) == CFNullGetTypeID()) {MP_ERR(s, "Cannot get %sdisplay UUID.\n", screenstring); return false;}

    CFDictionaryRef deviceInfo = ColorSyncDeviceCopyDeviceInfo(kColorSyncDisplayDeviceClass, displayUUID);
    CFRelease(displayUUID);
    if (!deviceInfo) {MP_ERR(s, "Cannot get %sdisplay info.\n", screenstring); return false;}

    CFDictionaryRef factoryInfo = CFDictionaryGetValue(deviceInfo, kColorSyncFactoryProfiles);
    if (!factoryInfo) {MP_ERR(s, "Cannot get %sdisplay factory settings.\n", screenstring); CFRelease(deviceInfo); return false;}
    CFStringRef defaultProfileID = CFDictionaryGetValue(factoryInfo, kColorSyncDeviceDefaultProfileID);
    if (!defaultProfileID) {MP_ERR(s, "Cannot get %sdisplay default profile ID.\n", screenstring); CFRelease(deviceInfo); return false;}

    CFDictionaryRef customProfileInfo = CFDictionaryGetValue(deviceInfo, kColorSyncCustomProfiles);
    if (customProfileInfo)
    {
        profileURL = CFDictionaryGetValue(customProfileInfo, defaultProfileID);
        // If profileURL is NULL, the profile URL could not be retrieved although a custom profile was specified.
        // This points to a configuration error, so we should not fall back to the factory profile, but return an error instead.
        if (!profileURL) {MP_ERR(s, "Cannot get %sdisplay profile URL.\n", screenstring); CFRelease(deviceInfo); return false;}
    }
    else    // No custom profile specified; try factory profile for the device
    {
        CFDictionaryRef factoryProfileInfo = CFDictionaryGetValue(factoryInfo, defaultProfileID);
        if (!factoryProfileInfo) {MP_ERR(s, "Cannot get %sdisplay profile info.\n", screenstring); CFRelease(deviceInfo); return false;}
        profileURL = CFDictionaryGetValue(factoryProfileInfo, kColorSyncDeviceProfileURL);
        if (!profileURL) {MP_ERR(s, "Cannot get %sdisplay factory profile URL.\n", screenstring); CFRelease(deviceInfo); return false;}
    }

    Boolean result = CFURLGetFileSystemRepresentation(profileURL, true, (UInt8*)pathBuffer, maxPathSize);
    CFRelease(deviceInfo);
    if (!result) MP_ERR(s, "Cannot get %sdisplay profile path.\n", screenstring);
    return result;
}

[UliZappe]

I changed some variable names in the above code to better reflect Apple’s naming conventions, and thus make it clearer to read.

[pigoz]

I mostly use chromium conventions in mpv (because I think it looks better when mixing C and ObjC), but don't worry, I will change that on my own.

[UliZappe]

Oh, feel free to use whatever naming conventions (i.e. something like my _myname vs. myName) fit well within mpv.

What I referred to in the above post was the actual names wrt/ factory, default and custom – my previous variable names partly added to the confusion I tried to clear up with the "Apple’s terminology" posting above.

[pigoz]

I integrated your code locally and it works quite well, the problem is mpv initializes the 3dlut way before VOCTRL_UPDATE_SCREENINFO is issued (which is responsible for getting the current_screen).

So before integrating this patch I we should make it possible to set the 3dlut much later as well set it multiple times (for each reconfig).

[UliZappe]

A possible workaround for the time being would be to use kCGDirectMainDisplay instead of using displayID, as mentioned in the comment in my final code above.

This would mean that for now, the :icc-profile=auto (or whatever you’ll call it) option won’t work for multiple display configurations, but if that’s clearly documented, it’s better than nothing, I think.

[pigoz]

Yes, that's better than nothing and good enough for the time being. Also it pretty much is enough for all of my use cases (hopefully this doesn't make me lazy).

[UliZappe]

I think that in this case, it should also be mentioned in the documentation that for now, mpv will stick to the OS X system display profile at mpv’s launch time and not change the profile if the OS X system display profile is changed later on.

[pigoz]

I actually had a brainfart before posting, the current window is already available, just not through a VO_CTRL call. A simple function call works though.

[UliZappe]

What exactly is the "current window" in mpv terminology? If (at the time of mpv’s launch) it’s the window where mpv’s menu bar is located, then this is still equivalent to simply using kCGDirectMainDisplay which is defined exactly this way.

If so, the more elaborate method with using displayID will only produce different (enhanced) results if the 3dlut can be reset dynamically when the mpv window is dragged to a different screen after mpv’s launch, or moved to another screen in full screen mode.

(Of course, you might still want to use the more elaborated method now, for better compatibility with later versions where the 3dlut can be reset during runtime.)

[pigoz]

current_screen is the screen where the window is initialized. That is the main display, or the reference to the screen passed in via --screen. fs_screen is the same but for the --fs-screen option.

I pushed a colorsync branch so that you can take a look before the merge.

[UliZappe]

There seems to be some bug in your rewrite of my memory management code (although I can’t spot it on first glimpse); if I specify icc-profile-auto, I get a segmentation fault from a faulty CFRelease() within cocoa_get_icc_profile_path().

Also, icc-approx-gamma is missing from this branch?

Besides from these issues, two points wrt/ documentation/error messages:

video/out/vo_opengl.c line 410: "selected backend failed to icc-profile auto selection\n" It seems there’s a verb missing after "to"? "perform"?

DOCS/man/en/vo.rst line 454…: I would suggest to be more specific:

Automatically select the ICC display profile currently specified by the display settings of the operating system. The profile setting refers to the screen where mpv is initialized and is not changed at runtime.

(As long as only OS X is supported, you could also shorten "specified by the display settings of the operating system" to "used by OS X".)

[haasn]

Also, icc-approx-gamma is missing from this branch?

Not sure which branch you're referring to but it was renamed to approx-gamma on master, together with the rework of the CMS shaders (and also the new shader generation code, although that variant hasn't been merged in yet).

[pigoz]

There seems to be some bug in your rewrite of my memory management code (although I can’t spot it on first glimpse); if I specify icc-profile-auto, I get a segmentation fault from a faulty CFRelease() within cocoa_get_icc_profile_path().

You are correct, the bug seems random though (I can't always reproduce it).

I'll incorporate your manpage suggestions and typo fix in the error message.

[UliZappe]

@haasn The colorsync branch @pigoz mentioned in the posting before. Anway, approx-gamma is the solution; I didn’t know that.

@pigoz The segmentation fault stems from the fact that you ignored Apple’s "Get rule" which says that you don’t own (and therefore must not release) objects you retrieve by a function call with "Get" in its name.

So at the end of cocoa_get_icc_profile_path() you must delete

CF_RELEASE(custom_profile_info);
CF_RELEASE(factory_info);

(These are in fact just pointers to somewhere within the device_info structure – which you got via a function with "Copy" in its name, and which therefore you own and must release.)

[pigoz]

@UliZappe woops, that's embarrassing. I almost never write CoreFoundation and I am not even sure why I added those when your implementation didn't have them. I pushed the result of all your feedbacks to the same branch (will squash to a single commit in the final merge).

[UliZappe]

You are correct, the bug seems random though (I can't always reproduce it).

Yep, but that’s typical for memory allocation errors, since the allocation within the RAM differs each time you run the program.

[UliZappe]

woops, that's embarrassing.

Never mind, we're all human. :-D

[ghost]

So, do we need runtime ICC profile switching in vo_opengl sometime around now, or what?

Never mind, we're all human. :-D

I'm not...

[UliZappe]

Hmm, I had never experienced any performance issues with mpv so far, but the build from the colorsync branch cannot handle a Full HD movie (with deinterlace enabled) on a full-fledged Mac Pro (audio sync issues, and obviously a much reduced frame rate). CPU load is < 50%, though (was more like 70% with deinterlacing before). Is there any performance degrading debug code in this build? (The last build I used before this was the master from February 26.)

[UliZappe]

@wm4

So, do we need runtime ICC profile switching in vo_opengl sometime around now, or what?

It works fine as is if you only use one display and if you never pause a movie, switch your OS X display profile and then continue the movie.

But of course, runtime ICC profile switching in vo_opengl would address these issues, which would certainly be nice. :-)

I'm not...

Thanks for warning me in advance! :-D

[UliZappe]

Is there any performance degrading debug code in this build?

Uh oh … It’s the same with the master branch. Seems that -vo opengl(-hq) and the latest OS X builds don’t play nice with each other. :-(

BTW, which of the -vo options is the default? Not specifying -vo at all works reasonably, but any -vo option currently seems to degrade performance beyond usable.

[ghost]

vo_opengl can now switch icc profiles at runtime. So all what's needed in addition to this branch is some sort of notification or reinitialization mechanism.

[UliZappe]

Great! I’ll try to come up with the notification code, which @pigoz can then incorporate.

Might take some time, as I have to get hold of a second monitor to test multi-display configurations.

Is there already a mechanism in mpv that notes when the mpv window is dragged from one screen to another?

[ghost]

No, since absolutely nothing has to care about this case (until now I guess).

There is a rudimentary event machanism, like e.g. VO_EVENT_RESIZE.

[UliZappe]

OK, then I’ll add that to the notification mechanism (will be OS X specific, of course).

Just to make sure I understand correctly: This means that vo->cocoa->current_screen is (currently) not guaranteed to be up to date at any time, but only at launch time?

[pigoz]

It's updated only when reconfig happens (playlist advances and video is changed) or when one goes fullscreen. Maybe current_screen is a bad name.

[UliZappe]

Luckily, it turns out that the notification code can be extremely simple, because there is one notification (NSWindowDidChangeScreenProfileNotification) that covers exactly our situation.

From Apple’s documentation:

The NSWindowDidChangeScreenProfileNotification is sent when a majority of the window is moved to a different screen (whose profile is also different from the previous screen) or when the ColorSync profile for the current screen changes.

So this is what has to be done:

_1. When a window is created, tell it to send this kind of notification. To do this, add one line in video/out/cocoa_common.m in static void create_window():

  ...
  [s->window setRestorable:NO];
  [s->window setContentView:s->view];
  [s->window setDisplaysWhenScreenProfileChanges:YES];

_2. In the Cocoa init code in osdep/macosx_application.m, register for this notification:

[[NSNotificationCenter defaultCenter] addObserver: self
                                      selector: @selector(new_screen_profile:)
                                      name: NSWindowDidChangeScreenProfileNotification
                                      object:nil];

and add the method that deals with this notification:

- (void) new_screen_profile:(NSNotification*)notification
    {
        NSWindow *window = [notification object]; // the window whose profile has changed
        NSScreen *screen = [window screen]; // the screen where most of the window is on
        mpv_function_to_update_display_profile_for_screen(screen);
    }

That’s it. (I think the switch to a new profile should be logged.)

As far as I know, mpv can open only one window at a time, but since the notification contains the window whose profile has changed, even a multiple window situation could be handled this way.

There is one situation that might not be handled here, and that is fullscreen. Does fullscreen mode get rid of the window (rather than use the window in fullscreen mode)? If so, this case would obviously have to be handled separately. (Basically, check the screen’s profile when fullscreen mode is entered or exited, and register for a NSScreenColorSpaceDidChangeNotification for the fullscreen screen while in fullscreen mode.)

[pigoz]

Nice, this seems simple. Yes, fullscreen uses the NSView API, which under the hood changes the view parent window to a fullscreen window created by the cocoa framework.