[vlc-devel] [PATCH] macosx: fix inaccurate RGB color display
Thomas Guillem
thomas at gllm.fr
Thu Jul 27 04:51:33 CEST 2017
Great job!
You could split this commit into 3 commits and...
On Thu, Jul 27, 2017, at 01:17, Victorien Le Couviour--Tuffet wrote:
> Handling the primaries and color space results in inaccurate RGB color
> display.
> BT601, BT709 and BT2020 outputs get accurate without handling.
> Tested on several full red samples of different sizes (different
> primaries).
>
> Fixes #17383.
> ---
> remove conflict with 44ae5e72db59afc31996e35c11022f3dd5d2d153
>
> modules/video_output/caopengllayer.m | 70 ----------------------
> modules/video_output/macosx.m | 87
> ----------------------------
> modules/video_output/opengl/converter_cvpx.c | 6 +-
> 3 files changed, 3 insertions(+), 160 deletions(-)
>
> diff --git a/modules/video_output/caopengllayer.m
> b/modules/video_output/caopengllayer.m
> index 2a3cca5712..b1ddcc21a7 100644
> --- a/modules/video_output/caopengllayer.m
> +++ b/modules/video_output/caopengllayer.m
> @@ -43,15 +43,8 @@
>
> #include "opengl/vout_helper.h"
>
> -#define OSX_EL_CAPITAN_AND_HIGHER (NSAppKitVersionNumber >= 1404)
> #define OSX_SIERRA_AND_HIGHER (NSAppKitVersionNumber >= 1485)
>
> -#if MAC_OS_X_VERSION_MIN_ALLOWED <= MAC_OS_X_VERSION_10_11
> -const CFStringRef kCGColorSpaceDCIP3 = CFSTR("kCGColorSpaceDCIP3");
> -const CFStringRef kCGColorSpaceITUR_709 =
> CFSTR("kCGColorSpaceITUR_709");
> -const CFStringRef kCGColorSpaceITUR_2020 =
> CFSTR("kCGColorSpaceITUR_2020");
> -#endif
> -
> /*****************************************************************************
> * Vout interface
> *****************************************************************************/
> @@ -99,8 +92,6 @@ struct vout_display_sys_t {
> vout_window_t *embed;
> VLCCAOpenGLLayer *cgLayer;
>
> - CGColorSpaceRef cgColorSpace;
> -
> vlc_gl_t *gl;
> vout_display_opengl_t *vgl;
>
> @@ -215,64 +206,6 @@ static int Open (vlc_object_t *p_this)
> vd->display = PictureDisplay;
> vd->control = Control;
>
> - /* handle color space if supported by the OS */
> - if ([sys->cgLayer respondsToSelector:@selector(setColorspace:)])
> {
> -
> - /* support for BT.709 and BT.2020 color spaces was
> introduced with OS X 10.11
> - * on older OS versions, we can't show correct colors, so we
> fallback on linear RGB */
> - if (OSX_EL_CAPITAN_AND_HIGHER) {
> - switch (fmt.primaries) {
> - case COLOR_PRIMARIES_BT601_525:
> - case COLOR_PRIMARIES_BT601_625:
> - {
> - msg_Dbg(vd, "Using BT.601 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
> - break;
> - }
> - case COLOR_PRIMARIES_BT709:
> - {
> - msg_Dbg(vd, "Using BT.709 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_709);
> - break;
> - }
> - case COLOR_PRIMARIES_BT2020:
> - {
> - msg_Dbg(vd, "Using BT.2020 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020);
> - break;
> - }
> - case COLOR_PRIMARIES_DCI_P3:
> - {
> - msg_Dbg(vd, "Using DCI P3 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceDCIP3);
> - break;
> - }
> - default:
> - {
> - msg_Dbg(vd, "Guessing color space based on video
> dimensions (%ix%i)", fmt.i_visible_width, fmt.i_visible_height);
> - if (fmt.i_visible_height >= 2000 ||
> fmt.i_visible_width >= 3800) {
> - msg_Dbg(vd, "Using BT.2020 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020);
> - } else if (fmt.i_height > 576) {
> - msg_Dbg(vd, "Using BT.709 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_709);
> - } else {
> - msg_Dbg(vd, "SD content, using linear RGB
> color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
> - }
> - break;
> - }
> - }
> - } else {
> - msg_Dbg(vd, "OS does not support BT.709 or BT.2020 color
> spaces, output may vary");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceGenericRGBLinear);
> - }
> -
> - [sys->cgLayer setColorspace: sys->cgColorSpace];
> - } else {
> - msg_Dbg(vd, "OS does not support custom color spaces, output
> may be undefined");
> - }
> -
> if (OSX_SIERRA_AND_HIGHER) {
> /* request our screen's HDR mode (introduced in OS X 10.11,
> but correctly supported in 10.12 only) */
> if ([sys->cgLayer
> respondsToSelector:@selector(setWantsExtendedDynamicRangeContent:)])
> {
> @@ -333,9 +266,6 @@ static void Close (vlc_object_t *p_this)
> if ([sys->cgLayer glContext])
> CGLReleaseContext([sys->cgLayer glContext]);
>
> - if (sys->cgColorSpace != nil)
> - CGColorSpaceRelease(sys->cgColorSpace);
> -
> free(sys);
> }
>
> diff --git a/modules/video_output/macosx.m
> b/modules/video_output/macosx.m
> index 3fa35c592d..2770c44305 100644
> --- a/modules/video_output/macosx.m
> +++ b/modules/video_output/macosx.m
> @@ -49,14 +49,6 @@
> #include <vlc_dialog.h>
> #include "opengl/vout_helper.h"
>
> -#define OSX_EL_CAPITAN (NSAppKitVersionNumber >= 1404)
> -
> -#if MAC_OS_X_VERSION_MIN_ALLOWED <= MAC_OS_X_VERSION_10_11
> -const CFStringRef kCGColorSpaceDCIP3 = CFSTR("kCGColorSpaceDCIP3");
> -const CFStringRef kCGColorSpaceITUR_709 =
> CFSTR("kCGColorSpaceITUR_709");
> -const CFStringRef kCGColorSpaceITUR_2020 =
> CFSTR("kCGColorSpaceITUR_2020");
> -#endif
> -
> /**
> * Forward declarations
> */
> @@ -112,9 +104,6 @@ struct vout_display_sys_t
> VLCOpenGLVideoView *glView;
> id<VLCOpenGLVideoViewEmbedding> container;
>
> - CGColorSpaceRef cgColorSpace;
> - NSColorSpace *nsColorSpace;
> -
> vout_window_t *embed;
> vlc_gl_t *gl;
> vout_display_opengl_t *vgl;
> @@ -176,57 +165,6 @@ static int Open (vlc_object_t *this)
> * main thread, after we are done using it. */
> sys->container = [container retain];
>
> - /* support for BT.709 and BT.2020 color spaces was introduced
> with OS X 10.11
> - * on older OS versions, we can't show correct colors, so we
> fallback on linear RGB */
> - if (OSX_EL_CAPITAN) {
> - switch (vd->fmt.primaries) {
> - case COLOR_PRIMARIES_BT601_525:
> - case COLOR_PRIMARIES_BT601_625:
> - {
> - msg_Dbg(vd, "Using BT.601 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
> - break;
> - }
> - case COLOR_PRIMARIES_BT709:
> - {
> - msg_Dbg(vd, "Using BT.709 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_709);
> - break;
> - }
> - case COLOR_PRIMARIES_BT2020:
> - {
> - msg_Dbg(vd, "Using BT.2020 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020);
> - break;
> - }
> - case COLOR_PRIMARIES_DCI_P3:
> - {
> - msg_Dbg(vd, "Using DCI P3 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceDCIP3);
> - break;
> - }
> - default:
> - {
> - msg_Dbg(vd, "Guessing color space based on video
> dimensions (%ix%i)", vd->fmt.i_visible_width, vd->fmt.i_visible_height);
> - if (vd->fmt.i_visible_height >= 2000 ||
> vd->fmt.i_visible_width >= 3800) {
> - msg_Dbg(vd, "Using BT.2020 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_2020);
> - } else if (vd->fmt.i_height > 576) {
> - msg_Dbg(vd, "Using BT.709 color space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceITUR_709);
> - } else {
> - msg_Dbg(vd, "SD content, using linear RGB color
> space");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
> - }
> - break;
> - }
> - }
> - } else {
> - msg_Dbg(vd, "OS does not support BT.709 or BT.2020 color
> spaces, output may vary");
> - sys->cgColorSpace =
> CGColorSpaceCreateWithName(kCGColorSpaceSRGB);
> - }
> - sys->nsColorSpace = [[NSColorSpace alloc]
> initWithCGColorSpace:sys->cgColorSpace];
> -
> /* Get our main view*/
> [VLCOpenGLVideoView
> performSelectorOnMainThread:@selector(getNewView:)
> withObject:[NSValue
> valueWithPointer:&sys->glView]
> @@ -360,12 +298,6 @@ void Close (vlc_object_t *this)
>
> [sys->glView release];
>
> - if (sys->cgColorSpace != nil)
> - CGColorSpaceRelease(sys->cgColorSpace);
> -
> - if (sys->nsColorSpace != nil)
> - [sys->nsColorSpace release];
> -
> if (sys->embed)
> vout_display_DeleteWindow (vd, sys->embed);
> free (sys);
> @@ -904,23 +836,4 @@ static void OpenglSwap (vlc_gl_t *gl)
> return YES;
> }
>
> -- (void)viewWillMoveToWindow:(nullable NSWindow *)newWindow
> -{
> - [super viewWillMoveToWindow:newWindow];
> -
> - if (newWindow == nil)
> - return;
> -
> - @synchronized (self) {
> - @try {
> - if (vd) [newWindow setColorSpace:vd->sys->nsColorSpace];
> - }
> - @catch (NSException *exception) {
> - msg_Warn(vd, "Setting the window color space failed due to
> an Obj-C exception (%s, %s", [exception.name UTF8String],
> [exception.reason UTF8String]);
> - }
> -
> - }
> -
> -}
> -
> @end
> diff --git a/modules/video_output/opengl/converter_cvpx.c
> b/modules/video_output/opengl/converter_cvpx.c
> index 7864d3f25b..e388551b2e 100644
> --- a/modules/video_output/opengl/converter_cvpx.c
> +++ b/modules/video_output/opengl/converter_cvpx.c
> @@ -196,7 +196,7 @@ opengl_tex_converter_cvpx_init(opengl_tex_converter_t
> *tc)
> case VLC_CODEC_CVPX_UYVY:
> fragment_shader =
> opengl_fragment_shader_init(tc, tex_target,
> VLC_CODEC_UYVY,
> - tc->fmt.space);
> + COLOR_SPACE_UNDEF);
Explain in a comment that this is handled by videotoolbox.
> tc->texs[0].internal = GL_RGB;
> tc->texs[0].format = GL_RGB_422_APPLE;
> tc->texs[0].type = GL_UNSIGNED_SHORT_8_8_APPLE;
> @@ -205,13 +205,13 @@
> opengl_tex_converter_cvpx_init(opengl_tex_converter_t *tc)
> {
> fragment_shader =
> opengl_fragment_shader_init(tc, tex_target,
> VLC_CODEC_NV12,
> - tc->fmt.space);
> + COLOR_SPACE_UNDEF);
> break;
> }
> case VLC_CODEC_CVPX_I420:
> fragment_shader =
> opengl_fragment_shader_init(tc, tex_target,
> VLC_CODEC_I420,
> - tc->fmt.space);
> + COLOR_SPACE_UNDEF);
> break;
> case VLC_CODEC_CVPX_BGRA:
> fragment_shader =
> --
> 2.13.1
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list