GNOME Bugzilla – Bug 330996
basesrc emits EOS unconditionally when going to READY
Last modified: 2006-02-24 11:12:20 UTC
Currently basesrc emits an EOS event unconditionally when going PAUSED->READY. This occurs even if an EOS was already sent. This is a bug and should be fixed. Reverting http://cvs.freedesktop.org/gstreamer/gstreamer/tests/check/gst/gstutils.c.diff?r1=1.10&r2=1.11 will show the problem in the test suite. The bug was introduced on http://cvs.freedesktop.org/gstreamer/gstreamer/libs/gst/base/gstbasesrc.c?r1=1.92&r2=1.93. The bug was not previously noticed because event probes used to not be fired if the pad was flushing, but now they are.
Created attachment 59267 [details] [review] proposed fix Proposed patch attached (felt it was time to add a private struct ...)
Should add a pointer to the priv data from the public struct, as in http://blogs.gnome.org/view/markmc/2006/01/25/2 and in GstBus.
Created attachment 59278 [details] [review] new patch New patch. Note in particular the additions to gst_base_src_activate_pull() and gst_base_src_activate_push(), which are there to avoid sending the event when last operating pullrange()-based. (on a side note, basesrc->pad_mode seems unused)
Perhaps the correct fix is to set basesrc->pad_mode appropriately. Would make things a bit more maintainable.
I am not sure whether setting basesrc->pad_mode would help, as it would be set to NONE when the pad gets deactivated, so in the state change function we still wouldn't know what the value before NONE was. Sending the EOS in the pad de-activation function also doesn't appear to be an option, as seems perfectly legitimate to me to change the mode of operation while running, no? (elements downstream might be unplugged/replaced etc.)
Sending EOS when going from READY->PAUSED is pretty useless since the downstream element will already be in READY and the event will thus be pushed onto a flushing pad. Apparently sound-juicer sets the source to READY _first_ so that the EOS event is able to travel downstream and the elements can flush and close properly... I have a feeling a more elegant way of handling this use case exists...
Originally I simply wanted to add a gst_base_src_stop() or _send_eos() (or use a signal, but that's a bit hacky) ...
Ping? Given that this 'sends-EOS-on-setting-to-READY' behaviour went into the 0.10.3 release we probably need to keep it in future 0.10.x releases, no? If so, is there really an alternative to the patch I proposed, even if it's admittedly not a nice solution code-wise? (btw, it's gnome-sound-recorder, not sound-juicer)
patch stays in. Keeping bug open because we must not forget to document how to shut down this type of (live) pipelines.
Patch committed, gstutils test case reverted, and new tests for GstBaseSrc EOS behaviour added: 2006-02-23 Tim-Philipp Müller <tim at centricular dot net> * tests/check/Makefile.am: * tests/check/libs/basesrc.c: (eos_event_counter), (basesrc_eos_events_pull), (basesrc_eos_events_push), (basesrc_eos_events_push_live_op), (basesrc_eos_events_pull_live_op), (gst_basesrc_suite), (main): ... and add some tests for the base source EOS stuff. 2006-02-23 Tim-Philipp Müller <tim at centricular dot net> * tests/check/gst/gstutils.c: (test_buffer_probe_n_times): Test case originally showed the problem fixed below, but was then amended. Add checks back at the place where they used to be. 2006-02-23 Tim-Philipp Müller <tim at centricular dot net> * libs/gst/base/gstbasesrc.c: (gst_base_src_class_init), (gst_base_src_init), (gst_base_src_loop), (gst_base_src_activate_push), (gst_base_src_activate_pull), (gst_base_src_change_state): * libs/gst/base/gstbasesrc.h: Don't unconditionally send EOS when going from PAUSED to READY state, esp. make sure we don't send two EOS events in some cases (e.g. one when reaching EOS and one when going from PAUSED to READY). Also, we don't want to send EOS events when operating in pull mode. However, we do want to send an EOS event when shutting down a live source explicitly, for example (fixes #330996).
Added some docs, closing. 2006-02-24 Tim-Philipp Müller <tim at centricular dot net> * libs/gst/base/gstbasesrc.c: Document how applications can stop recording from live sources (see #330996).