GNOME Bugzilla – Bug 164197
[pngdec] & [pngenc] The decoded image is in RGB(LE) byte order
Last modified: 2005-02-08 11:08:19 UTC
Get a 32bit (RGBA) image, and run the following command: gst-launch filesrc blocksize=100000 location=eye.png ! pngdec ! ffmpegcolorspace ! video/x-raw-rgb,bpp=(int)24,depth=(int)24 ! pngenc ! filesink location=eye2.png The result will have inverted R and B channels. For some reason, inserting an alphacolor into the pipeline, before ffmpegcolorspace makes the problem go away. Another interesting thing to note, is that when launched like this, pngdec's srcpad will fixate to bpp=24,depth=24, which is bogus, as the image is bpp=32,depth=32. That's another bug, though. (Why the image does not get horribly distorted by this, is beyond me) As for the fix... Most probably the red_mask and blue_mask properties of pngdec's source pad should be exchanged, when neccessary. Question is, how do I change those, and how do I detect I need to? (fortunately, PNG is always in network byte order) Another option is to use png_set_bgr(pngdec->png) on little-endian systems, so libpng does not perform the MSB->LSB conversion itself, and the default values for red_mask and green_mask are ok. I'll look into libpng, if there is a way to tell it that it should not modify the byte order at all.. That would be best.
Okay, libpng sucks, I can't tell it to leave the image in network byte order. It always converts it to RGB order, and when calling png_set_bgr() on a big-endian box, the image goes native byte order (BE)->RGB order (LE)->BGR order (BE). So, the only viable solution I see is to switch red_mask and blue_mask. If someone can prove me wrong, by all means, do so!
Created attachment 36077 [details] [review] Patch fixing the bug
Your patch inverts masks, but *always*. If this is host-dependent, shouldn't it be: #if byte-order==... RGB #else BGR #endif ?
The title is misleading (fixing it now), it is not native byte order, rather, it is ALWAYS in RGB byte order (ie, BGR in the big-endian notation used by gstreamer caps).
So we've done this wrong from day one on? How's that possible? :).
No idea... Probably most everything that dealt with pngdec's output assumed that bytes were in RGB order (which is true on LE), regardless of masks. (On the other hand, I misunderstood colorspace stuff before, this might be another case.. Though, http://www.libpng.org/pub/png/spec/iso/index-object.html#7Integers-and-byte-order and http://www.libpng.org/pub/png/libpng-1.2.5-manual.html#section-3.7 make me quite confident I'm right this time. Provided that my information is correct that network byte order == big-endian. This means that if the thing is stored in RGB byte order, always, then with big-endian notation, that's exactly BGR)
As for the fixation, pngdec uses a getcaps function which can be fed anything. It should use explicit caps since format depends on data input and is not changeable.
Created attachment 36736 [details] [review] Patch doing the RGB->BGR stuff, and converting pngdec to explicit caps This patch makes png* work on BGR(BE), and converts pngdec to use explicit caps. This solves part of the problem. Another one (probably in ffmpegcolorspace?) is that pngdec (BGRA)->ffmpegcolorspace->pngenc (BGR24) results in wrong colors. However, this patch breaks alphacolor, since that only accepts RGBA, and not BGRA. I'll submit a patch for that separately.
Applied that too, thanks.