[Mesa-dev] [PATCH v2] egl: implement EXT_surface_SMPTE2086_metadata and EXT_surface_CTA861_3_metadata
Eric Engestrom
eric.engestrom at intel.com
Tue Aug 14 14:41:45 UTC 2018
On Tuesday, 2018-08-14 14:49:28 +0300, Tapani Pälli wrote:
>
>
> On 08/14/2018 01:54 PM, Eric Engestrom wrote:
> > On Tuesday, 2018-08-14 11:58:52 +0300, Tapani Pälli wrote:
> > > Patch implements common bits for EXT_surface_SMPTE2086_metadata
> > > and EXT_surface_CTA861_3_metadata extensions by adding new required
> > > attributes and eglQuerySurface + eglSurfaceAttrib changes.
> > >
> > > Currently none of the drivers are utilizing this data but this patch
> > > is enabler in getting there.
> > >
> > > v2: don't enable extension globally, should be only enabled by
> > > EGL drivers that can transfer metadata to the window system
> > > (Jason)
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> > > ---
> > > src/egl/main/eglapi.c | 2 +
> > > src/egl/main/egldisplay.h | 2 +
> > > src/egl/main/eglsurface.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++
> > > src/egl/main/eglsurface.h | 19 ++++++
> > > 4 files changed, 192 insertions(+)
> > >
> > > diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
> > > index 19fae12f5b7..0fd3b3f9023 100644
> > > --- a/src/egl/main/eglapi.c
> > > +++ b/src/egl/main/eglapi.c
> > > @@ -488,6 +488,8 @@ _eglCreateExtensionsString(_EGLDisplay *dpy)
> > > _EGL_CHECK_EXTENSION(EXT_create_context_robustness);
> > > _EGL_CHECK_EXTENSION(EXT_image_dma_buf_import);
> > > _EGL_CHECK_EXTENSION(EXT_image_dma_buf_import_modifiers);
> > > + _EGL_CHECK_EXTENSION(EXT_surface_CTA861_3_metadata);
> > > + _EGL_CHECK_EXTENSION(EXT_surface_SMPTE2086_metadata);
> > > _EGL_CHECK_EXTENSION(EXT_swap_buffers_with_damage);
> > > _EGL_CHECK_EXTENSION(IMG_context_priority);
> > > diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
> > > index 4a7f248e34c..50176bde887 100644
> > > --- a/src/egl/main/egldisplay.h
> > > +++ b/src/egl/main/egldisplay.h
> > > @@ -105,6 +105,8 @@ struct _egl_extensions
> > > EGLBoolean EXT_image_dma_buf_import;
> > > EGLBoolean EXT_image_dma_buf_import_modifiers;
> > > EGLBoolean EXT_pixel_format_float;
> > > + EGLBoolean EXT_surface_CTA861_3_metadata;
> > > + EGLBoolean EXT_surface_SMPTE2086_metadata;
> > > EGLBoolean EXT_swap_buffers_with_damage;
> > > unsigned int IMG_context_priority;
> > > diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> > > index 926b7ab569a..95677105e58 100644
> > > --- a/src/egl/main/eglsurface.c
> > > +++ b/src/egl/main/eglsurface.c
> > > @@ -86,6 +86,90 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
> > > break;
> > > surf->GLColorspace = val;
> > > break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_RX_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.display_primary_r.x = val;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_RY_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.display_primary_r.y = val;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_GX_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.display_primary_g.x = val;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_GY_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.display_primary_g.y = val;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_BX_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.display_primary_b.x = val;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_BY_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.display_primary_b.y = val;
> > > + break;
> > > + case EGL_SMPTE2086_WHITE_POINT_X_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.white_point.x = val;
> > > + break;
> > > + case EGL_SMPTE2086_WHITE_POINT_Y_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.white_point.y = val;
> > > + break;
> > > + case EGL_SMPTE2086_MAX_LUMINANCE_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.max_luminance = val;
> > > + break;
> > > + case EGL_SMPTE2086_MIN_LUMINANCE_EXT:
> > > + if (!dpy->Extensions.EXT_surface_SMPTE2086_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.min_luminance = val;
> > > + break;
> > > + case EGL_CTA861_3_MAX_CONTENT_LIGHT_LEVEL_EXT:
> > > + if (!dpy->Extensions.EXT_surface_CTA861_3_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.max_cll = val;
> > > + break;
> > > + case EGL_CTA861_3_MAX_FRAME_AVERAGE_LEVEL_EXT:
> > > + if (!dpy->Extensions.EXT_surface_CTA861_3_metadata) {
> > > + err = EGL_BAD_ATTRIBUTE;
> > > + break;
> > > + }
> > > + surf->HdrMetadata.max_fall = val;
> > > + break;
> > > case EGL_VG_COLORSPACE:
> > > switch (val) {
> > > case EGL_VG_COLORSPACE_sRGB:
> > > @@ -312,6 +396,19 @@ _eglInitSurface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> > > /* the default swap interval is 1 */
> > > surf->SwapInterval = 1;
> > > + surf->HdrMetadata.display_primary_r.x = EGL_DONT_CARE;
> > > + surf->HdrMetadata.display_primary_r.y = EGL_DONT_CARE;
> > > + surf->HdrMetadata.display_primary_g.x = EGL_DONT_CARE;
> > > + surf->HdrMetadata.display_primary_g.y = EGL_DONT_CARE;
> > > + surf->HdrMetadata.display_primary_b.x = EGL_DONT_CARE;
> > > + surf->HdrMetadata.display_primary_b.y = EGL_DONT_CARE;
> > > + surf->HdrMetadata.white_point.x = EGL_DONT_CARE;
> > > + surf->HdrMetadata.white_point.y = EGL_DONT_CARE;
> > > + surf->HdrMetadata.max_luminance = EGL_DONT_CARE;
> > > + surf->HdrMetadata.min_luminance = EGL_DONT_CARE;
> > > + surf->HdrMetadata.max_cll = EGL_DONT_CARE;
> > > + surf->HdrMetadata.max_fall = EGL_DONT_CARE;
> > > +
> > > err = _eglParseSurfaceAttribList(surf, attrib_list);
> > > if (err != EGL_SUCCESS)
> > > return _eglError(err, func);
> > > @@ -438,6 +535,42 @@ _eglQuerySurface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surface,
> > > *value = result;
> > > surface->BufferAgeRead = EGL_TRUE;
> > > break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_RX_EXT:
> > > + *value = surface->HdrMetadata.display_primary_r.x;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_RY_EXT:
> > > + *value = surface->HdrMetadata.display_primary_r.y;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_GX_EXT:
> > > + *value = surface->HdrMetadata.display_primary_g.x;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_GY_EXT:
> > > + *value = surface->HdrMetadata.display_primary_g.y;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_BX_EXT:
> > > + *value = surface->HdrMetadata.display_primary_b.x;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_BY_EXT:
> > > + *value = surface->HdrMetadata.display_primary_b.y;
> > > + break;
> > > + case EGL_SMPTE2086_WHITE_POINT_X_EXT:
> > > + *value = surface->HdrMetadata.white_point.x;
> > > + break;
> > > + case EGL_SMPTE2086_WHITE_POINT_Y_EXT:
> > > + *value = surface->HdrMetadata.white_point.y;
> > > + break;
> > > + case EGL_SMPTE2086_MAX_LUMINANCE_EXT:
> > > + *value = surface->HdrMetadata.max_luminance;
> > > + break;
> > > + case EGL_SMPTE2086_MIN_LUMINANCE_EXT:
> > > + *value = surface->HdrMetadata.min_luminance;
> > > + break;
> > > + case EGL_CTA861_3_MAX_CONTENT_LIGHT_LEVEL_EXT:
> > > + *value = surface->HdrMetadata.max_cll;
> > > + break;
> > > + case EGL_CTA861_3_MAX_FRAME_AVERAGE_LEVEL_EXT:
> > > + *value = surface->HdrMetadata.max_fall;
> > > + break;
> > > default:
> > > return _eglError(EGL_BAD_ATTRIBUTE, "eglQuerySurface");
> > > }
> > > @@ -527,6 +660,42 @@ _eglSurfaceAttrib(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surface,
> > > break;
> > > surface->SwapBehavior = value;
> > > break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_RX_EXT:
> > > + surface->HdrMetadata.display_primary_r.x = value;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_RY_EXT:
> > > + surface->HdrMetadata.display_primary_r.y = value;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_GX_EXT:
> > > + surface->HdrMetadata.display_primary_g.x = value;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_GY_EXT:
> > > + surface->HdrMetadata.display_primary_g.y = value;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_BX_EXT:
> > > + surface->HdrMetadata.display_primary_b.x = value;
> > > + break;
> > > + case EGL_SMPTE2086_DISPLAY_PRIMARY_BY_EXT:
> > > + surface->HdrMetadata.display_primary_b.y = value;
> > > + break;
> > > + case EGL_SMPTE2086_WHITE_POINT_X_EXT:
> > > + surface->HdrMetadata.white_point.x = value;
> > > + break;
> > > + case EGL_SMPTE2086_WHITE_POINT_Y_EXT:
> > > + surface->HdrMetadata.white_point.y = value;
> > > + break;
> > > + case EGL_SMPTE2086_MAX_LUMINANCE_EXT:
> > > + surface->HdrMetadata.max_luminance = value;
> > > + break;
> > > + case EGL_SMPTE2086_MIN_LUMINANCE_EXT:
> > > + surface->HdrMetadata.min_luminance = value;
> > > + break;
> > > + case EGL_CTA861_3_MAX_CONTENT_LIGHT_LEVEL_EXT:
> > > + surface->HdrMetadata.max_cll = value;
> > > + break;
> > > + case EGL_CTA861_3_MAX_FRAME_AVERAGE_LEVEL_EXT:
> > > + surface->HdrMetadata.max_fall = value;
> > > + break;
> > > default:
> > > err = EGL_BAD_ATTRIBUTE;
> > > break;
> > > diff --git a/src/egl/main/eglsurface.h b/src/egl/main/eglsurface.h
> > > index 5d69bf487cf..2189c0b1d09 100644
> > > --- a/src/egl/main/eglsurface.h
> > > +++ b/src/egl/main/eglsurface.h
> > > @@ -41,6 +41,23 @@
> > > extern "C" {
> > > #endif
> > > +struct _egl_xyy
> >
> > "xyy" -> "xy" ?
> >
> > > +{
> > > + uint16_t x, y;
> >
> > Doesn't the compiler complain here? Shouldn't these (and below) be
> > EGLint?
>
> This is valid declaration but agreed, not really according to coding style
> so will move y to it's own line.
Oh no, I didn't mean that, I was referring to the range loss between the
function input and the variable here storing it, which I expected the
compiler would warn about.
>
> I bet uint16_t was chosen because that would meet the required range of
> values (required by smpte2086 and cta861.3 specs) but I agree that EGLint is
> proper here, any conversions can be done later. Will change.
I didn't see anything about type size in the specs (although I didn't
read them that thoroughly), which is why I was surprised to not see the
usual EGLint here.
>
>
> > Other than that, it looks all good to me:
> > Reviewed-by: Eric Engestrom <eric.engestrom at intel.com>
> >
> > > +};
> > > +
> > > +struct _egl_hdr_metadata
> > > +{
> > > + struct _egl_xyy display_primary_r;
> > > + struct _egl_xyy display_primary_g;
> > > + struct _egl_xyy display_primary_b;
> > > + struct _egl_xyy white_point;
> > > + uint16_t max_luminance;
> > > + uint16_t min_luminance;
> > > + uint16_t max_cll;
> > > + uint16_t max_fall;
> > > +};
> > > +
> > > /**
> > > * "Base" class for device driver surfaces.
> > > */
> > > @@ -150,6 +167,8 @@ struct _egl_surface
> > > EGLBoolean BoundToTexture;
> > > EGLBoolean PostSubBufferSupportedNV;
> > > +
> > > + struct _egl_hdr_metadata HdrMetadata;
> > > };
> > > --
> > > 2.14.4
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list