rgvolume: make tag list writable before modifying it

Making the event itself writable is not enough, it won't make
the actual taglist in the event writable as well. Instead, just
make a copy of the taglist and then create a new tag event from
that if required, replacing the old one. Before we would
inadvertently modify taglists upstream elements might still
be holding on to. Add unit test for this as well.

https://bugzilla.gnome.org/show_bug.cgi?id=762793

2 files changed, 11 insertions, 6 deletions

diff --git a/gst/replaygain/gstrgvolume.c b/gst/replaygain/gstrgvolume.c
index beb8fda4c..a53f4d60a 100644
--- a/gst/replaygain/gstrgvolume.c
+++ b/gst/replaygain/gstrgvolume.c
@@ -559,9 +559,7 @@ gst_rg_volume_tag_event (GstRgVolume * self, GstEvent * event)
   self->has_album_gain |= has_album_gain;
   self->has_album_peak |= has_album_peak;
 
-  event = (GstEvent *) gst_mini_object_make_writable (GST_MINI_OBJECT (event));
-  gst_event_parse_tag (event, &tag_list);
-
+  tag_list = gst_tag_list_copy (tag_list);
   gst_tag_list_remove_tag (tag_list, GST_TAG_TRACK_GAIN);
   gst_tag_list_remove_tag (tag_list, GST_TAG_TRACK_PEAK);
   gst_tag_list_remove_tag (tag_list, GST_TAG_ALBUM_GAIN);
@@ -570,12 +568,13 @@ gst_rg_volume_tag_event (GstRgVolume * self, GstEvent * event)
 
   gst_rg_volume_update_gain (self);
 
+  gst_event_unref (event);
   if (gst_tag_list_is_empty (tag_list)) {
-    gst_event_unref (event);
-    event = NULL;
+    gst_tag_list_unref (tag_list);
+    return NULL;
   }
 
-  return event;
+  return gst_event_new_tag (tag_list);
 }
 
 static void
diff --git a/tests/check/elements/rgvolume.c b/tests/check/elements/rgvolume.c
index b02fcc17f..c8ae8bc2b 100644
--- a/tests/check/elements/rgvolume.c
+++ b/tests/check/elements/rgvolume.c
@@ -383,8 +383,14 @@ GST_START_TEST (test_events)
       GST_TAG_TRACK_GAIN, +4.95, GST_TAG_TRACK_PEAK, 0.59463,
       GST_TAG_ALBUM_GAIN, -1.54, GST_TAG_ALBUM_PEAK, 0.693415,
       GST_TAG_ARTIST, "Foobar", NULL);
+  gst_tag_list_ref (tag_list);
   event = gst_event_new_tag (tag_list);
   new_event = send_tag_event (element, event);
+
+  /* Make sure our tags weren't modified in place while we still got a ref */
+  fail_unless_equals_int (5, gst_tag_list_n_tags (tag_list));
+  gst_tag_list_unref (tag_list);
+
   gst_event_parse_tag (new_event, &tag_list);
   fail_unless (gst_tag_list_get_string (tag_list, GST_TAG_ARTIST, &artist));
   fail_unless (g_str_equal (artist, "Foobar"));