After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 163177 - [pngdec] Can't cope with png's without an alpha channel
[pngdec] Can't cope with png's without an alpha channel
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins
git master
Other Linux
: Normal normal
: 0.8.8
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-01-06 21:38 UTC by Gergely Nagy
Modified: 2005-01-09 01:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch fixing the problem (957 bytes, patch)
2005-01-06 21:52 UTC, Gergely Nagy
none Details | Review
Patch to fix the problem (3.67 KB, patch)
2005-01-08 16:46 UTC, Gergely Nagy
none Details | Review

Description Gergely Nagy 2005-01-06 21:38:57 UTC
Grab a png without an alpha channel, and decode it with pngdec, then re-encode
it with pngenc, and observe that the result is somewhat disappointing.

Problem lies in ext/libpng/gstpngdec.c, function gst_pngdec_chain, near the end,
where the code reads:

  inp = GST_BUFFER_DATA (out);
  for (i = 0; i < height; i++) {
    rows[i] = inp;
    inp += width * 4;
  }

That assumes that the thing is 32 bits wide, when it isn't. It is only 24.
Two solutions exist: first, force libpng to add a dummy alpha channel
(PNG_TRANSFORM_EXPAND flag, iirc), or to use pngdec->bpp / depth instead of that
hard-coded 4.

I can supply a patch for any of those (for now, I did the pngdec->bpp / depth
thing, that one is more memory friendly, and pngdec only supports 8 bpp RGB+RGBA
stuff now anyway).
Comment 1 Gergely Nagy 2005-01-06 21:52:38 UTC
Created attachment 35582 [details] [review]
Patch fixing the problem

Does two things:

in gst_pngdec_src_getcaps(): sets up ->bpp.
in gst_pngdec_chain(): uses that (together with the depth variable computed
there) to determine the length of a line in bytes.
Comment 2 Gergely Nagy 2005-01-07 14:52:25 UTC
#162306 obsoletes this one, as the patch I submitted there fixes this problem too.
Comment 3 Gergely Nagy 2005-01-08 16:46:46 UTC
Created attachment 35670 [details] [review]
Patch to fix the problem

Compared to the previous patch, this does:

Bail out earlier if encountering an image that has a bit_depth other than 8,
free resources and unref the buffer on error paths in gst_pngdec_chain(), and
add proper support for alpha channelled pngs. Instead of computing the length
of the row ourselves when setting up row pointers for libpng, the patch makes
pngdec use pngdec->info->rowbytes.

Also, in gst_pngdec_chain(), only allocate just enough memory, so we don't send
out too much data for nothing.
Comment 4 Ronald Bultje 2005-01-09 01:40:00 UTC
applied, thanks.