GNOME Bugzilla – Bug 164176
[jpegenc/dec] conversion bugs (wrong strides?)
Last modified: 2005-10-06 19:22:08 UTC
It seems that ffmpegcolorspace insists on rounding up widths and heights at times, which breaks other elements. gst-launch filesrc blocksize=1000000 location=aay.jpg ! jpegdec ! ffmpegcolorspace ! video/x-raw-rgb ! ffmpegcolorspace ! jpegenc ! filesink location=out.jpg This pipeline results in out.jpg having bad colors. The Y plane is correct (no rounding there, no wonder it is good!), but the U and V planes appear to have bad strides. Similar problems happen in other cases too, but I haven't been able to come up with a simple case to reproduce those, yet. Find attached the input and output pictures mentioned above.
Created attachment 36068 [details] The input picture used in the test pipeline
Created attachment 36069 [details] The output from the pipeline
Created attachment 36071 [details] [review] Patch fixing the problem This patch fixes the strides for the YUV colorspaces (by removing the rounding-up). I'm not 100% sure this is correct, but it does fix the problem for me, and I couldn't come up with a case where it does not work, and I tried quite a many colorspace conversions.
The patch is actually 100% wrong.
Any idea what element I should fix so the problem goes away, or a pointer to what might cause the problem?
It's probably in jpegdec/jpegenc.
Created attachment 37199 [details] [review] Proposed fix I got the same problem with an AVI RGB24 input (180x120). Without the patch, ffmpegcolorspace's output buffer isn't the size it should be (180*120*3/2). I think this patch is correct, but I'm not 100% sure.
Are you sure that this doesn't break other conversions? Did you test the same conversions with videotestsrc (which is known-good)?
Works For Me(tm) (after fixing the warning introduced by the patch) Tested with: gst-launch videotestsrc ! video/x-raw-rgb,width=180,height=120 ! ffmpegcolorspace ! ximagesink Produces the same output before and after the patch.
I tested I420 and YUY2 output. I'm not sure how I can test the other ones with videotestsrc (how do you pass the FOURCC?). Can you provide a sample pipeline?
*sigh* /me needs a think-first-type-after plugin. obviously I wasn't testing with x-raw-rgb, but with x-raw-yuv.. Since ffmpegcolorspace only supports I420, YUY2 and AYUV, there's no need to test with more, methinks (the elements won't link at all). AYUV can be tested with this: gst-launch videotestsrc ! video/x-raw-yuv,width=180,height=120,format=(fourcc)I420 ! alpha ! ffmpegcolorspace ! jpegenc ! filesink location=out.jpg (works fine here before and after the patch; and is an example on how to pass the fourcc)
escape the brackets (at least I have to...), so \(fourcc\)I420 and so on, otherwise Gergely is correct.
Tested with I420, Y42B and YUV9 (which are affected by the patch. YUY2 isn't changed) and it fixes the buffer size for all of them. They all display correctly in xvimagesink without the patch, but the buffer size they allocate is wrong. It should be w*h*3/2 for I420 and I think w*h*2 for Y42B and YUV9. To me, it also makes more sense to round the size of Y and then use that to calculate U and V. And thanks for the \(fourcc\) trick, it's very useful ;-)
I can't tell for sure, but if this is what X expects, then this is correct (since that's essentially how we've defined padding). Dave, Wim, agreed?
Created attachment 37306 [details] [review] stride fixes for jpegdec and jpegenc I think it makes more sense to first fix jpegdec and jpegenc to use the same rowstrides and buffer sizes that other GStreamer elements, most notably ffmpegcolorspace and videotestsrc, use for video/x-raw-yuv,format=I420. Attached patch does exactly that. gst-launch-0.8 filesrc blocksize=1000000 location=aay.jpg ! jpegdec ! ffmpegcolorspace ! video/x-raw-rgb ! ffmpegcolorspace ! jpegenc ! filesink location=out.jpg now produces a picture without the bad stripes. The picture produced still has some artifacts though, as if one of the color planes is missing or wrong lines get passed to the encoder. I haven't found the cause of that yet. Cheers -Tim
That's the chroma planes wrongly aligned, Sebastien's patch fixes that.
With sebastien's patch, rowstride for the U plane of an I420 yuv image that is 300 pixels wide becomes stride2 = DIV_ROUND_UP_X (300, 1) = 150 while many other plugins use the same stride as videotestsrc for I420, which is stride2 = (ROUND_UP_8 (300) /2) = 152; In other words, that patch will likely break other things (and it doesn't fix the issue for me either). Long live implicit row strides ... not. Cheers -Tim
As for the alignment issues, you'll see that the first line of each block *is* decoded correctly, only the next 15 lines per block have wrong alignment on the left border. The colors in the rest of the image are correct. So we may actually not have any stride issues, but just a missing setting in libjpeg or something. Now, if we look at line 488 of gstjpegdec.c, we do for (i = 0; i < height; i += r_v * DCTSIZE) ... DCTSIZE is 8 and r_v = v_scale = 2, so this is indeed the problem. We're just setting block offset, we're not setting stride.
Hm, the above is not actually correct (the second part). The first part is still true, though.
also, further analysis shows that the problem is in jpegdec; jpegenc is fine. Can be easily checked by using png. Gergeley, I thought you fixed PNG colorspaces? All my colours are wrong (endianness inversion). :).
Pff, I'm starting to suspect a libjpeg bug, I'll dig into libjpeg later.
I think we should start by fixing the stride in the colorspace, then fix the plugins that assume incorrect strides (maybe we should open another bug for that?). For I420, the stride of the U and V plane is half the size of the Y plane. So, for a width of 300, the stride of U and V will be 150. I don't know much about the other YUV formats, but for U and V they all seem to use either half the size or the full size of Y. Here are some references: http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnwmt/html/YUVFormats.asp http://www.fourcc.org/yuv.php#IYUV
Sebastien, we use other conventions. In this case, the bug is reproduceable w/o ffmpegcolorspace. Convert the image to PNG using GIMP and run: gst-launch filesrc blocksize=9999999 location=in.png ! pngdec ! ffmpegcolorspace ! jpegenc ! filesink location=out.jpg gst-launch filesrc blocksize=9999999 location=in.png ! pngdec ! pngenc ! filesink location=out.png gst-launch filesrc blocksize=9999999 location=in.png ! pngdec ! ffmpegcolorspace ! video/x-raw-yuv ! ffmpegcolorspace ! pngenc ! filesink location=out.png All are fine. Now run: gst-launch filesrc blocksize=9999999 location=in.jpg ! jpegdec ! jpegenc ! filesink location=out.jpg gst-launch filesrc blocksize=9999999 location=in.jpg ! jpegdec ! ffmpegcolorspace ! video/x-raw-rgb,bpp=32,depth=32 ! ffmpegcolorspace ! jpegenc ! filesink location=out.jpg gst-launch filesrc blocksize=9999999 location=in.jpg ! jpegdec ! ffmpegcolorspace ! pngenc ! filesink location=out.png All have random data as first few bytes for each line except the first per block. This automatically implies that the bug is in jpegdec and that ffmpegcolorspace is bugfree.
(marking other patch by Sebastien as rejected as per Tim's comments.)
After today's round of fixes jpegdec decodes this picture correctly and without artefacts for me. It seems there is still an ffmpegcolorspace issue though: 1) perfect picture: gst-launch-0.8 filesrc location=pic.jpg ! jpegdec ! xvimagesink 2) perfect picture: gst-launch-0.8 filesrc location=pic.jpg ! jpegdec ! ffmpegcolorspace ! ximagesink 3) messed up chroma: gst-launch-0.8 -v filesrc location=pic.jpg blocksize=1000000 ! jpegdec ! ffmpegcolorspace ! video/x-raw-rgb ! ffmpegcolorspace ! xvimagesink Happens both when the filtered link between the last colorspace and xvimagesink is forced to YUY2 or I420. In case 2) the RGB caps are video/x-raw-rgb, width=(int)300, height=(int)240, framerate=(double)1, bpp=(int)32, depth=(int)24, red_mask=(int)65280, green_mask=(int)16711680, blue_mask=(int)-16777216, endianness=(int)4321, pixel-aspect-ratio=(fraction)1/1 while in case 3) the RGB caps are video/x-raw-rgb, width=(int)300, height=(int)240, framerate=(double)1, bpp=(int)8, depth=(int)8, endianness=(int)1234 If I force the RGB caps from (1) between the two colourspaces in (3) everything is peachy as well. Cheers -Tim
Gah, so the last thing was just a silly fixation issue :) 2005-10-06 Tim-Philipp Muller <tim at centricular dot net> * gst/ffmpegcolorspace/gstffmpegcolorspace.c: (gst_ffmpegcsp_base_init), (gst_ffmpegcsp_class_init), (gst_ffmpegcsp_fixate_field_nearest_int), (gst_ffmpegcsp_fixate), (gst_ffmpegcsp_init): Add a fixate function to both pads to make sure we don't fixate to silly values like, say, 8bpp RGB video, if we can just as well do better. (#164176, #318136)