GNOME Bugzilla – Bug 627413
jifmux causes broken jpeg images at least with some rgb pixel format
Last modified: 2010-08-21 19:10:28 UTC
Sorry, I don't have the time right now to do further testing. I was trying a Cheese patch to save images with appsrc ! jpegenc ! jifmux ! filesink and found out that jifmux causes broken colors with some rgb cap. Will try to better nail down broken formats tomorrow, here's a pipeline that reproduces the issue here: gst-launch-0.10 -e videotestsrc num-buffe ! "video/x-raw-rgb, bpp=(int)24, depth=(int)24, endianness=(int)4321, red_mask=(int)16711680, green_mask=(int)65280, blue_mask=(int)255, width=(int)640, height=(int)480, framerate=(fraction)30/1" ! jpegenc ! jifmux ! filesink location=broken-jifmux-test.jpg The same without jifmux outputs a perfectly fine image while this gives the broken colors you can see in the attached image
Created attachment 168312 [details] broken jifmux output
Just reproduced this. Assigning to me.
It only happens with RGB and if xmp tags are written.
Created attachment 168330 [details] [review] jpegdec: Prevent crash when reading image with problems This is not the fix for jifmux, but these broken images would make jpegdec crash if they were used with: filesrc ! jpegparse ! jpegdec ! ... This patch fixes it.
I have very little knowledge of jpeg so forgive me if I'm saying something silly. It seems that jpegenc gives RGB data to libjpeg without any conversion and just sets the colorspace to RGB. After some googling, as far as I can tell, it seems JFIF mandates image data to be YCbCr (CCIR 601). So jifmux just adds its headers around the rgb compressed data and when you give the file to a decoder it expects it to be YCbCr and gives those wrong colors.
After some more googling (see e.g. http://freedesktop.org/wiki/Software/libjpeg, Special Markers section), it seems that the key to get a correct colorspace is the presence of APP0 and APP14 markers. APP14 is written by libjpeg when colorspace is RGB. APP0 when colorspace is YCbCr or grayscale. With jifmux APP0 is always written no matter what the colorspace is (I guess it's needed by JFIF format). APP14 is preserved too but APP0 seems to have precedence at least in the decoder used by eog (I guess libjpeg). Even putting APP14 first doesn't change anything, while commenting out the line that adds APP0 from gstjifmux.c gives correct colors.
Yes, I've been playing with these markers yesterday and it seems you're right: "The color space to be used is YCbCr as defined by CCIR 601 (256 levels). The RGB components calculated by linear conversion from YCbCr shall not be gamma corrected (gamma = 1.0). If only one component is used, that component shall be Y." From the JFIF spec. We need something on image/jpeg caps to detect and prevent that.
Created attachment 168409 [details] [review] jifmux: Avoid recombining RGB jpegs JFIF only allows YUV as colorspace, when we receive an RGB jpeg, we should just push it forward without adding the JFIF marker. Fixes #627413 This patch fixes the problem, but it doesn't add the JFIF marker to the output buffer. I left it like that because it is the default behavior when parsing the image fails.
Setting to blocker as it crashes jpegdec.
commit 407f6158871691f8442d903874e64873d43fdca6 Author: Thiago Santos <thiago.sousa.santos@collabora.co.uk> Date: Fri Aug 20 11:09:19 2010 -0300 jifmux: Avoid recombining RGB jpegs JFIF only allows YUV as colorspace, when we receive an RGB jpeg, we should just push it forward without adding the JFIF marker. Fixes #627413