discoverer: Advertise container-specific tags with a new API
authorPhilippe Normand <philn@igalia.com>
Sun, 10 Oct 2021 16:07:33 +0000 (17:07 +0100)
committerGStreamer Marge Bot <gitlab-merge-bot@gstreamer-foundation.org>
Mon, 18 Oct 2021 20:08:35 +0000 (20:08 +0000)
Since commit a55dafe341ac7398e7c37c30d8b760228296da92, stream-scoped tags no
longer appeared as top-level tags, introducing a behaviour regression, specially
for MP3 files.

The `gst_discoverer_info_get_tags()` API now returns all tags detected for the
given media, as documented.

A new API is introduced to get container-specific tags,
`gst_discoverer_container_info_get_tags()`. The discoverer tool was adapted to
use it. `gst_discoverer_info_get_tags()` is now deprecated in favor of
`gst_discoverer_container_info_get_tags()` and
`gst_discoverer_stream_info_get_tags()`.

Fixes #759

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1107>

subprojects/gst-plugins-base/gst-libs/gst/pbutils/gstdiscoverer-types.c
subprojects/gst-plugins-base/gst-libs/gst/pbutils/gstdiscoverer.c
subprojects/gst-plugins-base/gst-libs/gst/pbutils/gstdiscoverer.h
subprojects/gst-plugins-base/gst-libs/gst/pbutils/pbutils-private.h
subprojects/gst-plugins-base/tools/gst-discoverer.c

index d679803..dd77cf3 100644 (file)
@@ -175,6 +175,9 @@ gst_discoverer_container_info_finalize (GObject * object)
 
   gst_discoverer_stream_info_list_free (info->streams);
 
+  if (info->tags)
+    gst_tag_list_unref (info->tags);
+
   gst_discoverer_stream_info_finalize ((GObject *) info);
 }
 
@@ -204,6 +207,9 @@ gst_stream_container_info_copy_int (GstDiscovererContainerInfo * ptr,
       g_hash_table_insert (stream_map, tmp->data, subtop);
   }
 
+  if (ptr->tags)
+    ret->tags = gst_tag_list_copy (ptr->tags);
+
   return ret;
 }
 
@@ -738,6 +744,23 @@ gst_discoverer_container_info_get_streams (GstDiscovererContainerInfo * info)
   return res;
 }
 
+/**
+ * gst_discoverer_container_info_get_tags:
+ * @info: a #GstDiscovererStreamInfo
+ *
+ * Returns: (transfer none): tags specific to the given container. If you wish to use
+ * the tags after the life-time of @info, you will need to copy them.
+ *
+ * Since: 1.20
+ */
+const GstTagList *
+gst_discoverer_container_info_get_tags (const GstDiscovererContainerInfo * info)
+{
+  g_return_val_if_fail (GST_IS_DISCOVERER_CONTAINER_INFO (info), NULL);
+
+  return info->tags;
+}
+
 /* GstDiscovererAudioInfo */
 
 #define AUDIO_INFO_ACCESSOR_CODE(fieldname, type, failval)             \
@@ -1057,7 +1080,6 @@ DISCOVERER_INFO_ACCESSOR_CODE (live, gboolean, FALSE);
  */
 
 DISCOVERER_INFO_ACCESSOR_CODE (misc, const GstStructure *, NULL);
-#endif
 
 /**
  * gst_discoverer_info_get_tags:
@@ -1065,9 +1087,12 @@ DISCOVERER_INFO_ACCESSOR_CODE (misc, const GstStructure *, NULL);
  *
  * Returns: (transfer none): all tags contained in the URI. If you wish to use
  * the tags after the life-time of @info, you will need to copy them.
+ *
+ * Deprecated: 1.20: Use gst_discoverer_{container,stream}_info_get_tags() instead.
  */
 
 DISCOVERER_INFO_ACCESSOR_CODE (tags, const GstTagList *, NULL);
