GNOME Bugzilla – Bug 697723
audioringbuffer: Reset segdone when releasing audioringbuffer
Last modified: 2013-12-18 17:39:16 UTC
Created attachment 241167 [details] [review] Patch In gstaudioringbuffer.c and in the function gst_audio_ring_buffer_release(), segdone is not set to 0 whereas ringbuffer memory is freed and I guess it should be the case. I actually had a problem in the case of such a pipeline: alsasrc ! ... ! udpsink It happened (sometimes...) that after handling audio buffers, caps are set again and gst_audio_base_src_setcaps() releases and acquires a new audioringbuffer. Doing this, will stop the thread retrieving audio packets in gstaudiosrc and then restart it when acquiring a new audioringbuffer. When the thread will start it will wait for the ringbuffer to be started (this is done in wait_segment() of gstaudioringbuffer()), in the mean time gst_audio_ring_buffer_read() will be called and process the first buffers. If there is no element creating a thread between alsasrc and udpsink, it happened that udpsink blocked on sync waiting for the clock (generated by audiobasesrc) to reach a value that it never reached. To avoid such a case I reset segdone when releasing the audioringbuffer. When calling gst_audio_ring_buffer_read(), no buffer are pushed as there is no data in the audioringbuffer and wait_segment() is called starting the audioringbuffer which will signal to the thread in gstaudiosrc to start filling the audioringbuffer and data will be processed without risk of locks. P.S.: in the provided patch, I also added initialization of segbase and segdone as I did not find any initialization of these variables anywhere. I guess the compiler set them to 0.
Created attachment 241168 [details] Trace
commit 98f41f1c390e22269f3042adda29d3d345699016 Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Apr 15 10:13:14 2013 +0200 audioringbuffer: Also reset segbase commit 587b2721c876ca70b16f568c0ce12d8675d606ca Author: Paul HENRYS <visechelle@gmail.com> Date: Wed Apr 10 16:38:14 2013 +0200 audioringbuffer: Reset segdone when releasing audioringbuffer https://bugzilla.gnome.org/show_bug.cgi?id=697723
(In reply to comment #0) > in the provided patch, I also added initialization of segbase and segdone > as I did not find any initialization of these variables anywhere. I guess the > compiler set them to 0. The instance/class/private structs in GObject are initialized by GObject to 0 during allocation
Ok, thx for the info Sebastian
This patch causes playbin gapless playback to become gapful with alsasink. tools/gst-play-1.0 --gapless a.wav b.wav This will wait an amount equal to the length of a.wav after playing a.wav, when using alsasink. When using pulsesink, this does not happens as pulsesink uses its own commit function. It seems with this patch, alsasink will loop in commit as it thinks it's back at playback start. Commenting out "g_atomic_int_set (&buf->segdone, 0);" in _release fixes the pause, but presumably introduces again the issue fixed here. So it seems that introduces a bug in the ring buffer (or exposes an existing one) when a ring buffer is "reused", and I can't tell which atm.
I fixed the issue with https://bugzilla.gnome.org/show_bug.cgi?id=720692 Hopefully this does not reintroduce the bug that was fixed here.