clapper: Reworked infinite nested playlists detection

Instead of doing this inside player, logic was reworked and moved into playlist demuxer
itself. It uses GstQueries to communicate with upstream elements. This way we can post
an error from running pipeline which helps keep this logic in single place and have it
shared with other pipelines (like the one used in Media Scanner Enhancer).

As part of the rework, nested playlist limit was added and set to 10 redirects.
Seems like a safe/optimal number and this simplifies stuff a lot. Instead of
keeping a list of strings with previous redirected URIs, we can just store the
one most recent in media item.

src/lib/clapper/clapper-media-item-private.h

@@ -34,7 +34,7 @@ G_GNUC_INTERNAL
 void clapper_media_item_update_from_discoverer_info (ClapperMediaItem *self, GstDiscovererInfo *info);
 
 G_GNUC_INTERNAL
-gboolean clapper_media_item_update_from_item (ClapperMediaItem *item, ClapperMediaItem *other_item, ClapperPlayer *player);
+gboolean clapper_media_item_update_from_parsed_playlist (ClapperMediaItem *item, GListStore *playlist, GstObject *playlist_src, ClapperPlayer *player);
 
 G_GNUC_INTERNAL
 gboolean clapper_media_item_set_duration (ClapperMediaItem *item, gdouble duration, ClapperAppBus *app_bus);

src/lib/clapper/clapper-media-item.c

@@ -54,7 +54,7 @@ struct _ClapperMediaItem
   /* Whether using title from URI */
   gboolean title_is_parsed;
 
-  GSList *redirects;
+  gchar *redirect_uri;
   gchar *cache_uri;
 
   /* For shuffle */
@@ -203,8 +203,8 @@ clapper_media_item_get_id (ClapperMediaItem *self)
 }
 
 /* FIXME: 1.0:
- * Consider change to be transfer-full and just return latest data from redirects
- * list (alternatively expose redirect URI). This should make it possible to work
+ * Consider change to be transfer-full and just return latest redirect URI
+ * (alternatively expose redirect URI). This should make it possible to work
  * with enhancers that would benefit from knowledge about URI changes
  * (e.g "Recall" could read actual media instead of playlist file).
  */
@@ -681,26 +681,41 @@ clapper_media_item_update_from_discoverer_info (ClapperMediaItem *self, GstDisco
 
 /* XXX: Must be set from player thread */
 static inline gboolean
-clapper_media_item_set_redirect_uri (ClapperMediaItem *self, const gchar *redirect_uri)
+clapper_media_item_set_redirect_uri (ClapperMediaItem *self, const gchar *redirect_uri,
+    GstObject *redirect_src)
 {
-  /* Check if we did not already redirect into that URI (prevent endless loop) */
-  if (!redirect_uri || g_slist_find_custom (self->redirects, redirect_uri, (GCompareFunc) strcmp))
+  /* Safety checks */
+  if (G_UNLIKELY (!redirect_uri)) {
+    GST_ERROR_OBJECT (self, "Received redirect request without an URI set");
     return FALSE;
+  }
+  if (G_UNLIKELY (!redirect_src)) {
+    GST_ERROR_OBJECT (self, "Received redirect request without source object set");
+    return FALSE;
+  }
 
-  self->redirects = g_slist_prepend (self->redirects, g_strdup (redirect_uri));
-  GST_DEBUG_OBJECT (self, "Set redirect URI: \"%s\"", (gchar *) self->redirects->data);
+  g_set_str (&self->redirect_uri, redirect_uri);
+  GST_DEBUG_OBJECT (self, "Set redirect URI: \"%s\", source: %s",
+      self->redirect_uri, G_OBJECT_TYPE_NAME (redirect_src));
 
   return TRUE;
 }
 
 gboolean
-clapper_media_item_update_from_item (ClapperMediaItem *self, ClapperMediaItem *other_item,
-    ClapperPlayer *player)
+clapper_media_item_update_from_parsed_playlist (ClapperMediaItem *self, GListStore *playlist,
+    GstObject *playlist_src, ClapperPlayer *player)
 {
+  ClapperMediaItem *other_item;
+  const gchar *redirect_uri;
   gboolean title_changed = FALSE;
 
-  if (!clapper_media_item_set_redirect_uri (self, clapper_media_item_get_uri (other_item)))
+  other_item = g_list_model_get_item (G_LIST_MODEL (playlist), 0);
+  redirect_uri = clapper_media_item_get_uri (other_item);
+
+  if (!clapper_media_item_set_redirect_uri (self, redirect_uri, playlist_src)) {
+    gst_object_unref (other_item);
     return FALSE;
+  }
 
   GST_OBJECT_LOCK (other_item);
 
@@ -731,6 +746,8 @@ clapper_media_item_update_from_item (ClapperMediaItem *self, ClapperMediaItem *o
       clapper_features_manager_trigger_item_updated (features_manager, self);
   }
 
+  gst_object_unref (other_item);
+
   return TRUE;
 }
 
@@ -768,8 +785,8 @@ clapper_media_item_get_playback_uri (ClapperMediaItem *self)
     clapper_media_item_set_cache_location (self, NULL);
   }
 
