GNOME Bugzilla – Bug 790023
matroskademux: Add parsing Colour element
Last modified: 2017-11-08 09:33:02 UTC
... and forward colorimetry to downstream. The Colour element describes various color information (similar to 'colr' box in isobmff). Note that, due to the comparatively limited syntax for color information in vpx codecs, the color information in mkv/wemb container level should be used for sophisticated color handling (e.g., HDR video).
Created attachment 363155 [details] [review] matroskademux: Add parsing Colour element
Review of attachment 363155 [details] [review]: Is there also something for stereoscopic video information (left-right, top-bottom, etc)? Looks generally good in any case ::: gst/matroska/matroska-demux.c @@ +443,3 @@ + break; + default: + break; You probably want to print something here for all unsupported ones. GST_FIXME_OBJECT() or so
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 363155 [details] [review] [review]: > > Is there also something for stereoscopic video information (left-right, > top-bottom, etc)? Looks generally good in any case The Colour element has no stereoscopic video info, but has more color specific (chroma subsampling, HDR, etd) info which I didn't add in the patch (because the missing parts are not supported in gstreamer AFAIK). Actually, the missing feature in gstreamer is color mastering information which is essential for HDR support. Please refer to https://github.com/FFmpeg/FFmpeg/blob/master/libavutil/mastering_display_metadata.h
(In reply to Sebastian Dröge (slomo) from comment #2) > Review of attachment 363155 [details] [review] [review]: One more comment is, for stereoscopic info, we need to parse StereoMode element, which has been implemented already https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/matroska/matroska-demux.c#n737
Created attachment 363187 [details] [review] matroskademux: Add parsing Colour element Add some informal debug log
Review of attachment 363187 [details] [review]: ::: gst/matroska/matroska-demux.c @@ +465,3 @@ + break; + default: + break; Also print here @@ +5742,3 @@ + gchar *colorimetry = + gst_video_colorimetry_to_string (&videocontext->colorimetry); + gst_caps_set_simple (caps, "colorimetry", G_TYPE_STRING, colorimetry, And it would make sense to also print the final string here that is actually set in the caps
Created attachment 363190 [details] [review] matroskademux: Add parsing Colour element
(In reply to Sebastian Dröge (slomo) from comment #6) > Review of attachment 363187 [details] [review] [review]: > > ::: gst/matroska/matroska-demux.c > @@ +465,3 @@ > + break; > + default: > + break; > > Also print here > > @@ +5742,3 @@ > + gchar *colorimetry = > + gst_video_colorimetry_to_string (&videocontext->colorimetry); > + gst_caps_set_simple (caps, "colorimetry", G_TYPE_STRING, colorimetry, > > And it would make sense to also print the final string here that is actually > set in the caps All done :)
Attachment 363190 [details] pushed as 5dd39d8 - matroskademux: Add parsing Colour element
For HDR specific things, chroma-site (is there only subsampling, no chroma-site?), etc. please open new bugs :) If there is API missing in GStreamer for HDR, please also feel free to propose API to be added (to gst-plugins-base's libgstvideo in this case).
(In reply to Sebastian Dröge (slomo) from comment #10) > For HDR specific things, chroma-site (is there only subsampling, no > chroma-site?), etc. please open new bugs :) > > If there is API missing in GStreamer for HDR, please also feel free to > propose API to be added (to gst-plugins-base's libgstvideo in this case). I'll report new bug for HDR after study more. Briefly, example of missing HDR related meta data is - Mastering Display Metadata – SMPTE ST 2086 (min/max luminance, color volume) - MaxCLL – Maximum Content Light Level ....