GNOME Bugzilla – Bug 780100
video: add support for planar GBR and GBRA formats with 12 bits per channel, GBRA with 8, 10 and 12 bits per channel, 4:2:0/4:2:2/4:4:4 YUV with 12 bits per channel
Last modified: 2018-11-03 11:56:12 UTC
Created attachment 348013 [details] [review] Add GBR_12LE/BE and GBRA_12LE/BE pixel format mapping for CineForm decoder Add support for BGR and BGRA with 12 bits per channel so we can use the CineForm decoder from ffmpeg. This is a work in progress!
Created attachment 348014 [details] [review] Add GBR_12LE/BE and GBRA_12LE/BE pixel format
You say it's WIP. Do you need any help with any parts of it? What parts are missing? From a (very fast) look over the patch, it looks like it does all the right things but I'll take a closer look later. While we're at it, we might also want to add GBRA_10LE/BE formats though, for consistency :)
Created attachment 348015 [details] [review] Add ARGB64_LE/BE pixel format I also have this patch to add ARGB64_LE and ARGB64_BE (or b64a). I was not able to test those patches today because my GST version seems broken, but at least they build. Like the GBR12 patch, it's mostly to know if I'm on the right path. I'll be available tomorrow on IRC to talk about this. Thanks.
(In reply to Sebastian Dröge (slomo) from comment #2) > You say it's WIP. Do you need any help with any parts of it? What parts are > missing? > > From a (very fast) look over the patch, it looks like it does all the right > things but I'll take a closer look later. While we're at it, we might also > want to add GBRA_10LE/BE formats though, for consistency :) I was only able to build the patch on today's git, not test it, that's why the WIP. I think I have an unrelated problem with gst-libav (all my attempts are ending up with "libav :0:: get_buffer() failed" even when not trying CineForm videos. If I use my own CineForm plugin (or any other plugins from good) I end up stuck on "Redistribute latency...". Regarding the first patch, it's mostly copy/past but I'm not sure about the GST_VIDEO_PACK_FLAG_TRUNCATE_RANGE section. I'll add GBRA_10LE/BE tomorrow. For the second patch, I'm not sure about the "swap" approach alltogether.
Created attachment 348073 [details] [review] Add GBRA_10LE/BE, GBR_12LE/BE, GBRA_12LE/BE pixel formats This patch adds GBRA_10LE/BE and has a small fix. I was able to backport everything to 1.10.4 to test, but while I can indeed play CineForm YUV422 I still cannot play RGB/A videos, so something may still be missing. ERROR: from element /GstPipeline:pipeline0/GstDecodeBin:decodebin0/avdec_cfhd:avdec_cfhd0: Unable to allocate memory Additional debug info: gstavviddec.c(857): gst_ffmpegviddec_get_buffer2 (): /GstPipeline:pipeline0/GstDecodeBin:decodebin0/avdec_cfhd:avdec_cfhd0: The downstream pool failed to allocated buffer. ERROR: pipeline doesn't want to preroll.
Can you attach a debug log? The patch looks complete to me. Should probably also add equivalents of the AV_PIX_FMT_YUV4XXP12LE/BE formats from ffmpeg, but not relevant here. I'll just do that later :) Did you recompile videoconvert, etc after your patch?
Created attachment 348089 [details] [review] video: Add GBRA_10LE/BE, GBR_12LE/BE, GBRA_12LE/BE pixel formats
Comment on attachment 348073 [details] [review] Add GBRA_10LE/BE, GBR_12LE/BE, GBRA_12LE/BE pixel formats This did not compile cleanly, I fixed that now. Please base any future work on this patch on the one I just attached with those fixes :)
Review of attachment 348073 [details] [review]: ::: gst-libs/gst/video/video-format.h @@ +103,3 @@ + * @GST_VIDEO_FORMAT_GBR_12LE: planar 4:4:4 RGB, 12 bits per channel + * @GST_VIDEO_FORMAT_GBRA_12BE: planar 4:4:4:4 ARGB, 12 bits per channel + * @GST_VIDEO_FORMAT_GBRA_12LE: planar 4:4:4:4 ARGB, 12 bits per channel I didn't realize this was planar RGB. I'm thinking out loud here, but could we create a notation to differentiate packed RGB from planar RGB. When I read the format first, I though it was going to be packed, with 64bit pixels, and each component being padded. Clearly this is not the case. Other then that, I'm surprised of the component / plane order, but I need to document myself. ::: gst-libs/gst/video/video-info.c @@ +918,3 @@ + case GST_VIDEO_FORMAT_GBRA_12LE: + case GST_VIDEO_FORMAT_GBRA_12BE: + info->stride[0] = GST_ROUND_UP_4 (width * 2); Your pack/unpack function only require 16bit alignment. Do you really need to round up here ?
Created attachment 348090 [details] cineform playback error log Here is the cineform playback error log
(In reply to Nicolas Dufresne (stormer) from comment #9) > ::: gst-libs/gst/video/video-format.h > @@ +103,3 @@ > + * @GST_VIDEO_FORMAT_GBR_12LE: planar 4:4:4 RGB, 12 bits per channel > + * @GST_VIDEO_FORMAT_GBRA_12BE: planar 4:4:4:4 ARGB, 12 bits per channel > + * @GST_VIDEO_FORMAT_GBRA_12LE: planar 4:4:4:4 ARGB, 12 bits per channel > > I didn't realize this was planar RGB. I'm thinking out loud here, but could > we create a notation to differentiate packed RGB from planar RGB. When I > read the format first, I though it was going to be packed, with 64bit > pixels, and each component being padded. Clearly this is not the case. > > Other then that, I'm surprised of the component / plane order, but I need to > document myself. We already support GBR in various versions, nothing new here :)
What's new is that I notice that our naming does not help differentiating planar and packed/interleaved RGB. What happens now if someone wants to add an interleave variant of these ?
(In reply to emeric.grange from comment #10) > Created attachment 348090 [details] > cineform playback error log > > Here is the cineform playback error log I couldn't find the problem while shortly looking, but there's definitely something wrong with the patch. It also crashes in various ways with very simple pipelines: gst-launch-1.0 videotestsrc ! "video/x-raw,format=GBR_12BE" ! videoconvert ! "video/x-raw,format=RGBA" ! glimagesink gst-launch-1.0 videotestsrc ! "video/x-raw,format=GBRA_10BE" ! videoconvert ! "video/x-raw,format=RGBA" ! glimagesink Same with the LE variants. GBR_10BE/LE works fine though (the old one). I'd look into these problems first, that might also solve the gst-libav ones.
Time to use valgrind, and to review the offsets.
Created attachment 348092 [details] [review] video: Add GBRA_10LE/BE, GBR_12LE/BE, GBRA_12LE/BE pixel formats
With this updated the GBR variants work, and also the GBR12 CFHD video. GBRA gets colors wrong.
Created attachment 348093 [details] [review] video: Add GBRA_10LE/BE, GBR_12LE/BE, GBRA_12LE/BE pixel formats
And with this also the GBRA variants and the corresponding CFHD video are working.
Created attachment 348097 [details] [review] video: Add I420/I422/Y444_10LE/BE and GBRA video formats
Created attachment 348098 [details] [review] video: Add I420/I422/Y444_12LE/BE and GBRA video formats
Created attachment 348099 [details] [review] avcodecmap: Add mappings for I420/I422/Y444_12LE/BE and GBRA
And for completeness a few other missing formats. GBRA is broken currently
Created attachment 348100 [details] [review] video: Add I420/I422/Y444_12LE/BE and GBRA video formats
Created attachment 348101 [details] [review] video-format: Add Since markers to all new formats from 1.2
Attachment 348093 [details] pushed as 2fcab9e - video: Add GBRA_10LE/BE, GBR_12LE/BE, GBRA_12LE/BE pixel formats Attachment 348100 [details] pushed as 77f802f - video: Add I420/I422/Y444_12LE/BE and GBRA video formats Attachment 348101 [details] pushed as 95ddfde - video-format: Add Since markers to all new formats from 1.2
Attachment 348099 [details] pushed as fecf973 - avcodecmap: Add mappings for I420/I422/Y444_12LE/BE and GBRA
Some further fixups: commit ce9bac465a4b088be42efba8e8d4478214f80901 Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Mar 17 13:43:04 2017 +0200 video-format: Shift correctly when packing I420_12BE commit eda8bd4de122b5ff5605462f0b6c3f46c49ae316 Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Mar 17 13:14:58 2017 +0200 video-format: Order all formats in GST_VIDEO_FORMATS_ALL like in the enum And remove duplicated entries.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/349.