+#endif
 
 /**
  * gst_discoverer_info_get_toc:
index 685bb27..1873c6d 100644 (file)
@@ -107,6 +107,9 @@ struct _GstDiscovererPrivate
   GError *current_error;
   GstStructure *current_topology;
 
+  GstTagList *all_tags;
+  GstTagList *global_tags;
+
   /* List of private streams */
   GList *streams;
 
@@ -357,6 +360,9 @@ gst_discoverer_init (GstDiscoverer * dc)
   dc->priv->target_state = GST_STATE_NULL;
   dc->priv->no_more_pads = FALSE;
 
+  dc->priv->all_tags = NULL;
+  dc->priv->global_tags = NULL;
+
   GST_LOG ("Creating pipeline");
   dc->priv->pipeline = (GstBin *) gst_pipeline_new ("Discoverer");
   GST_LOG_OBJECT (dc, "Creating uridecodebin");
@@ -537,36 +543,6 @@ _event_probe (GstPad * pad, GstPadProbeInfo * info, PrivateStream * ps)
   GstEvent *event = GST_PAD_PROBE_INFO_EVENT (info);
 
   switch (GST_EVENT_TYPE (event)) {
-    case GST_EVENT_TAG:{
-      GstTagList *tl = NULL, *tmp;
-      GstTagScope scope;
-
-      gst_event_parse_tag (event, &tl);
-      scope = gst_tag_list_get_scope (tl);
-      GST_DEBUG_OBJECT (pad, "tags %" GST_PTR_FORMAT, tl);
-
-      if (scope != GST_TAG_SCOPE_STREAM) {
-        GST_DEBUG_OBJECT (pad, "Ignoring non-stream tags");
-        break;
-      }
-
-      DISCO_LOCK (ps->dc);
-      /* If preroll is complete, drop these tags - the collected information is
-       * possibly already being processed and adding more tags would be racy */
-      if (G_LIKELY (ps->dc->priv->processing)) {
-        GST_DEBUG_OBJECT (pad, "private stream %p old tags %" GST_PTR_FORMAT,
-            ps, ps->tags);
-        tmp = gst_tag_list_merge (ps->tags, tl, GST_TAG_MERGE_APPEND);
-        if (ps->tags)
-          gst_tag_list_unref (ps->tags);
-        ps->tags = tmp;
-        GST_DEBUG_OBJECT (pad, "private stream %p new tags %" GST_PTR_FORMAT,
-            ps, tmp);
-      } else
-        GST_DEBUG_OBJECT (pad, "Dropping tags since preroll is done");
-      DISCO_UNLOCK (ps->dc);
-      break;
-    }
     case GST_EVENT_TOC:{
       GstToc *tmp;
 
@@ -1317,7 +1293,6 @@ parse_stream_topology (GstDiscoverer * dc, const GstStructure * topology,
   } else if (GST_VALUE_HOLDS_LIST (nval)) {
     guint i, len;
     GstDiscovererContainerInfo *cont;
-    GstTagList *tags;
     GstPad *srcpad;
 
     if (gst_structure_id_get (topology, _ELEMENT_SRCPAD_QUARK, GST_TYPE_PAD,
@@ -1338,26 +1313,9 @@ parse_stream_topology (GstDiscoverer * dc, const GstStructure * topology,
     cont = (GstDiscovererContainerInfo *)
         g_object_new (GST_TYPE_DISCOVERER_CONTAINER_INFO, NULL);
     cont->parent.caps = caps;
+    cont->tags = gst_tag_list_ref (dc->priv->global_tags);
     res = (GstDiscovererStreamInfo *) cont;
 
-    if (gst_structure_id_has_field (topology, _TAGS_QUARK)) {
-      GstTagList *tmp;
-
-      gst_structure_id_get (topology, _TAGS_QUARK,
-          GST_TYPE_TAG_LIST, &tags, NULL);
-
-      GST_DEBUG ("Merge tags %" GST_PTR_FORMAT, tags);
-
-      tmp =
-          gst_tag_list_merge (cont->parent.tags, (GstTagList *) tags,
-          GST_TAG_MERGE_APPEND);
-      gst_tag_list_unref (tags);
-      if (cont->parent.tags)
-        gst_tag_list_unref (cont->parent.tags);
-      cont->parent.tags = tmp;
-      GST_DEBUG ("Container info tags %" GST_PTR_FORMAT, tmp);
-    }
-
     for (i = 0; i < len; i++) {
       const GValue *subv = gst_value_list_get_value (nval, i);
       const GstStructure *subst = gst_value_get_structure (subv);
@@ -1400,14 +1358,24 @@ setup_next_uri_locked (GstDiscoverer * dc)
   }
 }
 
+static GstDiscovererInfo *
+_ensure_info_tags (GstDiscoverer * dc)
+{
+  GstDiscovererInfo *info = dc->priv->current_info;
+
+  if (dc->priv->all_tags)
+    info->tags = dc->priv->all_tags;
+  dc->priv->all_tags = NULL;
+  return info;
+}
 
 static void
 emit_discovererd (GstDiscoverer * dc)
 {
-  GST_DEBUG_OBJECT (dc, "Emitting 'discoverered' %s",
-      dc->priv->current_info->uri);
+  GstDiscovererInfo *info = _ensure_info_tags (dc);
+  GST_DEBUG_OBJECT (dc, "Emitting 'discoverered' %s", info->uri);
   g_signal_emit (dc, gst_discoverer_signals[SIGNAL_DISCOVERED], 0,
-      dc->priv->current_info, dc->priv->current_error);
+      info, dc->priv->current_error);
   /* Clients get a copy of current_info since it is a boxed type */
   gst_discoverer_info_unref (dc->priv->current_info);
   dc->priv->current_info = NULL;
@@ -1718,25 +1686,34 @@ handle_message (GstDiscoverer * dc, GstMessage * msg)
       scope = gst_tag_list_get_scope (tl);
       GST_DEBUG_OBJECT (GST_MESSAGE_SRC (msg), "Got tags %" GST_PTR_FORMAT, tl);
 
-      if (scope != GST_TAG_SCOPE_GLOBAL) {
-        GST_DEBUG_OBJECT (GST_MESSAGE_SRC (msg), "Ignoring non-global tags");
-        gst_tag_list_unref (tl);
-        break;
+      tmp = gst_tag_list_merge (dc->priv->all_tags, tl, GST_TAG_MERGE_APPEND);
+      if (dc->priv->all_tags)
+        gst_tag_list_unref (dc->priv->all_tags);
+      dc->priv->all_tags = tmp;
+
+      if (scope == GST_TAG_SCOPE_STREAM) {
+        for (GList * curr = dc->priv->streams; curr; curr = curr->next) {
+          PrivateStream *ps = (PrivateStream *) curr->data;
+          if (GST_MESSAGE_SRC (msg) == GST_OBJECT_CAST (ps->sink)) {
+            tmp = gst_tag_list_merge (ps->tags, tl, GST_TAG_MERGE_APPEND);
+            if (ps->tags)
+              gst_tag_list_unref (ps->tags);
+            ps->tags = tmp;
+            GST_DEBUG_OBJECT (ps->pad, "tags %" GST_PTR_FORMAT, ps->tags);
+            break;
+          }
+        }
+      } else {
+        tmp =
+            gst_tag_list_merge (dc->priv->global_tags, tl,
+            GST_TAG_MERGE_APPEND);
+        if (dc->priv->global_tags)
+          gst_tag_list_unref (dc->priv->global_tags);
+        dc->priv->global_tags = tmp;
       }
-
-      /* Merge with current tags */
-      tmp =
-          gst_tag_list_merge (dc->priv->current_info->tags, tl,
-          GST_TAG_MERGE_APPEND);
       gst_tag_list_unref (tl);
-      if (dc->priv->current_info->tags)
-        gst_tag_list_unref (dc->priv->current_info->tags);
-      dc->priv->current_info->tags = tmp;
-      GST_DEBUG_OBJECT (GST_MESSAGE_SRC (msg), "Current info %p, tags %"
-          GST_PTR_FORMAT, dc->priv->current_info, tmp);
     }
       break;
-
     case GST_MESSAGE_TOC:
     {
       GstToc *tmp;
@@ -1978,6 +1955,16 @@ discoverer_cleanup (GstDiscoverer * dc)
 
   dc->priv->current_info = NULL;
 
+  if (dc->priv->all_tags) {
+    gst_tag_list_unref (dc->priv->all_tags);
+    dc->priv->all_tags = NULL;
+  }
+
+  if (dc->priv->global_tags) {
+    gst_tag_list_unref (dc->priv->global_tags);
+    dc->priv->global_tags = NULL;
+  }
+
   dc->priv->pending_subtitle_pads = 0;
 
   dc->priv->current_state = GST_STATE_NULL;
@@ -2410,6 +2397,7 @@ _parse_discovery (GVariant * variant, GstDiscovererInfo * info)
 
     GstDiscovererContainerInfo *cinfo = GST_DISCOVERER_CONTAINER_INFO (sinfo);
     g_variant_iter_init (&iter, specific);
+    cinfo->tags = sinfo->tags;
     while ((child = g_variant_iter_next_value (&iter))) {
       GstDiscovererStreamInfo *child_info;
       child_info = _parse_discovery (g_variant_get_variant (child), info);
@@ -2625,7 +2613,7 @@ gst_discoverer_discover_uri (GstDiscoverer * discoverer, const gchar * uri,
         discoverer->priv->current_info->result);
     discoverer->priv->current_info->result = res;
   }
-  info = discoverer->priv->current_info;
+  info = _ensure_info_tags (discoverer);
 
   discoverer_cleanup (discoverer);
 
index 2a34f9a..f30f0ce 100644 (file)
@@ -104,6 +104,9 @@ GType gst_discoverer_container_info_get_type (void);
 GST_PBUTILS_API
 GList *gst_discoverer_container_info_get_streams(GstDiscovererContainerInfo *info);
 
+GST_PBUTILS_API
+const GstTagList* gst_discoverer_container_info_get_tags(const GstDiscovererContainerInfo *info);
+
 
 /**
  * GstDiscovererAudioInfo:
@@ -306,8 +309,8 @@ gboolean                  gst_discoverer_info_get_live(const GstDiscovererInfo*
 GST_PBUTILS_DEPRECATED_FOR(gst_discoverer_info_get_missing_elements_installer_details)
 const GstStructure*       gst_discoverer_info_get_misc(const GstDiscovererInfo* info);
 
-GST_PBUTILS_API
-const GstTagList*         gst_discoverer_info_get_tags(const GstDiscovererInfo* info); 
+GST_PBUTILS_DEPRECATED
+const GstTagList*         gst_discoverer_info_get_tags(const GstDiscovererInfo* info);
 GST_PBUTILS_API
 const GstToc*             gst_discoverer_info_get_toc(const GstDiscovererInfo* info);
 
index 856b63f..835d670 100644 (file)
@@ -37,6 +37,7 @@ struct _GstDiscovererContainerInfo {
   GstDiscovererStreamInfo parent;
 
   GList               *streams;
+  GstTagList          *tags;
 };
 
 struct _GstDiscovererAudioInfo {
index 5bd27ac..6cfea21 100644 (file)
@@ -388,6 +388,12 @@ print_stream_info (GstDiscovererStreamInfo * info, void *depth)
     desc =
         gst_stream_subtitle_information_to_string (info,
         GPOINTER_TO_INT (depth) + 1);
+  else if (GST_IS_DISCOVERER_CONTAINER_INFO (info)) {
+    const GstTagList *tags =
+        gst_discoverer_container_info_get_tags (GST_DISCOVERER_CONTAINER_INFO
+        (info));
+    print_tags_topology (GPOINTER_TO_INT (depth) + 1, tags);
+  }
   if (desc) {
     g_print ("%s", desc);
     g_free (desc);