GNOME Bugzilla – Bug 630257
GST_DEBUG_DUMP_DOT_DIR not working anymore
Last modified: 2010-10-05 09:31:14 UTC
GST_DEBUG_DUMP_DOT_DIR is not working anymore. $export GST_DEBUG_DUMP_DOT_DIR=/tmp $gst-launch videotestsrc ! xvimagesink and this pipeline does not want to preroll...
Confirmed, pipeline doesn't want to preroll but only if that environment variable is set.
Relevant part of the backtrace. Something else is holding the preroll lock too apparently ;)
+ Trace 223889
Thread 3 (Thread 0x7ffff38f7710 (LWP 27490))
Marking as blocker, since it looks like a regression.
The preroll lock is always taken while buffers/events are processed, the regression here most probably is this: commit 3eb97aa32c9a9be382d2be7b2fb617c4d88042fc Author: Stefan Kost <ensonic@users.sf.net> Date: Thu Sep 9 15:57:15 2010 +0300 gst-launch: add a sync bus handler and move state-change logging there The sync handler is called for all mesages, the event loop we previously use was not. In the sync handler trigger pipeline dot dumps and call access for file in tmp-dir to add markers interceptable by strace and co. Before it would be deferred to the main loop while now it happens from the streaming thread, giving a deadlock. Stefan, what exactly is the reason not to use a bus handler on the mainloop?
Drop the "most probably" from the last comment, reverting this makes everything work again.
The reason for the change is that the main-loop is not running for most of the state change and thus this was only working partialy before. I haven't noticed any problem with the change so far, will now have a look.
Isn't the problem here that *reading* a property is protected with a lock in basesink: gst_base_sink_get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec) { GstBaseSink *sink = GST_BASE_SINK (object); switch (prop_id) { case PROP_PREROLL_QUEUE_LEN: GST_PAD_PREROLL_LOCK (sink->sinkpad); g_value_set_uint (value, sink->preroll_queue_max_len); GST_PAD_PREROLL_UNLOCK (sink->sinkpad); The lock around that parameter was introduced long time ago. commit 94e1664fca0024de8b20d930b5b9f58083b91243 Author: Wim Taymans <wim.taymans@gmail.com> Date: Wed Jan 18 16:40:16 2006 +0000 libs/gst/base/gstbasesink.c: Small cleanups. Removing the locks on the property reads make it work and I get not failing tests. I am attaching a patch that uses atomic ops (btw. the preroll_queue_max_len variable is gint and the property guint with a range of 0...G_MAXUINT).
Created attachment 171082 [details] [review] use atomic ops in _get_property
Not sure if you can use atomic ops on these fields. The preroll locks is taken a very long time and during the call of many functions, it might be that basesink expects it to not change during all the time.
(In reply to comment #9) > Not sure if you can use atomic ops on these fields. The preroll locks is taken > a very long time and during the call of many functions, it might be that > basesink expects it to not change during all the time. We don'T change them, we only read them. The GST_DEBUG_DUMP_DOT_DIR shows non-default properties.
Yes that part is fine, but you change gst_base_sink_set_async_enabled() to atomic ops.
Created attachment 171101 [details] [review] use atomic ops in _get_property oops, fixed
(In reply to comment #12) > Created an attachment (id=171101) [details] [review] > use atomic ops in _get_property If you use atomic ops to read properties, you need to use atomic ops to write properties too, else nothing is protected anymore.
Created attachment 171421 [details] [review] use atomic ops in _get_property Also use the atomic ops to modify the parameters atomically, as otherwise reading them in the same way is not guaranteed.
commit a11b047d0074af375628bdd6cbbf99ccd0f20661 Author: Stefan Kost <ensonic@users.sf.net> Date: Sat Sep 25 14:24:46 2010 +0300 basesink: don't take preroll-lock in get_property Use atomic ops to read and write more properties. Taking the preroll lock in get_property can lock up applications reading the property during preroll.