audiodecoder: fix tag handling

Before we just merged everything in pretty much random ways
ad-hoc instead of keeping state properly. In 0.10 that was
how it worked, but in 1.x the tag events sent should always
reflect the latest state and replace any previous tags.

So save the upstream (stream) tags, and save the tags set
by the decoder subclass with merge mode, and then update
the merged tags whenever either of those two changes.

This slightly changes the behaviour of gst_audio_decoder_merge_tags()
in case it is called multiple times, since now any call replaces
the previously-set tags. However, it leads to much more predictable
outcomes, and also we are not aware of any subclass which sets this
multiple times and expects all the tags set to be merged.

If more complex tag merging scenarios are required, we'll have
to add a new vfunc for that or the subclass has to intercept
the upstream tags itself and send merged tags itself.

https://bugzilla.gnome.org/show_bug.cgi?id=679768
This commit is contained in:
Tim-Philipp Müller 2015-08-14 17:44:59 +01:00
parent 14867e4ebb
commit 5ccc8432e0

View file

@ -252,9 +252,15 @@ struct _GstAudioDecoderPrivate
guint sync_flush; guint sync_flush;
/* error count */ /* error count */
gint error_count; gint error_count;
/* codec id tag */
GstTagList *taglist; /* upstream stream tags (global tags are passed through as-is) */
gboolean taglist_changed; GstTagList *upstream_tags;
/* subclass tags */
GstTagList *taglist; /* FIXME: rename to decoder_tags */
GstTagMergeMode decoder_tags_merge_mode;
gboolean taglist_changed; /* FIXME: rename to tags_changed */
/* whether circumstances allow output aggregation */ /* whether circumstances allow output aggregation */
gint agg; gint agg;
@ -564,6 +570,11 @@ gst_audio_decoder_reset (GstAudioDecoder * dec, gboolean full)
gst_tag_list_unref (dec->priv->taglist); gst_tag_list_unref (dec->priv->taglist);
dec->priv->taglist = NULL; dec->priv->taglist = NULL;
} }
dec->priv->decoder_tags_merge_mode = GST_TAG_MERGE_KEEP_ALL;
if (dec->priv->upstream_tags) {
gst_tag_list_unref (dec->priv->upstream_tags);
dec->priv->upstream_tags = NULL;
}
dec->priv->taglist_changed = FALSE; dec->priv->taglist_changed = FALSE;
gst_segment_init (&dec->input_segment, GST_FORMAT_TIME); gst_segment_init (&dec->input_segment, GST_FORMAT_TIME);
@ -624,6 +635,32 @@ gst_audio_decoder_finalize (GObject * object)
G_OBJECT_CLASS (parent_class)->finalize (object); G_OBJECT_CLASS (parent_class)->finalize (object);
} }
static GstEvent *
gst_audio_decoder_create_merged_tags_event (GstAudioDecoder * dec)
{
GstTagList *merged_tags;
GST_LOG_OBJECT (dec, "upstream : %" GST_PTR_FORMAT, dec->priv->upstream_tags);
GST_LOG_OBJECT (dec, "decoder : %" GST_PTR_FORMAT, dec->priv->taglist);
GST_LOG_OBJECT (dec, "mode : %d", dec->priv->decoder_tags_merge_mode);
merged_tags =
gst_tag_list_merge (dec->priv->upstream_tags,
dec->priv->taglist, dec->priv->decoder_tags_merge_mode);
GST_DEBUG_OBJECT (dec, "merged : %" GST_PTR_FORMAT, merged_tags);
if (merged_tags == NULL)
return NULL;
if (gst_tag_list_is_empty (merged_tags)) {
gst_tag_list_unref (merged_tags);
return NULL;
}
return gst_event_new_tag (merged_tags);
}
static gboolean static gboolean
gst_audio_decoder_push_event (GstAudioDecoder * dec, GstEvent * event) gst_audio_decoder_push_event (GstAudioDecoder * dec, GstEvent * event)
{ {
@ -1369,10 +1406,13 @@ gst_audio_decoder_finish_frame (GstAudioDecoder * dec, GstBuffer * buf,
/* delayed one-shot stuff until confirmed data */ /* delayed one-shot stuff until confirmed data */
if (priv->taglist && priv->taglist_changed) { if (priv->taglist && priv->taglist_changed) {
GST_DEBUG_OBJECT (dec, "codec tag %" GST_PTR_FORMAT, priv->taglist); GstEvent *tags_event;
if (!gst_tag_list_is_empty (priv->taglist))
gst_audio_decoder_push_event (dec, tags_event = gst_audio_decoder_create_merged_tags_event (dec);
gst_event_new_tag (gst_tag_list_ref (priv->taglist)));
if (tags_event != NULL)
gst_audio_decoder_push_event (dec, tags_event);
priv->taglist_changed = FALSE; priv->taglist_changed = FALSE;
} }
@ -2158,12 +2198,12 @@ gst_audio_decoder_sink_eventfunc (GstAudioDecoder * dec, GstEvent * event)
gst_audio_decoder_flush (dec, FALSE); gst_audio_decoder_flush (dec, FALSE);
GST_DEBUG_OBJECT (dec, "received STREAM_START. Clearing taglist"); GST_DEBUG_OBJECT (dec, "received STREAM_START. Clearing taglist");
/* Flush our merged taglist after a STREAM_START */ /* Flush upstream tags after a STREAM_START */
if (dec->priv->taglist) { if (dec->priv->upstream_tags) {
gst_tag_list_unref (dec->priv->taglist); gst_tag_list_unref (dec->priv->upstream_tags);
dec->priv->taglist = NULL; dec->priv->upstream_tags = NULL;
} }
dec->priv->taglist_changed = FALSE; dec->priv->taglist_changed = TRUE;
GST_AUDIO_DECODER_STREAM_UNLOCK (dec); GST_AUDIO_DECODER_STREAM_UNLOCK (dec);
ret = gst_audio_decoder_push_event (dec, event); ret = gst_audio_decoder_push_event (dec, event);
@ -2292,11 +2332,16 @@ gst_audio_decoder_sink_eventfunc (GstAudioDecoder * dec, GstEvent * event)
gst_event_parse_tag (event, &tags); gst_event_parse_tag (event, &tags);
if (gst_tag_list_get_scope (tags) == GST_TAG_SCOPE_STREAM) { if (gst_tag_list_get_scope (tags) == GST_TAG_SCOPE_STREAM) {
gst_audio_decoder_merge_tags (dec, tags, GST_TAG_MERGE_REPLACE); GST_AUDIO_DECODER_STREAM_LOCK (dec);
if (dec->priv->upstream_tags != tags) {
if (dec->priv->upstream_tags)
gst_tag_list_unref (dec->priv->upstream_tags);
dec->priv->upstream_tags = gst_tag_list_ref (tags);
GST_INFO_OBJECT (dec, "upstream stream tags: %" GST_PTR_FORMAT, tags);
}
gst_event_unref (event); gst_event_unref (event);
event = NULL; event = gst_audio_decoder_create_merged_tags_event (dec);
ret = TRUE; GST_AUDIO_DECODER_STREAM_UNLOCK (dec);
break;
} }
/* fall through */ /* fall through */
@ -3500,36 +3545,39 @@ gst_audio_decoder_get_needs_format (GstAudioDecoder * dec)
/** /**
* gst_audio_decoder_merge_tags: * gst_audio_decoder_merge_tags:
* @dec: a #GstAudioDecoder * @dec: a #GstAudioDecoder
* @tags: a #GstTagList to merge * @tags: (allow-none): a #GstTagList to merge, or NULL
* @mode: the #GstTagMergeMode to use * @mode: the #GstTagMergeMode to use, usually #GST_TAG_MERGE_REPLACE
* *
* Adds tags to so-called pending tags, which will be processed * Sets the audio decoder tags and how they should be merged with any
* before pushing out data downstream. * upstream stream tags. This will override any tags previously-set
* with gst_audio_decoder_merge_tags().
* *
* Note that this is provided for convenience, and the subclass is * Note that this is provided for convenience, and the subclass is
* not required to use this and can still do tag handling on its own, * not required to use this and can still do tag handling on its own.
* although it should be aware that baseclass already takes care
* of the usual CODEC/AUDIO_CODEC tags.
*
* MT safe.
*/ */
void void
gst_audio_decoder_merge_tags (GstAudioDecoder * dec, gst_audio_decoder_merge_tags (GstAudioDecoder * dec,
const GstTagList * tags, GstTagMergeMode mode) const GstTagList * tags, GstTagMergeMode mode)
{ {
GstTagList *otags;
g_return_if_fail (GST_IS_AUDIO_DECODER (dec)); g_return_if_fail (GST_IS_AUDIO_DECODER (dec));
g_return_if_fail (tags == NULL || GST_IS_TAG_LIST (tags)); g_return_if_fail (tags == NULL || GST_IS_TAG_LIST (tags));
g_return_if_fail (mode != GST_TAG_MERGE_UNDEFINED);
GST_AUDIO_DECODER_STREAM_LOCK (dec); GST_AUDIO_DECODER_STREAM_LOCK (dec);
if (tags) if (dec->priv->taglist != tags) {
GST_DEBUG_OBJECT (dec, "merging tags %" GST_PTR_FORMAT, tags); if (dec->priv->taglist) {
otags = dec->priv->taglist; gst_tag_list_unref (dec->priv->taglist);
dec->priv->taglist = gst_tag_list_merge (dec->priv->taglist, tags, mode); dec->priv->taglist = NULL;
if (otags) dec->priv->decoder_tags_merge_mode = GST_TAG_MERGE_KEEP_ALL;
gst_tag_list_unref (otags); }
dec->priv->taglist_changed = TRUE; if (tags) {
dec->priv->taglist = gst_tag_list_ref ((GstTagList *) tags);
dec->priv->decoder_tags_merge_mode = mode;
}
GST_DEBUG_OBJECT (dec, "setting decoder tags to %" GST_PTR_FORMAT, tags);
dec->priv->taglist_changed = TRUE;
}
GST_AUDIO_DECODER_STREAM_UNLOCK (dec); GST_AUDIO_DECODER_STREAM_UNLOCK (dec);
} }