[vlc-commits] [Git][videolan/vlc][master] 10 commits: preparser: move executor check

Steve Lhomme (@robUx4) gitlab at videolan.org
Fri Nov 1 06:25:25 UTC 2024



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
16d9f52a by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: move executor check

- - - - -
61ba40e8 by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: remove atomic

Re-order call to input_item_parser_id_Release() in order to get ride of
the atomic.

- - - - -
d4c72c35 by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: remove useless check

Since the last commit, preparse_status is overridden in case of interrupt
after the call to input_item_parser_id_Release().

- - - - -
a4abaa45 by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: don't set interrupt in case of timeout

But preparse_status will be VLC_ETIMEOUT in case of interrupt or timeout.

- - - - -
f8b7832a by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: remove unused argument

- - - - -
6e0d037a by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: don't call extra callbacks when cancelled

- - - - -
95c147d8 by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: refactor code

No functional changes.

- - - - -
aad952ea by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: notify on_ended with -EINTR when cancelled

- - - - -
64cce805 by Thomas Guillem at 2024-11-01T06:11:06+00:00
preparser: don't held the lock in (cancel) callback

- - - - -
b2883408 by Thomas Guillem at 2024-11-01T06:11:06+00:00
thumbnailer: notify on_ended when cancelled

Like the preparser is doing.

- - - - -


6 changed files:

- include/vlc_input_item.h
- include/vlc_thumbnailer.h
- src/input/thumbnailer.c
- src/preparser/preparser.c
- test/libvlc/media.c
- test/src/input/thumbnail.c


Changes:

=====================================
include/vlc_input_item.h
=====================================
@@ -435,7 +435,7 @@ typedef struct input_item_parser_cbs_t
      *
      * @param item the parsed item
      * @param status VLC_SUCCESS in case of success, VLC_ETIMEOUT in case of
-     * timeout, an error otherwise
+     * timeout, -EINTR if cancelled, an error otherwise
      * @param userdata user data set by input_item_Parse()
      */
     void (*on_ended)(input_item_t *item, int status, void *userdata);


=====================================
include/vlc_thumbnailer.h
=====================================
@@ -45,9 +45,11 @@ struct vlc_thumbnailer_cbs
      *
      * @note This callback is mandatory.
      *
-     * In case of failure, p_thumbnail will be NULL.  The picture, if any, is
-     * owned by the thumbnailer, and must be acquired by using \link
-     * picture_Hold \endlink to use it pass the callback's scope.
+     * In case of failure, timeout or cancellation, p_thumbnail will be NULL.
+     * The picture, if any, is owned by the thumbnailer, and must be acquired
+     * by using \link picture_Hold \endlink to use it pass the callback's
+     * scope.
+     *
      * \param thumbnail The generated thumbnail, or NULL in case of failure or
      * timeout
      * \param data Is the opaque pointer passed as vlc_thumbnailer_Request last


=====================================
src/input/thumbnailer.c
=====================================
@@ -209,13 +209,12 @@ RunnableRun(void *userdata)
     picture_t* pic = task->pic;
     task->pic = NULL;
 
-    bool notify = task->status != INTERRUPTED;
+    bool interrupted = task->status == INTERRUPTED;
 
     vlc_list_remove(&task->node);
     vlc_mutex_unlock(&thumbnailer->lock);
 
-    if (notify)
-        NotifyThumbnail(task, pic);
+    NotifyThumbnail(task, interrupted ? NULL : pic);
 
     if (pic)
         picture_Release(pic);
