interlace: Protect all properties with the lock

Avoid blatant data races here.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1039>
This commit is contained in:
Jan Alexander Steffens (heftig) 2021-10-21 00:37:47 +02:00 committed by GStreamer Marge Bot
parent 683680f6e5
commit 2bf6e2a20e

View file

@ -313,7 +313,10 @@ gst_interlace_finalize (GObject * obj)
static void static void
gst_interlace_reset (GstInterlace * interlace) gst_interlace_reset (GstInterlace * interlace)
{ {
g_mutex_lock (&interlace->lock);
interlace->phase_index = interlace->pattern_offset; interlace->phase_index = interlace->pattern_offset;
g_mutex_unlock (&interlace->lock);
interlace->timebase = GST_CLOCK_TIME_NONE; interlace->timebase = GST_CLOCK_TIME_NONE;
interlace->field_index = 0; interlace->field_index = 0;
interlace->passthrough = FALSE; interlace->passthrough = FALSE;
@ -391,10 +394,12 @@ gst_interlace_decorate_buffer_ts (GstInterlace * interlace, GstBuffer * buf,
int n_fields) int n_fields)
{ {
gint src_fps_n, src_fps_d; gint src_fps_n, src_fps_d;
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
src_fps_n = interlace->src_fps_n; src_fps_n = interlace->src_fps_n;
src_fps_d = interlace->src_fps_d; src_fps_d = interlace->src_fps_d;
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
/* field duration = src_fps_d / (2 * src_fps_n) */ /* field duration = src_fps_d / (2 * src_fps_n) */
if (src_fps_n == 0) { if (src_fps_n == 0) {
/* If we don't know the fps, we can't generate timestamps/durations */ /* If we don't know the fps, we can't generate timestamps/durations */
@ -415,6 +420,12 @@ static void
gst_interlace_decorate_buffer (GstInterlace * interlace, GstBuffer * buf, gst_interlace_decorate_buffer (GstInterlace * interlace, GstBuffer * buf,
int n_fields, gboolean interlaced) int n_fields, gboolean interlaced)
{ {
GstInterlacePattern pattern;
g_mutex_lock (&interlace->lock);
pattern = interlace->pattern;
g_mutex_unlock (&interlace->lock);
gst_interlace_decorate_buffer_ts (interlace, buf, n_fields); gst_interlace_decorate_buffer_ts (interlace, buf, n_fields);
if (interlace->field_index == 0) { if (interlace->field_index == 0) {
@ -426,21 +437,20 @@ gst_interlace_decorate_buffer (GstInterlace * interlace, GstBuffer * buf,
if (n_fields == 1) { if (n_fields == 1) {
GST_BUFFER_FLAG_SET (buf, GST_VIDEO_BUFFER_FLAG_ONEFIELD); GST_BUFFER_FLAG_SET (buf, GST_VIDEO_BUFFER_FLAG_ONEFIELD);
} }
g_mutex_lock (&interlace->lock); if (pattern > GST_INTERLACE_PATTERN_2_2 && n_fields == 2 && interlaced) {
if (interlace->pattern > GST_INTERLACE_PATTERN_2_2 && n_fields == 2
&& interlaced) {
GST_BUFFER_FLAG_SET (buf, GST_VIDEO_BUFFER_FLAG_INTERLACED); GST_BUFFER_FLAG_SET (buf, GST_VIDEO_BUFFER_FLAG_INTERLACED);
} }
g_mutex_unlock (&interlace->lock);
} }
static const gchar * static const gchar *
interlace_mode_from_pattern (GstInterlace * interlace) interlace_mode_from_pattern (GstInterlace * interlace)
{ {
GstInterlacePattern pattern; GstInterlacePattern pattern;
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
pattern = interlace->pattern; pattern = interlace->pattern;
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
if (pattern > GST_INTERLACE_PATTERN_2_2) if (pattern > GST_INTERLACE_PATTERN_2_2)
return "mixed"; return "mixed";
else else
@ -470,7 +480,7 @@ gst_interlace_setcaps (GstInterlace * interlace, GstCaps * caps)
GstVideoInfo info, out_info; GstVideoInfo info, out_info;
GstCaps *othercaps, *src_peer_caps; GstCaps *othercaps, *src_peer_caps;
const PulldownFormat *pdformat; const PulldownFormat *pdformat;
gboolean alternate; gboolean top_field_first, alternate;
int i; int i;
int src_fps_n, src_fps_d; int src_fps_n, src_fps_d;
GstInterlacePattern pattern; GstInterlacePattern pattern;
@ -481,6 +491,7 @@ gst_interlace_setcaps (GstInterlace * interlace, GstCaps * caps)
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
interlace->pattern = interlace->new_pattern; interlace->pattern = interlace->new_pattern;
pattern = interlace->pattern; pattern = interlace->pattern;
top_field_first = interlace->top_field_first;
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
/* Check if downstream prefers alternate mode */ /* Check if downstream prefers alternate mode */
@ -528,14 +539,15 @@ gst_interlace_setcaps (GstInterlace * interlace, GstCaps * caps)
pdformat = &formats[pattern]; pdformat = &formats[pattern];
interlace->phase_index = interlace->pattern_offset;
src_fps_n = info.fps_n * pdformat->ratio_n; src_fps_n = info.fps_n * pdformat->ratio_n;
src_fps_d = info.fps_d * pdformat->ratio_d; src_fps_d = info.fps_d * pdformat->ratio_d;
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
interlace->phase_index = interlace->pattern_offset;
interlace->src_fps_n = src_fps_n; interlace->src_fps_n = src_fps_n;
interlace->src_fps_d = src_fps_d; interlace->src_fps_d = src_fps_d;
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
GST_DEBUG_OBJECT (interlace, "new framerate %d/%d", src_fps_n, src_fps_d); GST_DEBUG_OBJECT (interlace, "new framerate %d/%d", src_fps_n, src_fps_d);
if (alternate) { if (alternate) {
@ -592,8 +604,7 @@ gst_interlace_setcaps (GstInterlace * interlace, GstCaps * caps)
src_fps_d, NULL); src_fps_d, NULL);
if (pattern <= GST_INTERLACE_PATTERN_2_2 || alternate) { if (pattern <= GST_INTERLACE_PATTERN_2_2 || alternate) {
gst_caps_set_simple (othercaps, "field-order", G_TYPE_STRING, gst_caps_set_simple (othercaps, "field-order", G_TYPE_STRING,
interlace->top_field_first ? "top-field-first" : "bottom-field-first", top_field_first ? "top-field-first" : "bottom-field-first", NULL);
NULL);
} }
/* outcaps changed, regenerate out_info */ /* outcaps changed, regenerate out_info */
gst_video_info_from_caps (&out_info, othercaps); gst_video_info_from_caps (&out_info, othercaps);
@ -874,12 +885,14 @@ gst_interlace_getcaps (GstPad * pad, GstInterlace * interlace, GstCaps * filter)
const char *mode; const char *mode;
guint i; guint i;
gint pattern; gint pattern;
gboolean top_field_first;
otherpad = otherpad =
(pad == interlace->srcpad) ? interlace->sinkpad : interlace->srcpad; (pad == interlace->srcpad) ? interlace->sinkpad : interlace->srcpad;
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
pattern = interlace->new_pattern; pattern = interlace->new_pattern;
top_field_first = interlace->top_field_first;
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
if (filter != NULL) { if (filter != NULL) {
@ -929,8 +942,7 @@ gst_interlace_getcaps (GstPad * pad, GstInterlace * interlace, GstCaps * filter)
if (pad == interlace->srcpad) { if (pad == interlace->srcpad) {
gst_structure_set (s, "field-order", G_TYPE_STRING, gst_structure_set (s, "field-order", G_TYPE_STRING,
interlace->top_field_first ? "top-field-first" : top_field_first ? "top-field-first" : "bottom-field-first", NULL);
"bottom-field-first", NULL);
} else { } else {
gst_structure_remove_field (s, "field-order"); gst_structure_remove_field (s, "field-order");
} }
@ -1215,10 +1227,10 @@ gst_interlace_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
GstInterlace *interlace = GST_INTERLACE (parent); GstInterlace *interlace = GST_INTERLACE (parent);
GstFlowReturn ret = GST_FLOW_OK; GstFlowReturn ret = GST_FLOW_OK;
gint num_fields = 0; gint num_fields = 0;
guint current_fields; guint current_fields, pattern_offset;
const PulldownFormat *format; const PulldownFormat *format;
GstClockTime timestamp; GstClockTime timestamp;
gboolean alternate; gboolean allow_rff, top_field_first, alternate;
timestamp = GST_BUFFER_TIMESTAMP (buffer); timestamp = GST_BUFFER_TIMESTAMP (buffer);
@ -1236,6 +1248,13 @@ gst_interlace_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
return gst_pad_push (interlace->srcpad, buffer); return gst_pad_push (interlace->srcpad, buffer);
} }
g_mutex_lock (&interlace->lock);
format = &formats[interlace->pattern];
allow_rff = interlace->allow_rff;
pattern_offset = interlace->pattern_offset;
top_field_first = interlace->top_field_first;
g_mutex_unlock (&interlace->lock);
if (GST_BUFFER_FLAGS (buffer) & GST_BUFFER_FLAG_DISCONT) { if (GST_BUFFER_FLAGS (buffer) & GST_BUFFER_FLAG_DISCONT) {
GST_DEBUG ("discont"); GST_DEBUG ("discont");
@ -1245,7 +1264,7 @@ gst_interlace_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
interlace->stored_fields = 0; interlace->stored_fields = 0;
} }
if (interlace->top_field_first) { if (top_field_first) {
interlace->field_index = 0; interlace->field_index = 0;
} else { } else {
interlace->field_index = 1; interlace->field_index = 1;
@ -1257,12 +1276,8 @@ gst_interlace_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
interlace->timebase = timestamp; interlace->timebase = timestamp;
} }
g_mutex_lock (&interlace->lock);
format = &formats[interlace->pattern];
g_mutex_unlock (&interlace->lock);
if (interlace->stored_fields == 0 if (interlace->stored_fields == 0
&& interlace->phase_index == interlace->pattern_offset && interlace->phase_index == pattern_offset
&& GST_CLOCK_TIME_IS_VALID (timestamp)) { && GST_CLOCK_TIME_IS_VALID (timestamp)) {
interlace->timebase = timestamp; interlace->timebase = timestamp;
interlace->fields_since_timebase = 0; interlace->fields_since_timebase = 0;
@ -1370,7 +1385,7 @@ gst_interlace_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
gst_video_frame_unmap (&sframe); gst_video_frame_unmap (&sframe);
} }
if (num_fields >= 3 && interlace->allow_rff) { if (num_fields >= 3 && allow_rff) {
GST_DEBUG ("3 fields from current"); GST_DEBUG ("3 fields from current");
/* take both fields from incoming buffer */ /* take both fields from incoming buffer */
current_fields -= 3; current_fields -= 3;
@ -1459,26 +1474,35 @@ gst_interlace_set_property (GObject * object,
switch (prop_id) { switch (prop_id) {
case PROP_TOP_FIELD_FIRST: case PROP_TOP_FIELD_FIRST:
g_mutex_lock (&interlace->lock);
interlace->top_field_first = g_value_get_boolean (value); interlace->top_field_first = g_value_get_boolean (value);
g_mutex_unlock (&interlace->lock);
break; break;
case PROP_PATTERN:{ case PROP_PATTERN:{
gint pattern = g_value_get_enum (value); gint pattern = g_value_get_enum (value);
gboolean reconfigure = FALSE;
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
interlace->new_pattern = pattern; interlace->new_pattern = pattern;
if (pattern == interlace->pattern || interlace->src_fps_n == 0) { if (interlace->src_fps_n == 0 || interlace->pattern == pattern)
interlace->pattern = pattern; interlace->pattern = pattern;
else
reconfigure = TRUE;
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
} else {
g_mutex_unlock (&interlace->lock); if (reconfigure)
gst_pad_push_event (interlace->sinkpad, gst_event_new_reconfigure ()); gst_pad_push_event (interlace->sinkpad, gst_event_new_reconfigure ());
}
break; break;
} }
case PROP_PATTERN_OFFSET: case PROP_PATTERN_OFFSET:
g_mutex_lock (&interlace->lock);
interlace->pattern_offset = g_value_get_uint (value); interlace->pattern_offset = g_value_get_uint (value);
g_mutex_unlock (&interlace->lock);
break; break;
case PROP_ALLOW_RFF: case PROP_ALLOW_RFF:
g_mutex_lock (&interlace->lock);
interlace->allow_rff = g_value_get_boolean (value); interlace->allow_rff = g_value_get_boolean (value);
g_mutex_unlock (&interlace->lock);
break; break;
default: default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
@ -1494,7 +1518,9 @@ gst_interlace_get_property (GObject * object,
switch (prop_id) { switch (prop_id) {
case PROP_TOP_FIELD_FIRST: case PROP_TOP_FIELD_FIRST:
g_mutex_lock (&interlace->lock);
g_value_set_boolean (value, interlace->top_field_first); g_value_set_boolean (value, interlace->top_field_first);
g_mutex_unlock (&interlace->lock);
break; break;
case PROP_PATTERN: case PROP_PATTERN:
g_mutex_lock (&interlace->lock); g_mutex_lock (&interlace->lock);
@ -1502,10 +1528,14 @@ gst_interlace_get_property (GObject * object,
g_mutex_unlock (&interlace->lock); g_mutex_unlock (&interlace->lock);
break; break;
case PROP_PATTERN_OFFSET: case PROP_PATTERN_OFFSET:
g_mutex_lock (&interlace->lock);
g_value_set_uint (value, interlace->pattern_offset); g_value_set_uint (value, interlace->pattern_offset);
g_mutex_unlock (&interlace->lock);
break; break;
case PROP_ALLOW_RFF: case PROP_ALLOW_RFF:
g_mutex_lock (&interlace->lock);
g_value_set_boolean (value, interlace->allow_rff); g_value_set_boolean (value, interlace->allow_rff);
g_mutex_unlock (&interlace->lock);
break; break;
default: default:
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);