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
This commit is contained in:
Tim-Philipp Müller 2016-02-28 13:42:28 +00:00
parent bf5a72a6dd
commit a4d64b5caa
2 changed files with 11 additions and 6 deletions

View file

@ -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

View file

@ -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"));