imagefreeze: Fix another subtle race condition related to starting the srcpad task

Due to a seek the srcpad task could be started in rare circumstances although
it shouldn't be started anymore because no upstream buffer is available.
This commit is contained in:
Sebastian Dröge 2010-08-20 10:34:17 +02:00
parent e51fe6c181
commit 86403e85c5
2 changed files with 47 additions and 33 deletions

View File

@ -135,6 +135,8 @@ gst_image_freeze_init (GstImageFreeze * self, GstImageFreezeClass * g_class)
gst_pad_use_fixed_caps (self->srcpad); gst_pad_use_fixed_caps (self->srcpad);
gst_element_add_pad (GST_ELEMENT (self), self->srcpad); gst_element_add_pad (GST_ELEMENT (self), self->srcpad);
self->lock = g_mutex_new ();
gst_image_freeze_reset (self); gst_image_freeze_reset (self);
} }
@ -145,6 +147,10 @@ gst_image_freeze_finalize (GObject * object)
gst_image_freeze_reset (self); gst_image_freeze_reset (self);
if (self->lock)
g_mutex_free (self->lock);
self->lock = NULL;
G_OBJECT_CLASS (parent_class)->finalize (object); G_OBJECT_CLASS (parent_class)->finalize (object);
} }
@ -153,7 +159,7 @@ gst_image_freeze_reset (GstImageFreeze * self)
{ {
GST_DEBUG_OBJECT (self, "Resetting internal state"); GST_DEBUG_OBJECT (self, "Resetting internal state");
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
gst_buffer_replace (&self->buffer, NULL); gst_buffer_replace (&self->buffer, NULL);
gst_segment_init (&self->segment, GST_FORMAT_TIME); gst_segment_init (&self->segment, GST_FORMAT_TIME);
@ -162,7 +168,7 @@ gst_image_freeze_reset (GstImageFreeze * self)
self->fps_n = self->fps_d = 0; self->fps_n = self->fps_d = 0;
self->offset = 0; self->offset = 0;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
g_atomic_int_set (&self->seeking, 0); g_atomic_int_set (&self->seeking, 0);
} }
@ -224,10 +230,10 @@ gst_image_freeze_sink_setcaps (GstPad * pad, GstCaps * caps)
gst_structure_fixate_field_nearest_fraction (s, "framerate", 25, 1)) { gst_structure_fixate_field_nearest_fraction (s, "framerate", 25, 1)) {
gst_structure_get_fraction (s, "framerate", &fps_n, &fps_d); gst_structure_get_fraction (s, "framerate", &fps_n, &fps_d);
if (fps_d != 0) { if (fps_d != 0) {
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
self->fps_n = fps_n; self->fps_n = fps_n;
self->fps_d = fps_d; self->fps_d = fps_d;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
GST_DEBUG_OBJECT (pad, "Setting caps %" GST_PTR_FORMAT, candidate); GST_DEBUG_OBJECT (pad, "Setting caps %" GST_PTR_FORMAT, candidate);
gst_pad_set_caps (self->srcpad, candidate); gst_pad_set_caps (self->srcpad, candidate);
gst_caps_unref (candidate); gst_caps_unref (candidate);
@ -324,9 +330,9 @@ gst_image_freeze_sink_bufferalloc (GstPad * pad, guint64 offset, guint size,
*buf = NULL; *buf = NULL;
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
do_alloc = self->buffer == NULL; do_alloc = self->buffer == NULL;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
if (do_alloc) { if (do_alloc) {
gboolean seeking = FALSE; gboolean seeking = FALSE;
@ -374,14 +380,14 @@ gst_image_freeze_convert (GstImageFreeze * self,
case GST_FORMAT_DEFAULT:{ case GST_FORMAT_DEFAULT:{
switch (*dest_format) { switch (*dest_format) {
case GST_FORMAT_TIME: case GST_FORMAT_TIME:
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
if (self->fps_n == 0) if (self->fps_n == 0)
*dest_value = -1; *dest_value = -1;
else else
*dest_value = *dest_value =
gst_util_uint64_scale (src_value, GST_SECOND * self->fps_d, gst_util_uint64_scale (src_value, GST_SECOND * self->fps_d,
self->fps_n); self->fps_n);
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
ret = TRUE; ret = TRUE;
break; break;
default: default:
@ -391,11 +397,11 @@ gst_image_freeze_convert (GstImageFreeze * self,
case GST_FORMAT_TIME:{ case GST_FORMAT_TIME:{
switch (*dest_format) { switch (*dest_format) {
case GST_FORMAT_DEFAULT: case GST_FORMAT_DEFAULT:
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
*dest_value = *dest_value =
gst_util_uint64_scale (src_value, self->fps_n, gst_util_uint64_scale (src_value, self->fps_n,
self->fps_d * GST_SECOND); self->fps_d * GST_SECOND);
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
ret = TRUE; ret = TRUE;
break; break;
default: default:
@ -455,15 +461,15 @@ gst_image_freeze_src_query (GstPad * pad, GstQuery * query)
gst_query_parse_position (query, &format, NULL); gst_query_parse_position (query, &format, NULL);
switch (format) { switch (format) {
case GST_FORMAT_DEFAULT:{ case GST_FORMAT_DEFAULT:{
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
position = self->offset; position = self->offset;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
ret = TRUE; ret = TRUE;
} }
case GST_FORMAT_TIME:{ case GST_FORMAT_TIME:{
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
position = self->segment.last_stop; position = self->segment.last_stop;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
ret = TRUE; ret = TRUE;
} }
default: default:
@ -487,19 +493,19 @@ gst_image_freeze_src_query (GstPad * pad, GstQuery * query)
gst_query_parse_duration (query, &format, NULL); gst_query_parse_duration (query, &format, NULL);
switch (format) { switch (format) {
case GST_FORMAT_TIME:{ case GST_FORMAT_TIME:{
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
duration = self->segment.stop; duration = self->segment.stop;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
ret = TRUE; ret = TRUE;
} }
case GST_FORMAT_DEFAULT:{ case GST_FORMAT_DEFAULT:{
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
duration = self->segment.stop; duration = self->segment.stop;
if (duration != -1) if (duration != -1)
duration = duration =
gst_util_uint64_scale (duration, self->fps_n, gst_util_uint64_scale (duration, self->fps_n,
GST_SECOND * self->fps_d); GST_SECOND * self->fps_d);
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
ret = TRUE; ret = TRUE;
} }
default: default:
@ -630,7 +636,7 @@ gst_image_freeze_src_event (GstPad * pad, GstEvent * event)
GST_PAD_STREAM_LOCK (self->srcpad); GST_PAD_STREAM_LOCK (self->srcpad);
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
gst_event_replace (&self->close_segment, NULL); gst_event_replace (&self->close_segment, NULL);
if (!flush) { if (!flush) {
if (!self->need_segment && self->segment.rate >= 0) { if (!self->need_segment && self->segment.rate >= 0) {
@ -657,7 +663,7 @@ gst_image_freeze_src_event (GstPad * pad, GstEvent * event)
last_stop = self->segment.last_stop; last_stop = self->segment.last_stop;
start_task = self->buffer != NULL; start_task = self->buffer != NULL;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
if (flush) { if (flush) {
GstEvent *e; GstEvent *e;
@ -679,9 +685,16 @@ gst_image_freeze_src_event (GstPad * pad, GstEvent * event)
GST_DEBUG_OBJECT (pad, "Seek successful"); GST_DEBUG_OBJECT (pad, "Seek successful");
if (start_task) if (start_task) {
gst_pad_start_task (self->srcpad, g_mutex_lock (self->lock);
(GstTaskFunction) gst_image_freeze_src_loop, self->srcpad);
if (self->buffer != NULL)
gst_pad_start_task (self->srcpad,
(GstTaskFunction) gst_image_freeze_src_loop, self->srcpad);
g_mutex_unlock (self->lock);
}
ret = TRUE; ret = TRUE;
break; break;
} }
@ -702,19 +715,19 @@ gst_image_freeze_sink_chain (GstPad * pad, GstBuffer * buffer)
{ {
GstImageFreeze *self = GST_IMAGE_FREEZE (GST_PAD_PARENT (pad)); GstImageFreeze *self = GST_IMAGE_FREEZE (GST_PAD_PARENT (pad));
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
if (self->buffer) { if (self->buffer) {
GST_DEBUG_OBJECT (pad, "Already have a buffer, dropping"); GST_DEBUG_OBJECT (pad, "Already have a buffer, dropping");
gst_buffer_unref (buffer); gst_buffer_unref (buffer);
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
return GST_FLOW_UNEXPECTED; return GST_FLOW_UNEXPECTED;
} }
self->buffer = buffer; self->buffer = buffer;
GST_OBJECT_UNLOCK (self);
gst_pad_start_task (self->srcpad, (GstTaskFunction) gst_image_freeze_src_loop, gst_pad_start_task (self->srcpad, (GstTaskFunction) gst_image_freeze_src_loop,
self->srcpad); self->srcpad);
g_mutex_unlock (self->lock);
return GST_FLOW_OK; return GST_FLOW_OK;
} }
@ -728,16 +741,16 @@ gst_image_freeze_src_loop (GstPad * pad)
gint64 cstart, cstop; gint64 cstart, cstop;
gboolean in_seg, eos; gboolean in_seg, eos;
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
if (!self->buffer) { if (!self->buffer) {
GST_ERROR_OBJECT (pad, "Have no buffer yet"); GST_ERROR_OBJECT (pad, "Have no buffer yet");
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
gst_pad_pause_task (self->srcpad); gst_pad_pause_task (self->srcpad);
return; return;
} }
buffer = gst_buffer_ref (self->buffer); buffer = gst_buffer_ref (self->buffer);
buffer = gst_buffer_make_metadata_writable (buffer); buffer = gst_buffer_make_metadata_writable (buffer);
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
if (self->close_segment) { if (self->close_segment) {
GST_DEBUG_OBJECT (pad, "Closing previous segment"); GST_DEBUG_OBJECT (pad, "Closing previous segment");
@ -754,7 +767,7 @@ gst_image_freeze_src_loop (GstPad * pad)
self->segment.applied_rate, self->segment.format, self->segment.start, self->segment.applied_rate, self->segment.format, self->segment.start,
self->segment.stop, self->segment.start); self->segment.stop, self->segment.start);
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
if (self->segment.rate >= 0) { if (self->segment.rate >= 0) {
self->offset = self->offset =
gst_util_uint64_scale (self->segment.start, self->fps_n, gst_util_uint64_scale (self->segment.start, self->fps_n,
@ -764,14 +777,14 @@ gst_image_freeze_src_loop (GstPad * pad)
gst_util_uint64_scale (self->segment.stop, self->fps_n, gst_util_uint64_scale (self->segment.stop, self->fps_n,
self->fps_d * GST_SECOND); self->fps_d * GST_SECOND);
} }
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
self->need_segment = FALSE; self->need_segment = FALSE;
gst_pad_push_event (self->srcpad, e); gst_pad_push_event (self->srcpad, e);
} }
GST_OBJECT_LOCK (self); g_mutex_lock (self->lock);
offset = self->offset; offset = self->offset;
if (self->fps_n != 0) { if (self->fps_n != 0) {
@ -807,7 +820,7 @@ gst_image_freeze_src_loop (GstPad * pad)
self->offset++; self->offset++;
else else
self->offset--; self->offset--;
GST_OBJECT_UNLOCK (self); g_mutex_unlock (self->lock);
GST_DEBUG_OBJECT (pad, "Handling buffer with timestamp %" GST_TIME_FORMAT, GST_DEBUG_OBJECT (pad, "Handling buffer with timestamp %" GST_TIME_FORMAT,
GST_TIME_ARGS (timestamp)); GST_TIME_ARGS (timestamp));

View File

@ -47,6 +47,7 @@ struct _GstImageFreeze
GstPad *sinkpad; GstPad *sinkpad;
GstPad *srcpad; GstPad *srcpad;
GMutex *lock;
GstBuffer *buffer; GstBuffer *buffer;
gint fps_n, fps_d; gint fps_n, fps_d;