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 621332 - BaseTransform should disable proxy alloc if downstream changes caps
BaseTransform should disable proxy alloc if downstream changes caps
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 0.10.31
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-06-11 22:45 UTC by Olivier Crête
Modified: 2010-08-26 13:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test code to reproduce bug (897 bytes, text/x-python)
2010-07-08 01:22 UTC, Youness Alaoui
Details

Description Olivier Crête 2010-06-11 22:45:22 UTC
If I have a pipeline like  src ! caps1 ! transform ! caps1 ! sink and I replace caps1 by a different caps2. Then basetransform keeps on trying to do pad alloc and a gst_pad_accept_caps() for each buffer. This can be very slow (and is useless). 

I quickly tried to hack GstBaseTransform to make it work by doing priv->proxy_alloc = FALSE if the gst_pad_accept_caps() fails, but it doesn't re-start proxying when the downstream caps change.

Anyway, this caps/suggest/alloc/etc crap in BaseTransform is a disaster.


Minimal example program:
import gst, time, sys

p = gst.parse_launch("audiotestsrc is-live=1 ! audio/x-raw-int, rate=8000 ! audioresample ! capsfilter name=c ! fakesink");
c = p.get_by_name("c")
p.set_state(gst.STATE_PLAYING)
time.sleep(2)
# After this is it start doing too many caps nego after each alloc call
c.set_property("caps", gst.caps_from_string("audio/x-raw-int, rate=16000"))
time.sleep(2) 
# After this, it should get back to proxying pad alloc
c.set_property("caps", gst.caps_from_string("ANY"))
time.sleep(2)
p.set_state(gst.STATE_NULL)
Comment 1 Wim Taymans 2010-06-14 14:26:54 UTC
> Anyway, this caps/suggest/alloc/etc crap in BaseTransform is a disaster.
> 
Don't worry, we have it under control.

the problem was that a flag was not cleared:

commit f80a824a2f2ba3d85e25585871acb60a4ad802dd
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Jun 14 16:20:18 2010 +0200

    basetransform: reevaluate proxy_alloc when reconfigured
    
    When we reconfigure the transform element, make sure we reevaluate the proxying
    of buffer_alloc the next time around.
    
    Fixes #621332


> # After this, it should get back to proxying pad alloc
> c.set_property("caps", gst.caps_from_string("ANY"))

Arguably, it currently keeps doing wat it was doing instead of trying to renegotiate.
Comment 2 Youness Alaoui 2010-07-08 01:22:55 UTC
Created attachment 165451 [details]
test code to reproduce bug

This bug is not fixed yet.. I'm attaching a test file that reproduces the bug in a different way... to summarize, I end up with :
video/x-raw-yuv ! ffmpegcolorspace ! video/x-raw-rgb
and the ffmpegcolorspace proxies the pad_aloc while it shouldn't.
Comment 3 Thiago Sousa Santos 2010-08-25 20:09:03 UTC
Seems we need to reopen for further investigation.
Comment 4 Wim Taymans 2010-08-26 08:46:33 UTC
(In reply to comment #2)
> Created an attachment (id=165451) [details]
> test code to reproduce bug
> 
> This bug is not fixed yet.. I'm attaching a test file that reproduces the bug
> in a different way... to summarize, I end up with :
> video/x-raw-yuv ! ffmpegcolorspace ! video/x-raw-rgb
> and the ffmpegcolorspace proxies the pad_aloc while it shouldn't.

It seems like a problem with edgetv to me.
Comment 5 Wim Taymans 2010-08-26 10:33:49 UTC
This runs into Bug #614296.

In this particular case, we could improve things a little. 

The problem is that the first ffmpegcolorspace receives a buffer in yuv with framerate 30/1 and attempts a buffer_alloc with yuv (because that is the format it was doing before the edgetv was inserted). 

Then edgetv refuses that format (it only accepts rgb) and tries to come up with a good suggestion for a new format. It generates this suggestion by first doing a get_allowed_caps() upstream to get a list of allowed caps. 

Upstream reports that it can produce everything, which is false, it can produce anything as long as it has a framerate of 30/1 (ffmpegcolorspace can convert between all colorspaces but the framerate of the buffer it is currently handling cannot change).

Then ffmpegcolorspace refuses the downstream suggestion and continues pushing the yuv frame to edgetv, which refuses and ends the pipeline with not-negotiated.

I'm not quite sure what to do. Here are some possibilities:

1) edgetv makes it suggestion by removing the unfixed fields. upstream basetransform should then complete the suggested format by fixating/adding fields based on the input format. 

2) edgetv simply suggests the non-fixed caps and lets the upstream basetransform fixate. We don't currently put unfixed caps on buffers but maybe it makes sense for upstream caps renegotiation.
Comment 6 Wim Taymans 2010-08-26 13:20:02 UTC
The easiest solution is to let basetransform attempt to find an alternative format when the downstream suggestion is impossible.

commit f8abf35000570a8afe64268b430e998e2d735e61
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Thu Aug 26 15:12:49 2010 +0200

    basetransform: recover from invalid downstream suggestions
    
    When we are handling a buffer and need to allocate an output buffer, handle the
    case when downstream suggests us a format that we can't convert the input buffer
    to. In that case, check if there is another format available downstream instead
    of failing.
    
    Fixes #621332 and see also #614296