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 311848 - Renegotiation broken
Renegotiation broken
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: High major
: 0.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2005-07-28 11:05 UTC by Ronald Bultje
Modified: 2005-08-08 08:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
attempt (7.68 KB, patch)
2005-07-28 15:48 UTC, Ronald Bultje
none Details | Review

Description Ronald Bultje 2005-07-28 11:05:35 UTC
gst-launch-0.9 videotestsrc !
video/x-raw-yuv,width=200,height=150,format=\(fourcc\)YUY2 ! ffmpegcolorspace !
videoscale ! ximagesink

Resize the ximagesink window (to emulate what Totem does), and observe
interesting behaviour, such as crashes if you decrease size and garbled output
if you increase window size.

The negotiation of the new size will fail in ffmpegcolorspace's
gst_base_transform_setcaps() in the no_transform_possible case, because it
somehow tries to set the new window's size from videoscale on ffmpegcolorspace
(all this during gst_pad_alloc_buffer()), this fails because the sizes don't
match, and after that, stuff collapses.

Here's a backtrace of the no_transform_possible in ffmpegcolorspace:

  • #6 gst_base_transform_setcaps
    at gstbasetransform.c line 447
  • #7 gst_pad_set_caps
    at gstpad.c line 1933
  • #8 gst_pad_alloc_buffer
    at gstpad.c line 2021
  • #9 gst_base_transform_handle_buffer
    at gstbasetransform.c line 587
  • #10 gst_base_transform_chain
    at gstbasetransform.c line 658
  • #11 gst_pad_chain
    at gstpad.c line 2844
  • #12 gst_pad_push
    at gstpad.c line 2930
  • #13 gst_capsfilter_chain
    at gstcapsfilter.c line 194
  • #14 gst_pad_chain
    at gstpad.c line 2844
  • #15 gst_pad_push
    at gstpad.c line 2930
  • #16 gst_base_src_loop
    at gstbasesrc.c line 677
  • #17 gst_task_func
    at gsttask.c line 126
  • #18 g_thread_pool_free
    from /usr/lib/libglib-2.0.so.0
  • #19 g_static_private_free
    from /usr/lib/libglib-2.0.so.0
  • #20 start_thread
    from /lib/tls/libpthread.so.0
  • #21 clone
    from /lib/tls/libc.so.6
$1 = (
    gchar *) 0x86c7dd8 "video/x-raw-rgb, bpp=(int)32, depth=(int)24,
endianness=(int)4321, red_mask=(int)65280, green_mask=(int)16711680,
blue_mask=(int)-16777216, width=(int)201, height=(int)152, framerate=(double)30,
pixel"...

Let's now look at gst-launch-0.9 videotestsrc !
video/x-raw-yuv,width=200,height=150,format=\(fourcc\)YUY2 ! queue !
ffmpegcolorspace ! videoscale ! ximagesink, and resize the window again, which
more closely resembles Totem's behaviour; it will directly quit at:

ERROR (0x8f19310 - 0:00:02.947686000)  queue_dataflow(30547)
gstqueue.c(783):gst_queue_loop:<queue0> streaming stopped, reason pad not negotiated
ERROR (0x8f19310 - 0:00:02.947818000)  queue_dataflow(30547)
gstqueue.c(783):gst_queue_loop:<queue0> streaming stopped, reason pad not negotiated
ERROR: from element /pipeline0/queue0: streaming stopped, reason pad not negotiated

I have the feeling that this is because of ASYNC caps. Problem is that
gst_pad_buffer_alloc() calls gst_pad_set_caps(), whereas the normal dataflow
also calls this, and those could conflict in some cases. I'd rather see
gst_pad_buffer_alloc() only check whether pads can accept the caps, and have
actual normal dataflow take care of forward renegotiation afterwards (i.e. move
responsibility of renegotiation further down the stack).
Comment 1 Ronald Bultje 2005-07-28 11:43:00 UTC
Well, I got as far as blaming the trans->delay_configure in basetransform, since
the delayed capsnego will obviously never take place. Removing that makes the
queue-less pipeline work fine.

The queue-containing pipeline will still fail with this, though, and here's
where the ASYNC story comes into play again...
Comment 2 Ronald Bultje 2005-07-28 15:48:46 UTC
Created attachment 49886 [details] [review]
attempt

Above patch tries to implement it.

*) for some reason, it just doesn't work; part of the reason may be that
gst_pad_accept_caps() falsely succeeds.
*) gst_pad_accept_caps() is not recursive, which makes it practically useless,
see above. The reason that it's not recursive is that it calls
gst_pad_get_caps() instead of gst_pad_get_allowed_caps().
*) however, gst_pad_get_allowed_caps() leads to deadlocks.

More design-wise:
*) should videotestsrc ! sizefilter ! queue ! videoscale ! ximagesink have
videotestsrc do the scaling, or should videoscale do it, or should videoscale
do it for cached buffers of the old size and videotestsrc for the rest?
gst_pad_proxy_alloc_buffer() isn't very useful in any of those cases, because
it does not renegotiate the sourcepad doing the buffer request.
*) in-place elements should never change their sourcepad caps without sinkpad
caps being changed and should proxy buffer-alloc calls. Not-in-place elements
should not proxy buffer-alloc calls and call those themselves. But what about
elements that are sometimes in place and sometimes not (ffmpegcolorspace,
videoscale)? How to efficiently implement the ideal behaviour?

I don't really know how to make this system work for the non-synchronous case
(so anything with a queue in it)...
Comment 3 Ronald Bultje 2005-08-08 08:41:32 UTC
Andy implemented the above, closing.