audioencoder: fix tag handling

Merge upstream tags with encoder tags and update whenever
any of those changes.

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

diff --git a/gst-libs/gst/audio/gstaudioencoder.c b/gst-libs/gst/audio/gstaudioencoder.c
index 9aa442e..2ff8430 100644 (file)
--- a/gst-libs/gst/audio/gstaudioencoder.c
+++ b/gst-libs/gst/audio/gstaudioencoder.c
@@ -258,9 +258,15 @@ struct _GstAudioEncoderPrivate
   gboolean hard_min;
   gboolean drainable;
 
-  /* pending tags */
+  /* upstream stream tags (global tags are passed through as-is) */
+  GstTagList *upstream_tags;
+
+  /* subclass tags */
   GstTagList *tags;
+  GstTagMergeMode tags_merge_mode;
+
   gboolean tags_changed;
+
   /* pending serialized sink events, will be sent from finish_frame() */
   GList *pending_events;
 };
@@ -490,9 +496,14 @@ gst_audio_encoder_reset (GstAudioEncoder * enc, gboolean full)
     memset (&enc->priv->ctx, 0, sizeof (enc->priv->ctx));
     gst_audio_info_init (&enc->priv->ctx.info);
 
+    if (enc->priv->upstream_tags) {
+      gst_tag_list_unref (enc->priv->upstream_tags);
+      enc->priv->upstream_tags = NULL;
+    }
     if (enc->priv->tags)
       gst_tag_list_unref (enc->priv->tags);
     enc->priv->tags = NULL;
+    enc->priv->tags_merge_mode = GST_TAG_MERGE_APPEND;
     enc->priv->tags_changed = FALSE;
 
     g_list_foreach (enc->priv->pending_events, (GFunc) gst_event_unref, NULL);
@@ -611,28 +622,50 @@ gst_audio_encoder_push_pending_events (GstAudioEncoder * enc)
   }
 }
 
