[gstreamer-bugs] [Bug 429319] [alphacolor] distorts png images without alpha channel

[GStreamer (bugzilla.gnome.org)]

If you have any questions why you received this email, please see the text at
the end of this email. Replies to this email are NOT read, please see the text
at the end of this email. You can add comments to this bug at:
  http://bugzilla.gnome.org/show_bug.cgi?id=429319

  GStreamer | gst-plugins-good | Ver: 0.10.5

Tim-Philipp Müller changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |421543
  Attachment #86826|none                        |needs-work
               Flag|                            |
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED
            Summary|Alphacolor distorts png     |[alphacolor] distorts png
                   |images without alpha channel|images without alpha channel
   Target Milestone|HEAD                        |0.10.6




------- Comment #6 from Tim-Philipp Müller  2007-04-25 15:57 UTC -------
The real problem here is that pngdec outputs RGB24 data without an alpha
channel, a format that is incompatible with alphacolor's sink pad template
caps. Yet the core doesn't realise this for some reason and instead of erroring
out, alphacolor's _set_caps() function is called and doesn't notice the mistake
either.

I believe the core issue is bug #421543 (although I haven't actually tested the
patch there).

I've commited some fixes to alphacolor that make sure the input caps are in
fact RGBA caps now, so that it errors out properly when that is not the case:

 2007-04-25  Tim-Philipp Müller  <tim at centricular dot net>

        * gst/alpha/gstalphacolor.c: (gst_alpha_color_base_init),
        (gst_alpha_color_transform_caps), (gst_alpha_color_set_caps):
          Double-check that RGB input caps are really RGBA caps (apparently
          the core doesn't always catch it if those caps aren't a subset of
          our template caps, also see #421543). Fixes #429319 in a way.
          Also, don't leak the pad template in the transform_caps function.

        * tests/check/Makefile.am:
        * tests/check/elements/.cvsignore:
        * tests/check/elements/alphacolor.c: (setup_alphacolor),
        (cleanup_alphacolor), (create_caps_rgb24), (create_caps_rgba32),
        (create_buffer_rgb24_3x4), (create_buffer_rgba32_3x4),
        (GST_START_TEST), (alphacolor_suite):
          Add some basic unit tests for alphacolor.

I've also committed a fix to pngdec that makes it post any error messages
before the eos, so that gst-launch actually shows the error and doesn't think
everything went just fine.


About your patch (thanks for that, btw):  

a) It raises the question whether wewould want RGB24 => AYUV functionality in
alphacolor int the first place. I think the main advantage of alphacolor is
that it does colorspace conversion in-place, ie. it doesn't allocate a new
buffer for each conversion. If you convert RGB24 => AYUV, you need to allocate
a new buffer; this functionality could of course be added to alphacolor, but
I'm not really sure if it's required - plugging an ffmpegcolorspace is just as
good, isn't it? It doesn't buy you anything besides convenience.

b) Code-wise: the current alphacolor code uses the GstBaseTransform::in_place()
vfunc - if you want to accept RGB24 as input format you need to implement the
right GstBaseTransform methods in the right way so that in_place transform and
'normal' transform is chosen as appropriate etc. What you're doing is a bit
hackish and it leaks too if I'm not mistaken. You'll need to implement a
::get_unit_size() vfunc and a normal  ::transform() vfunc IIRC.


If you do want RGB24=>AYUV support in alphacolor, then please file a new
enhancement bug (or clone this bug).


For your pipeline you probably just want something like:

   filesrc ! pngdec ! ffmpegcolorspace ! alphacolor ! ....

The ffmpegcolorspace will operate in pass-through mode if pngdec outputs
buffers in RGBA and no conversion is needed.


-- 
See http://bugzilla.gnome.org/page.cgi?id=email.html for more info about why you received
this email, why you can't respond via email, how to stop receiving
emails (or reduce the number you receive), and how to contact someone
if you are having problems with the system.

You can add comments to this bug at http://bugzilla.gnome.org/show_bug.cgi?id=429319.