-  if (self->redirects)
-    return self->redirects->data;
+  if (self->redirect_uri)
+    return self->redirect_uri;
 
   return self->uri;
 }
@@ -835,7 +852,7 @@ clapper_media_item_finalize (GObject *object)
   gst_object_unparent (GST_OBJECT_CAST (self->timeline));
   gst_object_unref (self->timeline);
 
-  g_slist_free_full (self->redirects, g_free);
+  g_free (self->redirect_uri);
   g_free (self->cache_uri);
 
   G_OBJECT_CLASS (parent_class)->finalize (object);

src/lib/clapper/clapper-playbin-bus.c

@@ -29,6 +29,7 @@
 #include "clapper-stream-private.h"
 #include "clapper-stream-list-private.h"
 #include "gst/clapper-extractable-src-private.h"
+#include "gst/clapper-playlist-demux-private.h"
 
 #define GST_CAT_DEFAULT clapper_playbin_bus_debug
 GST_DEBUG_CATEGORY_STATIC (GST_CAT_DEFAULT);
@@ -812,20 +813,28 @@ clapper_playbin_bus_post_user_message (GstBus *bus, GstMessage *msg)
 static inline void
 _on_playlist_parsed_msg (GstMessage *msg, ClapperPlayer *player)
 {
+  GstObject *src = GST_MESSAGE_SRC (msg);
   ClapperMediaItem *playlist_item = NULL;
   GListStore *playlist = NULL;
-  const GstStructure *structure = gst_message_get_structure (msg);
+  const GstStructure *structure;
   guint n_items;
 
+  if (G_UNLIKELY (!src)) {
+    GST_WARNING_OBJECT (player, "Ignoring playlist parsed message without a source");
+    return;
+  }
+
+  structure = gst_message_get_structure (msg);
+
   /* If message contains item, use that.
    * Otherwise assume pending item was parsed. */
   if (gst_structure_has_field (structure, "item")) {
     gst_structure_get (structure,
         "item", CLAPPER_TYPE_MEDIA_ITEM, &playlist_item, NULL);
-  } else {
+  } else if (CLAPPER_IS_PLAYLIST_DEMUX (src)) {
     GST_OBJECT_LOCK (player);
 
-    /* Playlist is always parsed before playback starts */
+    /* Playlist from demuxer is always parsed before playback starts */
     if (player->pending_item)
       playlist_item = gst_object_ref (player->pending_item);
 
@@ -850,22 +859,10 @@ _on_playlist_parsed_msg (GstMessage *msg, ClapperPlayer *player)
     gboolean updated;
 
     /* Update redirect URI (must be done from player thread) */
-    updated = clapper_media_item_update_from_item (playlist_item, active_item, player);
+    updated = clapper_media_item_update_from_parsed_playlist (playlist_item, playlist, src, player);
     gst_object_unref (active_item);
 
-    if (!updated) {
-      GstMessage *msg;
-      GError *error;
-
-      error = g_error_new (GST_RESOURCE_ERROR, GST_RESOURCE_ERROR_FAILED,
-          "Detected infinite redirection in playlist");
-      msg = gst_message_new_error (GST_OBJECT (player), error, NULL);
-
-      _handle_error_msg (msg, player);
-
-      g_error_free (error);
-      gst_message_unref (msg);
-    } else if (n_items > 1) {
+    if (updated && n_items > 1) {
       /* Forward to append remaining items (must be done from main thread) */
       clapper_app_bus_post_insert_playlist (player->app_bus,
           GST_OBJECT_CAST (player),

src/lib/clapper/gst/clapper-playlist-demux.c

@@ -30,6 +30,10 @@
 #define URI_LIST_MEDIA_TYPE "text/uri-list"
 #define DATA_CHUNK_SIZE 4096
 
+#define NTH_REDIRECT_STRUCTURE_NAME "ClapperQueryNthRedirect"
+#define NTH_REDIRECT_FIELD "nth-redirect"
+#define MAX_REDIRECTS 10
+
 #define GST_CAT_DEFAULT clapper_playlist_demux_debug
 GST_DEBUG_CATEGORY_STATIC (GST_CAT_DEFAULT);
 
@@ -348,6 +352,50 @@ _handle_playlist (ClapperPlaylistDemux *self, GListStore *playlist, GCancellable
   return TRUE;
 }
 
+static void
+_query_parse_nth_redirect (GstQuery *query, guint *nth_redirect)
+{
+  const GstStructure *structure = gst_query_get_structure (query);
+  *nth_redirect = g_value_get_uint (gst_structure_get_value (structure, NTH_REDIRECT_FIELD));
+}
+
+static void
+_query_set_nth_redirect (GstQuery *query, guint nth_redirect)
+{
+  GstStructure *structure = gst_query_writable_structure (query);
+  gst_structure_set (structure, NTH_REDIRECT_FIELD, G_TYPE_UINT, nth_redirect, NULL);
+}
+
+static gboolean
+clapper_playlist_demux_handle_custom_query (ClapperUriBaseDemux *uri_bd, GstQuery *query)
+{
+  ClapperPlaylistDemux *self = CLAPPER_PLAYLIST_DEMUX_CAST (uri_bd);
+  const GstStructure *structure = gst_query_get_structure (query);
+
+  if (gst_structure_has_name (structure, NTH_REDIRECT_STRUCTURE_NAME)) {
+    GstPad *sink_pad;
+
+    GST_LOG_OBJECT (self, "Received custom query: " NTH_REDIRECT_STRUCTURE_NAME);
+
+    sink_pad = gst_element_get_static_pad (GST_ELEMENT_CAST (self), "sink");
+    gst_pad_peer_query (sink_pad, query);
+    gst_object_unref (sink_pad);
+
+    if (G_LIKELY (gst_query_is_writable (query))) {
+      guint nth_redirect = 0;
+
+      _query_parse_nth_redirect (query, &nth_redirect);
+      _query_set_nth_redirect (query, ++nth_redirect);
+    } else {
+      GST_ERROR_OBJECT (self, "Unwritable custom query: " NTH_REDIRECT_STRUCTURE_NAME);
+    }
+
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
 static gboolean
 clapper_playlist_demux_process_buffer (ClapperUriBaseDemux *uri_bd,
     GstBuffer *buffer, GCancellable *cancellable)
@@ -355,9 +403,11 @@ clapper_playlist_demux_process_buffer (ClapperUriBaseDemux *uri_bd,
   ClapperPlaylistDemux *self = CLAPPER_PLAYLIST_DEMUX_CAST (uri_bd);
   GstPad *sink_pad;
   GstQuery *query;
+  GstStructure *query_structure;
   GUri *uri = NULL;
   GListStore *playlist;
   GError *error = NULL;
+  guint nth_redirect = 0;
   gboolean handled;
 
   sink_pad = gst_element_get_static_pad (GST_ELEMENT_CAST (self), "sink");
@@ -375,11 +425,28 @@ clapper_playlist_demux_process_buffer (ClapperUriBaseDemux *uri_bd,
     }
   }
 
+  gst_query_unref (query);
+
+  query_structure = gst_structure_new (NTH_REDIRECT_STRUCTURE_NAME,
+      NTH_REDIRECT_FIELD, G_TYPE_UINT, 0, NULL);
+  query = gst_query_new_custom (GST_QUERY_CUSTOM, query_structure);
+
+  if (gst_pad_peer_query (sink_pad, query))
+    _query_parse_nth_redirect (query, &nth_redirect);
+
+  GST_DEBUG_OBJECT (self, "Current number of redirects: %u", nth_redirect);
+
   gst_query_unref (query);
   gst_object_unref (sink_pad);
 
   if (G_UNLIKELY (uri == NULL)) {
-    GST_ERROR_OBJECT (self, "Could not query source URI");
+    GST_ELEMENT_ERROR (self, RESOURCE, FAILED,
+        ("Could not query source URI"), (NULL));
+    return FALSE;
+  }
+  if (G_UNLIKELY (nth_redirect > MAX_REDIRECTS)) {
+    GST_ELEMENT_ERROR (self, RESOURCE, FAILED,
+        ("Too many nested playlists"), (NULL));
     return FALSE;
   }
 
@@ -506,6 +573,7 @@ clapper_playlist_demux_class_init (ClapperPlaylistDemuxClass *klass)
   gobject_class->finalize = clapper_playlist_demux_finalize;
 
   clapperuribd_class->handle_caps = clapper_playlist_demux_handle_caps;
+  clapperuribd_class->handle_custom_query = clapper_playlist_demux_handle_custom_query;
   clapperuribd_class->process_buffer = clapper_playlist_demux_process_buffer;
 
   param_specs[PROP_ENHANCER_PROXIES] = g_param_spec_object ("enhancer-proxies",

src/lib/clapper/gst/clapper-uri-base-demux-private.h

@@ -41,6 +41,8 @@ struct _ClapperUriBaseDemuxClass
   void (* handle_caps) (ClapperUriBaseDemux *uri_bd, GstCaps *caps);
 
   void (* handle_custom_event) (ClapperUriBaseDemux *uri_bd, GstEvent *event);
+
+  gboolean (* handle_custom_query) (ClapperUriBaseDemux *uri_bd, GstQuery *query);
 };
 
 gboolean clapper_uri_base_demux_set_uri (ClapperUriBaseDemux *uri_bd, const gchar *uri, const gchar *blacklisted_el);

src/lib/clapper/gst/clapper-uri-base-demux.c

@@ -189,6 +189,20 @@ _make_handler_for_uri (ClapperUriBaseDemux *self, const gchar *uri, const gchar
   return element;
 }
 
+static gboolean
+_src_pad_query_func (GstPad *pad, GstObject *parent, GstQuery *query)
+{
+  if (GST_QUERY_TYPE (query) == GST_QUERY_CUSTOM) {
+    ClapperUriBaseDemux *self = CLAPPER_URI_BASE_DEMUX_CAST (parent);
+    ClapperUriBaseDemuxClass *uri_bd_class = CLAPPER_URI_BASE_DEMUX_GET_CLASS (self);
+
+    if (uri_bd_class->handle_custom_query && uri_bd_class->handle_custom_query (self, query))
+      return TRUE;
+  }
+
+  return gst_pad_query_default (pad, parent, query);
+}
+
 gboolean
 clapper_uri_base_demux_set_uri (ClapperUriBaseDemux *self, const gchar *uri, const gchar *blacklisted_el)
 {
@@ -254,6 +268,7 @@ clapper_uri_base_demux_set_uri (ClapperUriBaseDemux *self, const gchar *uri, con
 
     src_ghostpad = gst_ghost_pad_new_from_template ("src", priv->typefind_src,
         gst_element_class_get_pad_template (GST_ELEMENT_GET_CLASS (self), "src"));
+    gst_pad_set_query_function (src_ghostpad, (GstPadQueryFunction) _src_pad_query_func);
 
     gst_pad_set_active (src_ghostpad, TRUE);