From 0cee3cd8330dfa731bacc7838d2c09422eaa1425 Mon Sep 17 00:00:00 2001 From: Camilo Celis Guzman Date: Thu, 4 May 2023 02:41:09 +0900 Subject: [PATCH] rtpvp8pay: rtpvp9pay: access picture_id property atomically Atomically set and get the picture_id. This changeset only atomically gets the picture-id when such property is queried on the element, on every other place where it is accessed internally it is accessed directly. This is because there is no MT scenario where we would be modifying this value and reading it internally in parallel. Part-of: --- .../gst-plugins-good/gst/rtp/gstrtpvp8pay.c | 45 +++++++++++-------- .../gst-plugins-good/gst/rtp/gstrtpvp8pay.h | 2 +- .../gst-plugins-good/gst/rtp/gstrtpvp9pay.c | 34 +++++++------- .../gst-plugins-good/gst/rtp/gstrtpvp9pay.h | 2 +- 4 files changed, 46 insertions(+), 37 deletions(-) diff --git a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c index 8d3221911d..0b47c17f88 100644 --- a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c +++ b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.c @@ -109,34 +109,41 @@ picture_id_field_len (GstVP8RtpPayPictureIDMode mode) } static void -gst_rtp_vp8_pay_picture_id_reset (GstRtpVP8Pay * obj) +gst_rtp_vp8_pay_picture_id_reset (GstRtpVP8Pay * self) { gint nbits; + gint old_picture_id = self->picture_id; + gint picture_id = 0; - if (obj->picture_id_mode == VP8_PAY_NO_PICTURE_ID) { - obj->picture_id = 0; - } else { - if (obj->picture_id_offset == -1) - obj->picture_id = g_random_int (); - else - obj->picture_id = obj->picture_id_offset; - - nbits = picture_id_field_len (obj->picture_id_mode); - obj->picture_id &= (1 << nbits) - 1; + if (self->picture_id_mode != VP8_PAY_NO_PICTURE_ID) { + if (self->picture_id_offset == -1) { + picture_id = g_random_int (); + } else { + picture_id = self->picture_id_offset; + } + nbits = picture_id_field_len (self->picture_id_mode); + picture_id &= (1 << nbits) - 1; } + g_atomic_int_set (&self->picture_id, picture_id); + + GST_LOG_OBJECT (self, "picture-id reset %d -> %d", + old_picture_id, picture_id); } static void -gst_rtp_vp8_pay_picture_id_increment (GstRtpVP8Pay * obj) +gst_rtp_vp8_pay_picture_id_increment (GstRtpVP8Pay * self) { gint nbits; - if (obj->picture_id_mode == VP8_PAY_NO_PICTURE_ID) + if (self->picture_id_mode == VP8_PAY_NO_PICTURE_ID) return; - nbits = picture_id_field_len (obj->picture_id_mode); - obj->picture_id++; - obj->picture_id &= (1 << nbits) - 1; + /* Atomically increment and wrap the picture id if it overflows */ + nbits = picture_id_field_len (self->picture_id_mode); + gint picture_id = g_atomic_int_get (&self->picture_id); + picture_id++; + picture_id &= (1 << nbits) - 1; + g_atomic_int_set (&self->picture_id, picture_id); } static void @@ -247,7 +254,7 @@ gst_rtp_vp8_pay_get_property (GObject * object, switch (prop_id) { case PROP_PICTURE_ID: - g_value_set_int (value, rtpvp8pay->picture_id); + g_value_set_int (value, g_atomic_int_get (&rtpvp8pay->picture_id)); break; case PROP_PICTURE_ID_MODE: g_value_set_enum (value, rtpvp8pay->picture_id_mode); @@ -711,9 +718,9 @@ gst_rtp_vp8_pay_sink_event (GstRTPBasePayload * payload, GstEvent * event) GstEventType event_type = GST_EVENT_TYPE (event); if (event_type == GST_EVENT_GAP || event_type == GST_EVENT_FLUSH_START) { - guint picture_id = self->picture_id; + gint picture_id = self->picture_id; gst_rtp_vp8_pay_picture_id_increment (self); - GST_DEBUG_OBJECT (payload, "Incrementing picture ID on %s event %u->%u", + GST_DEBUG_OBJECT (payload, "Incrementing picture ID on %s event %d -> %d", GST_EVENT_TYPE_NAME (event), picture_id, self->picture_id); } diff --git a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.h b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.h index 851e3f736e..748bec1650 100644 --- a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.h +++ b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp8pay.h @@ -63,7 +63,7 @@ struct _GstRtpVP8Pay guint partition_size[9]; GstVP8RtpPayPictureIDMode picture_id_mode; gint picture_id_offset; - guint16 picture_id; + gint picture_id; gboolean temporal_scalability_fields_present; guint8 tl0picidx; }; diff --git a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c index 42b96b198e..153c56de41 100644 --- a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c +++ b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.c @@ -113,22 +113,22 @@ static void gst_rtp_vp9_pay_picture_id_reset (GstRtpVP9Pay * self) { gint nbits; - guint16 old_picture_id = self->picture_id; + gint old_picture_id = self->picture_id; + gint picture_id = 0; - if (self->picture_id_mode == VP9_PAY_NO_PICTURE_ID) { - self->picture_id = 0; - } else { - if (self->picture_id_offset == DEFAULT_PICTURE_ID_OFFSET) { - self->picture_id = g_random_int (); + if (self->picture_id_mode != VP9_PAY_NO_PICTURE_ID) { + if (self->picture_id_offset == -1) { + picture_id = g_random_int (); } else { - self->picture_id = self->picture_id_offset; + picture_id = self->picture_id_offset; } nbits = picture_id_field_len (self->picture_id_mode); - self->picture_id &= (1 << nbits) - 1; + picture_id &= (1 << nbits) - 1; } + g_atomic_int_set (&self->picture_id, picture_id); - GST_LOG_OBJECT (self, "picture-id reset %u -> %u", - old_picture_id, self->picture_id); + GST_LOG_OBJECT (self, "picture-id reset %d -> %d", + old_picture_id, picture_id); } static void @@ -229,7 +229,7 @@ gst_rtp_vp9_pay_get_property (GObject * object, switch (prop_id) { case PROP_PICTURE_ID: - g_value_set_int (value, rtpvp9pay->picture_id); + g_value_set_int (value, g_atomic_int_get (&rtpvp9pay->picture_id)); break; case PROP_PICTURE_ID_MODE: g_value_set_enum (value, rtpvp9pay->picture_id_mode); @@ -545,10 +545,12 @@ gst_rtp_vp9_pay_picture_id_increment (GstRtpVP9Pay * self) if (self->picture_id_mode == VP9_PAY_NO_PICTURE_ID) return; - /* Increment and wrap the picture id if it overflows */ + /* Atomically increment and wrap the picture id if it overflows */ nbits = picture_id_field_len (self->picture_id_mode); - self->picture_id++; - self->picture_id &= (1 << nbits) - 1; + gint picture_id = g_atomic_int_get (&self->picture_id); + picture_id++; + picture_id &= (1 << nbits) - 1; + g_atomic_int_set (&self->picture_id, picture_id); } static GstFlowReturn @@ -601,9 +603,9 @@ gst_rtp_vp9_pay_sink_event (GstRTPBasePayload * payload, GstEvent * event) GstEventType event_type = GST_EVENT_TYPE (event); if (event_type == GST_EVENT_GAP || event_type == GST_EVENT_FLUSH_START) { - guint picture_id = self->picture_id; + gint picture_id = self->picture_id; gst_rtp_vp9_pay_picture_id_increment (self); - GST_DEBUG_OBJECT (payload, "Incrementing picture ID on %s event %u->%u", + GST_DEBUG_OBJECT (payload, "Incrementing picture ID on %s event %d -> %d", GST_EVENT_TYPE_NAME (event), picture_id, self->picture_id); } diff --git a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.h b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.h index b9d966d08b..b2ad88445f 100644 --- a/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.h +++ b/subprojects/gst-plugins-good/gst/rtp/gstrtpvp9pay.h @@ -61,7 +61,7 @@ struct _GstRtpVP9Pay guint height; GstVP9RtpPayPictureIDMode picture_id_mode; gint picture_id_offset; - guint16 picture_id; + gint picture_id; }; GType gst_rtp_vp9_pay_get_type (void);