@@ -268,7 +267,15 @@ size_t vlc_thumbnailer_Cancel( vlc_thumbnailer_t* thumbnailer, vlc_thumbnailer_r
             if (canceled)
             {
                 vlc_list_remove(&task->node);
+                vlc_mutex_unlock(&thumbnailer->lock);
+                NotifyThumbnail(task, NULL);
                 TaskDestroy(task);
+
+                /* Small optimisation in the likely case where the user cancel
+                 * only one task */
+                if (id != VLC_THUMBNAILER_REQ_ID_INVALID)
+                    return count;
+                vlc_mutex_lock(&thumbnailer->lock);
             }
             else
             {


=====================================
src/preparser/preparser.c
=====================================
@@ -56,7 +56,7 @@ struct task
     input_item_parser_id_t *parser;
 
     vlc_sem_t preparse_ended;
-    atomic_int preparse_status;
+    int preparse_status;
     atomic_bool interrupted;
 
     struct vlc_runnable runnable; /**< to be passed to the executor */
@@ -85,7 +85,7 @@ TaskNew(vlc_preparser_t *preparser, input_item_t *item,
 
     task->parser = NULL;
     vlc_sem_init(&task->preparse_ended, 0);
-    atomic_init(&task->preparse_status, VLC_EGENERIC);
+    task->preparse_status = VLC_EGENERIC;
     atomic_init(&task->interrupted, false);
 
     task->runnable.run = RunnableRun;
@@ -130,17 +130,18 @@ PreparserRemoveTask(vlc_preparser_t *preparser, struct task *task)
 }
 
 static void
-NotifyPreparseEnded(struct task *task, bool art_fetched)
+NotifyPreparseEnded(struct task *task, bool cancelled)
 {
-    (void) art_fetched;
+    if (cancelled || atomic_load(&task->interrupted))
+        task->preparse_status = -EINTR;
+    else if (task->preparse_status == VLC_SUCCESS)
+        input_item_SetPreparsed(task->item);
+
     if (task->cbs == NULL)
         return;
 
-    if (task->cbs->on_ended) {
-        int status = atomic_load_explicit(&task->preparse_status,
-                                          memory_order_relaxed);
-        task->cbs->on_ended(task->item, status, task->userdata);
-    }
+    if (task->cbs->on_ended)
+        task->cbs->on_ended(task->item, task->preparse_status, task->userdata);
 }
 
 static void
@@ -149,15 +150,7 @@ OnParserEnded(input_item_t *item, int status, void *task_)
     VLC_UNUSED(item);
     struct task *task = task_;
 
-    if (atomic_load(&task->interrupted))
-        /*
-         * On interruption, the call to input_item_parser_id_Release() may
-         * trigger this "parser ended" callback. Ignore it.
-         */
-        return;
-
-    atomic_store_explicit(&task->preparse_status, status,
-                          memory_order_relaxed);
+    task->preparse_status = status;
     vlc_sem_post(&task->preparse_ended);
 }
 
@@ -168,6 +161,9 @@ OnParserSubtreeAdded(input_item_t *item, input_item_node_t *subtree,
     VLC_UNUSED(item);
     struct task *task = task_;
 
+    if (atomic_load(&task->interrupted))
+        return;
+
     if (task->cbs && task->cbs->on_subtree_added)
         task->cbs->on_subtree_added(task->item, subtree, task->userdata);
 }
@@ -180,19 +176,13 @@ OnParserAttachmentsAdded(input_item_t *item,
     VLC_UNUSED(item);
     struct task *task = task_;
 
+    if (atomic_load(&task->interrupted))
+        return;
+
     if (task->cbs && task->cbs->on_attachments_added)
         task->cbs->on_attachments_added(task->item, array, count, task->userdata);
 }
 
-static void
-SetItemPreparsed(struct task *task)
-{
-    int status = atomic_load_explicit(&task->preparse_status,
-                                      memory_order_relaxed);
-    if (status == VLC_SUCCESS)
-        input_item_SetPreparsed(task->item);
-}
-
 static void
 OnArtFetchEnded(input_item_t *item, bool fetched, void *userdata)
 {
@@ -201,10 +191,7 @@ OnArtFetchEnded(input_item_t *item, bool fetched, void *userdata)
 
     struct task *task = userdata;
 
-    if (!atomic_load(&task->interrupted))
-        SetItemPreparsed(task);
-
-    NotifyPreparseEnded(task, fetched);
+    NotifyPreparseEnded(task, false);
     TaskDelete(task);
 }
 
@@ -231,8 +218,7 @@ Parse(struct task *task, vlc_tick_t deadline)
     task->parser = input_item_Parse(obj, task->item, &cfg);
     if (!task->parser)
     {
-        atomic_store_explicit(&task->preparse_status, VLC_EGENERIC,
-                              memory_order_relaxed);
+        task->preparse_status = VLC_EGENERIC;
         return;
     }
 
@@ -242,9 +228,9 @@ Parse(struct task *task, vlc_tick_t deadline)
     else
         if (vlc_sem_timedwait(&task->preparse_ended, deadline))
         {
-            atomic_store_explicit(&task->preparse_status,
-                                  VLC_ETIMEOUT, memory_order_relaxed);
-            atomic_store(&task->interrupted, true);
+            input_item_parser_id_Release(task->parser);
+            task->preparse_status = VLC_ETIMEOUT;
+            return;
         }
 
     /* This call also interrupts the parsing if it is still running */
@@ -287,7 +273,7 @@ RunnableRun(void *userdata)
 
     PreparserRemoveTask(preparser, task);
 
-    if (atomic_load(&task->interrupted))
+    if (task->preparse_status == VLC_ETIMEOUT)
         goto end;
 
     int ret = Fetch(task);
@@ -295,9 +281,6 @@ RunnableRun(void *userdata)
     if (ret == VLC_SUCCESS)
         return; /* Remove the task and notify from the fetcher callback */
 
-    if (!atomic_load(&task->interrupted))
-        SetItemPreparsed(task);
-
 end:
     NotifyPreparseEnded(task, false);
     TaskDelete(task);
@@ -308,9 +291,6 @@ Interrupt(struct task *task)
 {
     atomic_store(&task->interrupted, true);
 
-    /* Wake up the preparser cond_wait */
-    atomic_store_explicit(&task->preparse_status, VLC_ETIMEOUT,
-                          memory_order_relaxed);
     vlc_sem_post(&task->preparse_ended);
 }
 
@@ -406,9 +386,6 @@ vlc_preparser_req_id vlc_preparser_Push( vlc_preparser_t *preparser, input_item_
 
 size_t vlc_preparser_Cancel( vlc_preparser_t *preparser, vlc_preparser_req_id id )
 {
-    if (preparser->executor == NULL)
-        return 0; /* TODO: the fetcher should be cancellable too */
-
     vlc_mutex_lock(&preparser->lock);
 
     struct task *task;
@@ -418,13 +395,21 @@ size_t vlc_preparser_Cancel( vlc_preparser_t *preparser, vlc_preparser_req_id id
         if (id == VLC_PREPARSER_REQ_ID_INVALID || task->id == id)
         {
             count++;
-            bool canceled =
-                vlc_executor_Cancel(preparser->executor, &task->runnable);
+            /* TODO: the fetcher should be cancellable too */
+            bool canceled = preparser->executor != NULL
+              && vlc_executor_Cancel(preparser->executor, &task->runnable);
             if (canceled)
             {
-                NotifyPreparseEnded(task, false);
                 vlc_list_remove(&task->node);
+                vlc_mutex_unlock(&preparser->lock);
+                NotifyPreparseEnded(task, true);
                 TaskDelete(task);
+
+                /* Small optimisation in the likely case where the user cancel
+                 * only one task */
+                if (id != VLC_PREPARSER_REQ_ID_INVALID)
+                    return count;
+                vlc_mutex_lock(&preparser->lock);
             }
             else
                 /* The task will be finished and destroyed after run() */


=====================================
test/libvlc/media.c
=====================================
@@ -218,6 +218,16 @@ static void input_item_preparse_timeout( input_item_t *item,
     vlc_sem_post(p_sem);
 }
 
+static void input_item_preparse_cancel( input_item_t *item,
+                                        int status, void *user_data )
+{
+    VLC_UNUSED(item);
+    vlc_sem_t *p_sem = user_data;
+
+    assert( status == -EINTR );
+    vlc_sem_post(p_sem);
+}
+
 static void test_input_metadata_timeout(libvlc_instance_t *vlc, int timeout,
                                         int wait_and_cancel)
 {
@@ -237,7 +247,8 @@ static void test_input_metadata_timeout(libvlc_instance_t *vlc, int timeout,
     vlc_sem_t sem;
     vlc_sem_init (&sem, 0);
     const input_item_parser_cbs_t cbs = {
-        .on_ended = input_item_preparse_timeout,
+        .on_ended = wait_and_cancel > 0 ? input_item_preparse_cancel
+                                        : input_item_preparse_timeout,
     };
 
     input_item_meta_request_option_t options =
@@ -255,7 +266,6 @@ static void test_input_metadata_timeout(libvlc_instance_t *vlc, int timeout,
         vlc_tick_sleep( VLC_TICK_FROM_MS(wait_and_cancel) );
         size_t count = vlc_preparser_Cancel(parser, id);
         assert(count == 1);
-
     }
     vlc_sem_wait(&sem);
 


=====================================
test/src/input/thumbnail.c
=====================================
@@ -173,9 +173,9 @@ static void test_thumbnails( libvlc_instance_t* p_vlc )
 
 static void thumbnailer_callback_cancel( picture_t* p_thumbnail, void *data )
 {
-    (void) data; (void) p_thumbnail;
+    (void) data;
     /* This callback should not be called since the request is cancelled */
-    vlc_assert_unreachable();
+    assert( p_thumbnail == NULL );
 }
 
 static void test_cancel_thumbnail( libvlc_instance_t* p_vlc )



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/176e312e48d40cfceacc909ae6cdaea73d781cbf...b2883408befa737d6cfe8dd50e6ea7e2622a9d60

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/176e312e48d40cfceacc909ae6cdaea73d781cbf...b2883408befa737d6cfe8dd50e6ea7e2622a9d60
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list