GNOME Bugzilla – Bug 694553
adder: rhythmbox crossfading stopped working after commit a86ca53
Last modified: 2013-03-11 23:31:34 UTC
After this commit: commit a86ca535c88a604daa431b0bc1cf4cd8fbb2d100 Author: Stefan Sauer <ensonic@users.sf.net> Date: Thu Feb 21 08:15:45 2013 +0100 adder: ensure sending a flush-stop after flush-start Previously adder was only sending the flush-stop, when it saw the flushing seek. If one sends a flushing see direcly to an element upstream of adder, it would fail to unflush the downstream pads. the crossfading player pipeline in rhythmbox doesn't work. This gst-launch line doesn't work either: $ gst-launch-1.0 -v audiotestsrc wave=silence ! queue ! adder name=a ! audio/x-raw,format=S16LE,rate=44100,channels=2 ! audioconvert ! autoaudiosink uridecodebin uri=file:///path/to/file.mp3 ! audioconvert ! queue ! a. It appears that elements downstream from adder aren't getting a new-segment: WARNING: from element /GstPipeline:pipeline0/GstAutoAudioSink:autoaudiosink0/GstPulseSink:autoaudiosink0-actual-sink-pulse: Internal data flow problem. Additional debug info: gstbasesink.c(3234): gst_base_sink_chain_unlocked (): /GstPipeline:pipeline0/GstAutoAudioSink:autoaudiosink0/GstPulseSink:autoaudiosink0-actual-sink-pulse: Received buffer without a new-segment. Assuming timestamps start from 0.
I can reproduce the issue with uridecodebin, but not when using two audiotestsrc elements. Also the queues in the above example are not needed. I'll see what I can use instead of uri decodebin to reproduce the problem for a unit test. gst-launch-1.0 -v audiotestsrc freq=400 ! adder name=a ! audio/x-raw,format=S16LE,rate=44100,channels=2 ! audioconvert ! autoaudiosink audiotestsrc freq=100 ! audio/x-raw,format=S16LE,rate=44100,channels=2 ! audioconvert ! a.
commit 1504153012ed4e93d83bef4aba919a716c9dfc6d Author: Stefan Sauer <ensonic@users.sf.net> Date: Mon Feb 25 18:50:33 2013 +0100 adder: mark pending flush-stop on segment event Also add more debug logging. Fixes #694553.
gst-launch works now, but rhythmbox is still broken. Where should I be looking?
Jonathan, does it change anything if you remove the "discard = TRUE" in: case GST_EVENT_FLUSH_START: /* discard flush start events, as we forwarded one already when handing the * flushing seek on the sink pad */ g_atomic_int_set (&adder->need_flush_stop, TRUE); discard = TRUE; GST_DEBUG_OBJECT (pad->pad, "eating flush start"); break;
I just reverted that part too. This helper in 0.10, but now it does not make a difference. Jonathan, it would be awesome if you could distill the essence of what the crossfading does in RB into a unit test. commit e2d0a1835b1efa0e0a9e011fea365707d54edda3 Author: Stefan Sauer <ensonic@users.sf.net> Date: Wed Feb 27 22:05:36 2013 +0100 adder: don't discard the flush-start events This reverts one more part of a86ca535c88a604daa431b0bc1cf4cd8fbb2d100 and hopefully fixes #694553 for good.
It's still not working, sadly. Removing the GST_EVENT_FLUSH_START case from gst_adder_sink_event gets it working again. The adder seems to be getting a new segment from the already linked audiotestsrc bin when I'm linking in a new stream, and I think the resulting flush stop is messing up the output side of the pipeline.
Are you linking in the new stream while the pipeline is playing? I guess you do a flusing seek on the new stream when it is linked? Could you provide a link to the source? We should better define the behavior in this case. Having a stand alone test or even better a unit test would really be nice. In my case I made the change to fix the seek in ready case. All my source play the same segment. The issues was that the pads downstream of adder did not got un-flushed. For adder we need to define how aggregate different segments from the input side to a segment on the output side.
Just to update on the severity: Adder did now work well at all before (at least in the in my tests). I'll try to come up with more tests during this week, but I have no idea how representative they are going to be for what RB is doing (Jonathan, ping?). Module: gst-plugins-base Branch: master Commit: cff9fccc69930f796cf1285a9e44cd74090f00b4 URL: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=cff9fccc69930f796cf1285a9e44cd74090f00b4 Author: Stefan Sauer <ensonic@users.sf.net> Date: Wed Feb 13 21:08:48 2013 +0100 adder: use the collect_pads_query func We were setting the query-func on the sink-pad, which got overwritten when adding the new pad to collect pads. Instead register our query-func with the collect pads object. This fixes filter caps. Add a test for it.
(In reply to comment #8) > > We were setting the query-func on the sink-pad, which got overwritten when > adding the new pad to collect pads. Instead register our query-func with the > collect pads object. This fixes filter caps. Add a test for it. IIRC fixed the exact same bug in videomixer, maybe we should go over the elements, ported from .10 that uses the GstCollectPads ?
Created attachment 238060 [details] reasonably close to what rhythmbox is doing I converted the relevant bits of the rhythmbox code (in full here: http://git.gnome.org/browse/rhythmbox/tree/backends/gstreamer/rb-player-gst-xfade.c) to python and the resulting script has the same problem. It's not pretty, and I suspect some of it isn't working exactly as it should, though. I'll see if I can replace the uridecodebin with another audiotestsrc that just starts later or something.
Jonathan, there was a missing break in the event handling. This caused the caps event handling also trigger a flush start handling, where before my changes this was only running into a flush stop handling. With that break inplace, your script works fine for me. Please let me know if there are still issues with RB. commit 60ccc2f17f1ddcf79f7f1fd8751788937579eb04 Author: Stefan Sauer <ensonic@users.sf.net> Date: Mon Mar 11 22:46:45 2013 +0100 adder: add a missing break
Seems to be working properly now, thanks.