-static inline void
-gst_audio_encoder_check_and_push_ending_tags (GstAudioEncoder * enc)
+static GstEvent *
+gst_audio_encoder_create_merged_tags_event (GstAudioEncoder * enc)
 {
-  if (G_UNLIKELY (enc->priv->tags && enc->priv->tags_changed)) {
-#if 0
-    GstCaps *caps;
-#endif
+  GstTagList *merged_tags;
+
+  GST_LOG_OBJECT (enc, "upstream : %" GST_PTR_FORMAT, enc->priv->upstream_tags);
+  GST_LOG_OBJECT (enc, "encoder  : %" GST_PTR_FORMAT, enc->priv->tags);
+  GST_LOG_OBJECT (enc, "mode     : %d", enc->priv->tags_merge_mode);
+
+  merged_tags =
+      gst_tag_list_merge (enc->priv->upstream_tags, enc->priv->tags,
+      enc->priv->tags_merge_mode);
+
+  GST_DEBUG_OBJECT (enc, "merged   : %" GST_PTR_FORMAT, merged_tags);
+
+  if (merged_tags == NULL)
+    return NULL;
 
-    /* add codec info to pending tags */
+  if (gst_tag_list_is_empty (merged_tags)) {
+    gst_tag_list_unref (merged_tags);
+    return NULL;
+  }
+
+  /* add codec info to pending tags */
 #if 0
-    if (!enc->priv->tags)
-      enc->priv->tags = gst_tag_list_new ();
-    enc->priv->tags = gst_tag_list_make_writable (enc->priv->tags);
-    caps = gst_pad_get_current_caps (enc->srcpad);
-    gst_pb_utils_add_codec_description_to_tag_list (enc->priv->tags,
-        GST_TAG_CODEC, caps);
-    gst_pb_utils_add_codec_description_to_tag_list (enc->priv->tags,
-        GST_TAG_AUDIO_CODEC, caps);
+  caps = gst_pad_get_current_caps (enc->srcpad);
+  gst_pb_utils_add_codec_description_to_tag_list (merged_tags,
+      GST_TAG_AUDIO_CODEC, caps);
 #endif
-    GST_DEBUG_OBJECT (enc, "sending tags %" GST_PTR_FORMAT, enc->priv->tags);
-    gst_audio_encoder_push_event (enc,
-        gst_event_new_tag (gst_tag_list_ref (enc->priv->tags)));
+
+  return gst_event_new_tag (merged_tags);
+}
+
+static void
+gst_audio_encoder_check_and_push_pending_tags (GstAudioEncoder * enc)
+{
+  if (enc->priv->tags_changed) {
+    GstEvent *tags_event;
+
+    tags_event = gst_audio_encoder_create_merged_tags_event (enc);
+
+    if (tags_event != NULL)
+      gst_audio_encoder_push_event (enc, tags_event);
+
     enc->priv->tags_changed = FALSE;
   }
 }
@@ -760,8 +793,8 @@ gst_audio_encoder_finish_frame (GstAudioEncoder * enc, GstBuffer * buf,
 
   gst_audio_encoder_push_pending_events (enc);
 
-  /* send after pending events, which likely includes newsegment event */
-  gst_audio_encoder_check_and_push_ending_tags (enc);
+  /* send after pending events, which likely includes segment event */
+  gst_audio_encoder_check_and_push_pending_tags (enc);
 
   /* remove corresponding samples from input */
   if (samples < 0)
@@ -1540,7 +1573,7 @@ gst_audio_encoder_sink_event_default (GstAudioEncoder * enc, GstEvent * event)
 
       /* check for pending events and tags */
       gst_audio_encoder_push_pending_events (enc);
-      gst_audio_encoder_check_and_push_ending_tags (enc);
+      gst_audio_encoder_check_and_push_pending_tags (enc);
 
       GST_AUDIO_ENCODER_STREAM_UNLOCK (enc);
 
@@ -1561,6 +1594,21 @@ gst_audio_encoder_sink_event_default (GstAudioEncoder * enc, GstEvent * event)
       break;
     }
 
+    case GST_EVENT_STREAM_START:
+    {
+      GST_AUDIO_ENCODER_STREAM_LOCK (enc);
+      /* Flush upstream tags after a STREAM_START */
+      GST_DEBUG_OBJECT (enc, "received STREAM_START. Clearing taglist");
+      if (enc->priv->upstream_tags) {
+        gst_tag_list_unref (enc->priv->upstream_tags);
+        enc->priv->upstream_tags = NULL;
+        enc->priv->tags_changed = TRUE;
+      }
+      GST_AUDIO_ENCODER_STREAM_UNLOCK (enc);
+      res = gst_audio_encoder_push_event (enc, event);
+      break;
+    }
+
     case GST_EVENT_TAG:
     {
       GstTagList *tags;
@@ -1568,31 +1616,34 @@ gst_audio_encoder_sink_event_default (GstAudioEncoder * enc, GstEvent * event)
       gst_event_parse_tag (event, &tags);
 
       if (gst_tag_list_get_scope (tags) == GST_TAG_SCOPE_STREAM) {
-        tags = gst_tag_list_copy (tags);
-
-        /* FIXME: make generic based on GST_TAG_FLAG_ENCODED */
-        gst_tag_list_remove_tag (tags, GST_TAG_CODEC);
-        gst_tag_list_remove_tag (tags, GST_TAG_AUDIO_CODEC);
-        gst_tag_list_remove_tag (tags, GST_TAG_VIDEO_CODEC);
-        gst_tag_list_remove_tag (tags, GST_TAG_SUBTITLE_CODEC);
-        gst_tag_list_remove_tag (tags, GST_TAG_CONTAINER_FORMAT);
-        gst_tag_list_remove_tag (tags, GST_TAG_BITRATE);
-        gst_tag_list_remove_tag (tags, GST_TAG_NOMINAL_BITRATE);
-        gst_tag_list_remove_tag (tags, GST_TAG_MAXIMUM_BITRATE);
-        gst_tag_list_remove_tag (tags, GST_TAG_MINIMUM_BITRATE);
-        gst_tag_list_remove_tag (tags, GST_TAG_ENCODER);
-        gst_tag_list_remove_tag (tags, GST_TAG_ENCODER_VERSION);
-
-        gst_audio_encoder_merge_tags (enc, tags, GST_TAG_MERGE_REPLACE);
-        gst_tag_list_unref (tags);
+        GST_AUDIO_ENCODER_STREAM_LOCK (enc);
+        if (enc->priv->upstream_tags != tags) {
+          tags = gst_tag_list_copy (tags);
+
+          /* FIXME: make generic based on GST_TAG_FLAG_ENCODED */
+          gst_tag_list_remove_tag (tags, GST_TAG_CODEC);
+          gst_tag_list_remove_tag (tags, GST_TAG_AUDIO_CODEC);
+          gst_tag_list_remove_tag (tags, GST_TAG_VIDEO_CODEC);
+          gst_tag_list_remove_tag (tags, GST_TAG_SUBTITLE_CODEC);
+          gst_tag_list_remove_tag (tags, GST_TAG_CONTAINER_FORMAT);
+          gst_tag_list_remove_tag (tags, GST_TAG_BITRATE);
+          gst_tag_list_remove_tag (tags, GST_TAG_NOMINAL_BITRATE);
+          gst_tag_list_remove_tag (tags, GST_TAG_MAXIMUM_BITRATE);
+          gst_tag_list_remove_tag (tags, GST_TAG_MINIMUM_BITRATE);
+          gst_tag_list_remove_tag (tags, GST_TAG_ENCODER);
+          gst_tag_list_remove_tag (tags, GST_TAG_ENCODER_VERSION);
+
+          if (enc->priv->upstream_tags)
+            gst_tag_list_unref (enc->priv->upstream_tags);
+          enc->priv->upstream_tags = tags;
+          GST_INFO_OBJECT (enc, "upstream stream tags: %" GST_PTR_FORMAT, tags);
+        }
         gst_event_unref (event);
-        event = NULL;
-        res = TRUE;
-        break;
+        event = gst_audio_encoder_create_merged_tags_event (enc);
+        GST_AUDIO_ENCODER_STREAM_UNLOCK (enc);
       }
       /* fall through */
     }
-
     default:
       /* Forward non-serialized events immediately. */
       if (!GST_EVENT_IS_SERIALIZED (event)) {
@@ -2591,16 +2642,16 @@ gst_audio_encoder_get_drainable (GstAudioEncoder * enc)
 /**
  * gst_audio_encoder_merge_tags:
  * @enc: a #GstAudioEncoder
- * @tags: a #GstTagList to merge
- * @mode: the #GstTagMergeMode to use
+ * @tags: (allow-none): a #GstTagList to merge, or NULL to unset
+ *     previously-set tags
+ * @mode: the #GstTagMergeMode to use, usually #GST_TAG_MERGE_REPLACE
  *
- * Adds tags to so-called pending tags, which will be processed
- * before pushing out data downstream.
+ * Sets the audio encoder tags and how they should be merged with any
+ * upstream stream tags. This will override any tags previously-set
+ * with gst_audio_encoder_merge_tags().
  *
  * 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,
- * although it should be aware that baseclass already takes care
- * of the usual CODEC/AUDIO_CODEC tags.
+ * not required to use this and can still do tag handling on its own.
  *
  * MT safe.
  */
@@ -2608,19 +2659,25 @@ void
 gst_audio_encoder_merge_tags (GstAudioEncoder * enc,
     const GstTagList * tags, GstTagMergeMode mode)
 {
-  GstTagList *otags;
-
   g_return_if_fail (GST_IS_AUDIO_ENCODER (enc));
   g_return_if_fail (tags == NULL || GST_IS_TAG_LIST (tags));
+  g_return_if_fail (tags == NULL || mode != GST_TAG_MERGE_UNDEFINED);
 
   GST_AUDIO_ENCODER_STREAM_LOCK (enc);
-  if (tags)
-    GST_DEBUG_OBJECT (enc, "merging tags %" GST_PTR_FORMAT, tags);
-  otags = enc->priv->tags;
-  enc->priv->tags = gst_tag_list_merge (enc->priv->tags, tags, mode);
-  if (otags)
-    gst_tag_list_unref (otags);
-  enc->priv->tags_changed = TRUE;
+  if (enc->priv->tags != tags) {
+    if (enc->priv->tags) {
+      gst_tag_list_unref (enc->priv->tags);
+      enc->priv->tags = NULL;
+      enc->priv->tags_merge_mode = GST_TAG_MERGE_APPEND;
+    }
+    if (tags) {
+      enc->priv->tags = gst_tag_list_ref ((GstTagList *) tags);
+      enc->priv->tags_merge_mode = mode;
+    }
+
+    GST_DEBUG_OBJECT (enc, "setting encoder tags to %" GST_PTR_FORMAT, tags);
+    enc->priv->tags_changed = TRUE;
+  }
   GST_AUDIO_ENCODER_STREAM_UNLOCK (enc);
 }