GNOME Bugzilla – Bug 532166
[ffmpegcolorspace] support NV12 format
Last modified: 2008-05-10 08:37:01 UTC
I have written a patch which adds support for the NV12 YUV format to ffmpegcolorspace in gst-plugins-base. The patch can be found here: http://pastebin.ca/1011642 I would appreciate if someone could review the patch and, if it is approved, commit it.
Maybe you could attach the patch?
Also patching the ffmpeg code is not a good idea IMHO, this should go to ffmpeg upstream better but imgconvert is deprecated there now. I'm planning to write a new colorspace conversion and scaling element based on the new swscale stuff from ffmpeg in the near future though... this should already support NV12/NV21
Created attachment 110614 [details] [review] The NV12 patch Here's the patch. I hope it's to some use despite Sebastians comments :-). I must admit that I'm not that familiar with gstreamer and all the plugins yet, so maybe he's got a point. When I wrote the NV12 patch it felt natural to put it in ffmpegcolorspace.
The patch looks fine in general and I guess we could simply apply it... the only reason why I'd be against that is, that the code in question is a copy of ffmpeg code and creating a larger difference on our side makes maintaining it harder. But as the code in question is deprecated upstream it should be fine I guess...
Ok, committed... I've added support for NV21 and support for conversions between NV* too. 2008-05-09 Sebastian Dröge <slomo@circular-chaos.org> Based on a patch by: Björn Benderius <bjoern dot benderius at axis dot com> * gst/ffmpegcolorspace/avcodec.h: * gst/ffmpegcolorspace/gstffmpegcodecmap.c: (gst_ffmpeg_pixfmt_to_caps), (gst_ffmpeg_caps_to_pixfmt), (gst_ffmpegcsp_avpicture_fill): * gst/ffmpegcolorspace/imgconvert.c: (nv12_to_nv21): * gst/ffmpegcolorspace/imgconvert_template.h: Add conversions from/to NV12 and NV21 and conversions between those two formats. Fixes bug #532166.
Nice, thank you!
Björn, what data did you use to check your patch btw?
I used NV12 YUV-images generated by an Axis Network Camera, I only tried a single resolution though (640x480). I tried converting to I420 and back again. I only did a visual inspection of the images before and after, I should have done a binary diff as well...
Well, it doesn't work properly with videotestsrc generating NV12 data (latest CVS) :) The stride seems to be wrong or something... could you take a look?
I will as soon as possible.
Created attachment 110675 [details] [review] stride nv12 This patch simplifies the calculation of the strides but don't solve the problem with the videotestsrc...
Well, the calculated stride is still the same ;) I'd like to keep the current code as it's the same for all other formats in imgconvert *shrug* :)
See bug #532454 for the bug...