From d7221846bdd2c30ed4797f997e88a972b3abd0bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Dzi=C4=99giel?= Date: Sat, 22 Nov 2025 20:44:08 +0100 Subject: [PATCH] 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 | 2 +- src/lib/clapper/clapper-media-item.c | 45 ++++++++---- src/lib/clapper/clapper-playbin-bus.c | 31 ++++---- src/lib/clapper/gst/clapper-playlist-demux.c | 70 ++++++++++++++++++- .../gst/clapper-uri-base-demux-private.h | 2 + src/lib/clapper/gst/clapper-uri-base-demux.c | 15 ++++ 6 files changed, 132 insertions(+), 33 deletions(-) diff --git a/src/lib/clapper/clapper-media-item-private.h b/src/lib/clapper/clapper-media-item-private.h index d1666355..c48467ff 100644 --- a/src/lib/clapper/clapper-media-item-private.h +++ b/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); diff --git a/src/lib/clapper/clapper-media-item.c b/src/lib/clapper/clapper-media-item.c index 12ca6209..cffd2f55 100644 --- a/src/lib/clapper/clapper-media-item.c +++ b/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); diff --git a/src/lib/clapper/clapper-playbin-bus.c b/src/lib/clapper/clapper-playbin-bus.c index 347beb09..4da0515e 100644 --- a/src/lib/clapper/clapper-playbin-bus.c +++ b/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), diff --git a/src/lib/clapper/gst/clapper-playlist-demux.c b/src/lib/clapper/gst/clapper-playlist-demux.c index dc83b161..0c7afac4 100644 --- a/src/lib/clapper/gst/clapper-playlist-demux.c +++ b/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", diff --git a/src/lib/clapper/gst/clapper-uri-base-demux-private.h b/src/lib/clapper/gst/clapper-uri-base-demux-private.h index 3237c419..169b91e3 100644 --- a/src/lib/clapper/gst/clapper-uri-base-demux-private.h +++ b/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); diff --git a/src/lib/clapper/gst/clapper-uri-base-demux.c b/src/lib/clapper/gst/clapper-uri-base-demux.c index 45bcdd97..b6baf2f1 100644 --- a/src/lib/clapper/gst/clapper-uri-base-demux.c +++ b/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);