[vlc-commits] [Git][videolan/vlc][master] 4 commits: lib: downloader: split request creation from submission
François Cartegnie (@fcartegnie)
gitlab at videolan.org
Fri Sep 25 05:41:27 UTC 2026
François Cartegnie pushed to branch master at VideoLAN / VLC
Commits:
75e168a6 by Ayush Dey at 2026-09-25T05:00:39+00:00
lib: downloader: split request creation from submission
Replace libvlc_downloader_queue with libvlc_downloader_task_new and
libvlc_downloader_submit.
The previous API allocated a task and queued it in one step.
This prevented users from holding a task handle before it starts running.
In case of early task completion, caller would receive an already freed
libvlc_downloader_task handle (if they released the task from the
on_state_update callback on a terminal state).
- - - - -
69327bda by Ayush Dey at 2026-09-25T05:00:39+00:00
lib: downloader: skip on_subitems callback for empty media lists
- - - - -
22830e94 by Ayush Dey at 2026-09-25T05:00:39+00:00
lib: downloader: document callback reentrancy warning
- - - - -
80e8286e by Ayush Dey at 2026-09-25T05:00:39+00:00
lib: downloader: close the stream when the download thread exits
The stream is created as a child of the libvlc instance but was only
deleted on the last libvlc_downloader_task_release(). A caller holding
the task handle past libvlc_downloader_destroy() could trigger that
release after the downloader dropped the last instance reference,
running module_unneed() on an unloaded plugin and a freed logger.
Delete the stream at the end of the download thread instead, which
libvlc_downloader_destroy() joins before releasing the instance, so
releasing a task no longer depends on any libvlc object.
- - - - -
5 changed files:
- doc/libvlc/downloader.c
- include/vlc/libvlc_downloader.h
- lib/downloader.c
- lib/libvlc.sym
- test/libvlc/downloader.c
Changes:
=====================================
doc/libvlc/downloader.c
=====================================
@@ -83,7 +83,6 @@ static void on_state_update(void *opaque, libvlc_downloader_task *task,
status == libvlc_downloader_status_error)
{
sem_post(ctx->done_sem);
- libvlc_downloader_task_release(task);
}
}
@@ -215,20 +214,25 @@ int main(int argc, const char **argv)
.media = media,
};
- libvlc_downloader_task *task = libvlc_downloader_queue(downloader, &req, &cbs, &ctxs[i]);
+ libvlc_downloader_task *task = libvlc_downloader_task_new(downloader, &req, &cbs, &ctxs[i]);
libvlc_media_release(media);
if (task == NULL)
- {
- fprintf(stderr, "[req %d] failed to queue download for '%s'\n", i, url);
continue;
+
+ if (libvlc_downloader_submit(downloader, task) == 0)
+ {
+ fprintf(stdout, "[req %d, id=%p] queued: %s\n",
+ i, task, url);
+ fflush(stdout);
+ queued++;
}
+ else
+ fprintf(stderr, "[req %d] failed to queue download for '%s'\n", i, url);
- fprintf(stdout, "[req %d, id=%p] queued: %s\n",
- i, task, url);
- fflush(stdout);
- queued++;
+ /* Task isn't used anymore by the caller, so drop the reference right away. */
+ libvlc_downloader_task_release(task);
}
for (int i = 0; i < queued; ++i)
=====================================
include/vlc/libvlc_downloader.h
=====================================
@@ -46,12 +46,13 @@ typedef struct libvlc_downloader_request_t libvlc_downloader_request_t;
/**
* Opaque handle of a downloader task.
*
- * Identifies a task request submitted via libvlc_downloader_queue().
+ * Identifies a task created by libvlc_downloader_task_new() and started with
+ * libvlc_downloader_submit().
* It can be passed to libvlc_downloader_cancel() to cancel that request,
* or to libvlc_downloader_set_pause() to pause/resume that request.
*
- * \note Validity starts when libvlc_downloader_queue() returns a non-NULL handle
- * and ends with libvlc_downloader_task_release().
+ * \note Validity starts when libvlc_downloader_task_new() returns a non-NULL
+ * handle and ends with libvlc_downloader_task_release().
*/
typedef struct libvlc_downloader_task libvlc_downloader_task;
@@ -101,7 +102,7 @@ struct libvlc_downloader_cbs
* And avoid blocking operations in this callback as it is invoked with the internal lock held.
*
* \param opaque user data
- * \param task opaque handle returned by libvlc_downloader_queue()
+ * \param task opaque handle returned by libvlc_downloader_task_new()
* \param buf pointer to buffer (owned by downloader, only valid during callback)
* \param len size of buffer
* \param position total number of bytes read by the downloader so far
@@ -135,7 +136,7 @@ struct libvlc_downloader_cbs
* And avoid blocking operations in this callback as it is invoked with the internal lock held.
*
* \param opaque user data
- * \param task opaque handle returned by libvlc_downloader_queue()
+ * \param task opaque handle returned by libvlc_downloader_task_new()
* \param status download status
*/
void (*on_state_update)(void *opaque, libvlc_downloader_task *task,
@@ -147,8 +148,11 @@ struct libvlc_downloader_cbs
* \note Optional (can be NULL),
* available since version 0
*
+ * \warning Do not call any libvlc_downloader_* API from within this callback.
+ * Only libvlc_downloader_task_* APIs may be called on the provided task handle.
+ *
* \param opaque user data
- * \param task opaque handle returned by libvlc_downloader_queue()
+ * \param task opaque handle returned by libvlc_downloader_task_new()
* \param subitems media list of subitems (owned by LibVLC)
*/
void (*on_subitems)(void *opaque, libvlc_downloader_task *task,
@@ -160,8 +164,11 @@ struct libvlc_downloader_cbs
* \note Optional (can be NULL),
* available since version 0
*
+ * \warning Do not call any libvlc_downloader_* API from within this callback.
+ * Only libvlc_downloader_task_* APIs may be called on the provided task handle.
+ *
* \param opaque user data
- * \param task opaque handle returned by libvlc_downloader_queue()
+ * \param task opaque handle returned by libvlc_downloader_task_new()
* \param slaves array of libvlc_media_slave_t* (owned by LibVLC)
* \param count number of slaves
*/
@@ -187,8 +194,9 @@ struct libvlc_downloader_request_t
*
* - Only finite-size media are allowed to download.
*
- * - If the media is a playlist or directory, the user will be notified of the
- * subitems via the on_subitems callback and the download will not proceed.
+ * - If the media is a playlist or directory, the download will not proceed
+ * and the task terminates with the error state. If it has subitems, the
+ * user is notified of them via the on_subitems callback before that.
*
* - If the media is a livestream or unknown type, the download will error out.
*/
@@ -234,7 +242,30 @@ LIBVLC_API libvlc_downloader_t *
libvlc_downloader_new(libvlc_instance_t *inst, const struct libvlc_downloader_cfg *cfg);
/**
- * Download a media asynchronously.
+ * Create a media download task.
+ *
+ * This prepares a task handle for downloading a media. Nothing runs and no
+ * callback can fire until the handle is passed to
+ * libvlc_downloader_submit().
+ *
+ * \param downloader downloader instance
+ * \param req a pointer to a valid request struct
+ * \param cbs a pointer to a valid callbacks struct. The pointed struct
+ * must be kept alive (and not modified) by the caller until libvlc_downloader_cbs.on_state_update()
+ * is called for the returned task handle with a terminal state (finished/cancelled/error).
+ * \param cbs_opaque opaque pointer for callbacks
+ * \return NULL in case of error, or a task handle owned by the caller. It must
+ * be released with libvlc_downloader_task_release(), whether or not it is
+ * submitted.
+ *
+ * \version LibVLC 4.0.0 or later
+ */
+LIBVLC_API libvlc_downloader_task *
+libvlc_downloader_task_new(libvlc_downloader_t *downloader, const libvlc_downloader_request_t *req,
+ const struct libvlc_downloader_cbs *cbs, void *cbs_opaque);
+
+/**
+ * Start a task created by libvlc_downloader_task_new()
*
* - The downloader first parses the media.
*
@@ -248,29 +279,32 @@ libvlc_downloader_new(libvlc_instance_t *inst, const struct libvlc_downloader_cf
*
* - If the media is a file type with finite size, the download starts in a separate thread.
*
- * \param downloader downloader instance
- * \param req a pointer to a valid request struct
- * \param cbs a pointer to a valid callbacks struct. The pointed struct
- * must be kept alive (and not modified) by the caller until libvlc_downloader_cbs.on_state_update()
- * is called for the returned task handle with a terminal state (finished/cancelled/error).
- * \param cbs_opaque opaque pointer for callbacks
- * \return NULL in case of error, or a valid handle if the request was
- * scheduled for downloading.
+ * On success the task is scheduled and the callbacks are guaranteed to be called
+ * and the task will eventually report a terminal state (finished/cancelled/error).
+ * That callback may run even before this function returns.
+ *
+ * \note On failure no callback is invoked. The caller keeps its reference and
+ * may submit the task again.
*
- * \note No callbacks will be invoked if the return value is NULL.
+ * \note A task may be submitted at most once, and may only be re-submitted if
+ * the previous attempt failed. Submitting does not transfer the caller's
+ * reference. It keeps owning the handle and must release it.
+ *
+ * \param downloader the downloader the task was created from
+ * \param task a task that has not yet been successfully submitted
+ * \return 0 on success, -1 on error
*
* \version LibVLC 4.0.0 or later
*/
-LIBVLC_API libvlc_downloader_task *
-libvlc_downloader_queue(libvlc_downloader_t *downloader, const libvlc_downloader_request_t *req,
- const struct libvlc_downloader_cbs *cbs, void *cbs_opaque);
+LIBVLC_API int
+libvlc_downloader_submit(libvlc_downloader_t *downloader, libvlc_downloader_task *task);
/**
* Cancel an ongoing download.
*
* \param downloader downloader instance
- * \param task a downloader task returned by libvlc_downloader_queue(),
- * or NULL to cancel all requests.
+ * \param task a downloader task returned by libvlc_downloader_task_new(),
+ * or NULL to cancel all submitted requests.
*
* \return the number of requests cancelled
*
@@ -282,7 +316,8 @@ libvlc_downloader_queue(libvlc_downloader_t *downloader, const libvlc_downloader
* with the cancelled state.
*
* - If the request is already in a terminated state (finished, cancelled, or error),
- * the call is a no-op and no callback will be invoked.
+ * or if it was never submitted, the call is a no-op and no callback will be
+ * invoked.
*
* \version LibVLC 4.0.0 or later
*/
@@ -292,7 +327,7 @@ LIBVLC_API size_t libvlc_downloader_cancel(libvlc_downloader_t *downloader, libv
* Toggle pause/resume for the download.
*
* \param downloader downloader instance
- * \param task a valid downloader task returned by libvlc_downloader_queue()
+ * \param task a valid downloader task returned by libvlc_downloader_task_new()
* \param paused true to pause, false to resume
*
* \note This API is valid only when the download is in pending/running/paused state.
@@ -300,6 +335,12 @@ LIBVLC_API size_t libvlc_downloader_cancel(libvlc_downloader_t *downloader, libv
* state changes. Else, for finished/cancelled/error states, it's a no-op and
* no callback will be called.
*
+ * Pausing a pending task (including one not submitted yet) does not
+ * pause parsing. The on_subitems and on_slaves callbacks are still invoked,
+ * if available. If the download starts, the paused state is reported before
+ * any data is read (without a prior running state), and nothing is downloaded
+ * until the task is resumed.
+ *
* \version LibVLC 4.0.0 or later
*/
LIBVLC_API void libvlc_downloader_set_pause(libvlc_downloader_t *downloader,
@@ -320,7 +361,7 @@ LIBVLC_API void libvlc_downloader_destroy(libvlc_downloader_t *downloader);
/**
* Get the media associated with the downloader request handle.
*
- * \param task opaque handle returned by libvlc_downloader_queue()
+ * \param task opaque handle returned by libvlc_downloader_task_new()
* \return the media associated with the request handle.
*
* \note The returned media is held by the task, it must not be
@@ -337,14 +378,17 @@ libvlc_downloader_task_get_media(libvlc_downloader_task *task);
* \param task the downloader task handle
*
* \note
- * - The task handle is retained when returned by libvlc_downloader_queue().
+ * - The libvlc_downloader_task_new() call transfers the ownership of the task
+ * handle to the caller. It must be released whether or not it was submitted.
*
* - Mandatory to call to avoid memory leaks.
*
* - It is safe to call this API from within the on_state_update callback, when it
* reports a terminal state (finished, cancelled, error) \see libvlc_downloader_status_t.
*
- * - The task handle should not be used after calling this function.
+ * - It is safe to call this API at any time, including after
+ * libvlc_downloader_destroy() has been called on the downloader that
+ * created it. The task handle should not be used after calling this function.
*
* - If called on an active task, it doesn't cancel the task,
* use \ref libvlc_downloader_cancel() for that.
=====================================
lib/downloader.c
=====================================
@@ -54,8 +54,9 @@ struct libvlc_downloader_t
/* list of ongoing tasks (terminated tasks are removed) */
struct vlc_list submitted_tasks;
- /* list of downloader threads (dead threads joined at libvlc_downloader_queue,
- all threads are joined at libvlc_downloader_destroy) */
+ /* list of downloader threads (dead threads joined at
+ libvlc_downloader_submit, all threads are joined at
+ libvlc_downloader_destroy) */
struct vlc_list threads;
};
@@ -83,6 +84,9 @@ struct libvlc_downloader_task
/* required for ongoing task abortion */
bool interrupted;
+ /* guard against a re-submission of the same handle */
+ bool submitted;
+
vlc_atomic_rc_t rc;
struct libvlc_downloader_thread *thread;
@@ -112,6 +116,7 @@ DownloaderTaskNew(libvlc_downloader_t *downloader, libvlc_media_t *media,
task->pause_requested = false;
vlc_cond_init(&task->interrupt_cond);
task->interrupted = false;
+ task->submitted = false;
task->status = libvlc_downloader_status_pending;
task->parser_task = NULL;
task->thread = NULL;
@@ -131,9 +136,6 @@ DownloaderTaskDestroy(struct libvlc_downloader_task *task)
if (task->parser_task)
libvlc_parser_task_release(task->parser_task);
- if (task->s)
- vlc_stream_Delete(task->s);
-
free(task);
}
@@ -293,6 +295,9 @@ cleanup_locked:
task->thread->terminated = true;
vlc_mutex_unlock(&downloader->lock);
free(buf);
+ if (task->s != NULL)
+ vlc_stream_Delete(task->s);
+
libvlc_downloader_task_release(task);
return NULL;
}
@@ -337,7 +342,13 @@ static void notify_subitems(const struct libvlc_downloader_cbs *cbs, void *cbs_o
if (mlist == NULL)
return;
- cbs->on_subitems(cbs_opaque, task, mlist);
+ libvlc_media_list_lock(mlist);
+ int count = libvlc_media_list_count(mlist);
+ libvlc_media_list_unlock(mlist);
+
+ if (count > 0)
+ cbs->on_subitems(cbs_opaque, task, mlist);
+
libvlc_media_list_release(mlist);
}
@@ -420,8 +431,8 @@ static const struct libvlc_parser_cbs parser_cbs = {
};
libvlc_downloader_task *
-libvlc_downloader_queue(libvlc_downloader_t *downloader, const libvlc_downloader_request_t *req,
- const struct libvlc_downloader_cbs *cbs, void *cbs_opaque)
+libvlc_downloader_task_new(libvlc_downloader_t *downloader, const libvlc_downloader_request_t *req,
+ const struct libvlc_downloader_cbs *cbs, void *cbs_opaque)
{
assert(downloader != NULL);
assert(req != NULL && req->media != NULL);
@@ -451,6 +462,18 @@ libvlc_downloader_queue(libvlc_downloader_t *downloader, const libvlc_downloader
return NULL;
}
+ return task;
+}
+
+int
+libvlc_downloader_submit(libvlc_downloader_t *downloader, libvlc_downloader_task *task)
+{
+ assert(downloader != NULL);
+ assert(task != NULL);
+ assert(!task->submitted);
+
+ task->submitted = true;
+
vlc_mutex_lock(&downloader->lock);
vlc_list_append(&task->node, &downloader->submitted_tasks);
@@ -474,11 +497,14 @@ libvlc_downloader_queue(libvlc_downloader_t *downloader, const libvlc_downloader
vlc_mutex_lock(&downloader->lock);
vlc_list_remove(&task->node);
vlc_mutex_unlock(&downloader->lock);
- DownloaderTaskDestroy(task);
- return NULL;
+ task->submitted = false;
+ /* no callback will be invoked, drop the callback reference. The caller
+ keeps its own and may submit the task again. */
+ libvlc_downloader_task_release(task);
+ return -1;
}
- return task;
+ return 0;
}
size_t libvlc_downloader_cancel(libvlc_downloader_t *downloader, libvlc_downloader_task *task)
=====================================
lib/libvlc.sym
=====================================
@@ -121,7 +121,8 @@ libvlc_parser_cancel_request
libvlc_parser_task_get_media
libvlc_parser_task_release
libvlc_downloader_new
-libvlc_downloader_queue
+libvlc_downloader_task_new
+libvlc_downloader_submit
libvlc_downloader_cancel
libvlc_downloader_set_pause
libvlc_downloader_destroy
=====================================
test/libvlc/downloader.c
=====================================
@@ -189,6 +189,7 @@ static ptrdiff_t on_buffer(void *opaque, libvlc_downloader_task *task, const uin
static void on_state_update(void *opaque, libvlc_downloader_task *task, libvlc_downloader_status_t status)
{
+ (void)task;
struct test_ctx_t *ctx = opaque;
ctx->state_counts[status]++;
if (status == libvlc_downloader_status_paused)
@@ -196,10 +197,7 @@ static void on_state_update(void *opaque, libvlc_downloader_task *task, libvlc_d
if (status == libvlc_downloader_status_finished ||
status == libvlc_downloader_status_cancelled ||
status == libvlc_downloader_status_error)
- {
vlc_sem_post(&ctx->terminated_sem);
- libvlc_downloader_task_release(task);
- }
}
static void reset_ctx(struct test_ctx_t *ctx)
@@ -248,13 +246,24 @@ static void test_basic_download(libvlc_instance_t *vlc)
.media = media2,
};
- libvlc_downloader_task *task1 = libvlc_downloader_queue(downloader, &req1, &cbs, &ctx1);
- libvlc_downloader_task *task2 = libvlc_downloader_queue(downloader, &req2, &cbs, &ctx2);
+ libvlc_downloader_task *task1 = libvlc_downloader_task_new(downloader, &req1, &cbs, &ctx1);
+ libvlc_downloader_task *task2 = libvlc_downloader_task_new(downloader, &req2, &cbs, &ctx2);
assert(task1 != NULL);
assert(task2 != NULL);
assert(task1 != task2);
+ /* a task that is never submitted, no callback may fire, cancel is a
+ no-op and releasing it must free it */
+ struct test_ctx_t ctx3;
+ reset_ctx(&ctx3);
+ libvlc_downloader_task *task3 = libvlc_downloader_task_new(downloader, &req1, &cbs, &ctx3);
+ assert(task3 != NULL);
+ assert(libvlc_downloader_cancel(downloader, task3) == 0);
+
+ assert(libvlc_downloader_submit(downloader, task1) == 0);
+ assert(libvlc_downloader_submit(downloader, task2) == 0);
+
/* wait for both downloads to reach a terminal state */
vlc_sem_wait(&ctx1.terminated_sem);
vlc_sem_wait(&ctx2.terminated_sem);
@@ -275,6 +284,13 @@ static void test_basic_download(libvlc_instance_t *vlc)
assert(ctx1.total_bytes == TEST_TOTAL_BYTES);
assert(ctx2.total_bytes == TEST_TOTAL_BYTES);
+ /* the unsubmitted task never ran */
+ assert(ctx3.state_counts[libvlc_downloader_status_running] == 0);
+ assert(ctx3.buffer_cb_calls == 0);
+
+ libvlc_downloader_task_release(task1);
+ libvlc_downloader_task_release(task2);
+ libvlc_downloader_task_release(task3);
libvlc_downloader_destroy(downloader);
libvlc_media_release(media1);
libvlc_media_release(media2);
@@ -302,8 +318,9 @@ static void test_pause_resume(libvlc_instance_t *vlc)
.media = media,
};
- libvlc_downloader_task *task = libvlc_downloader_queue(downloader, &req, &cbs, &ctx);
+ libvlc_downloader_task *task = libvlc_downloader_task_new(downloader, &req, &cbs, &ctx);
assert(task != NULL);
+ assert(libvlc_downloader_submit(downloader, task) == 0);
/* wait for a couple of buffer callbacks before pausing */
for (int i = 0; i < 2; ++i)
@@ -330,6 +347,7 @@ static void test_pause_resume(libvlc_instance_t *vlc)
assert(ctx.buffer_data_ok);
assert(ctx.total_bytes == TEST_TOTAL_BYTES);
+ libvlc_downloader_task_release(task);
libvlc_downloader_destroy(downloader);
libvlc_media_release(media);
}
@@ -356,8 +374,9 @@ static void test_cancel_download(libvlc_instance_t *vlc)
.media = media,
};
- libvlc_downloader_task *task = libvlc_downloader_queue(downloader, &req, &cbs, &ctx);
+ libvlc_downloader_task *task = libvlc_downloader_task_new(downloader, &req, &cbs, &ctx);
assert(task != NULL);
+ assert(libvlc_downloader_submit(downloader, task) == 0);
int progress_for_cancel = 2; /* cancel after 2 calls of buffer callback */
@@ -372,6 +391,7 @@ static void test_cancel_download(libvlc_instance_t *vlc)
assert(ctx.state_counts[libvlc_downloader_status_cancelled] == 1);
+ libvlc_downloader_task_release(task);
libvlc_downloader_destroy(downloader);
libvlc_media_release(media);
}
@@ -399,6 +419,7 @@ static void partial_ctx_init(struct partial_ctx_t *ctx)
static void partial_on_state(void *opaque, libvlc_downloader_task *task,
libvlc_downloader_status_t status)
{
+ (void)task;
struct partial_ctx_t *ctx = opaque;
ctx->state_counts[status]++;
if (status == libvlc_downloader_status_paused)
@@ -406,10 +427,7 @@ static void partial_on_state(void *opaque, libvlc_downloader_task *task,
if (status == libvlc_downloader_status_finished ||
status == libvlc_downloader_status_cancelled ||
status == libvlc_downloader_status_error)
- {
vlc_sem_post(&ctx->terminated_sem);
- libvlc_downloader_task_release(task);
- }
}
static ptrdiff_t partial_on_buffer(void *opaque, libvlc_downloader_task *task,
@@ -464,8 +482,9 @@ static void test_partial_read(libvlc_instance_t *vlc)
.media = media,
};
- libvlc_downloader_task *task = libvlc_downloader_queue(downloader, &req, &cbs, &ctx);
+ libvlc_downloader_task *task = libvlc_downloader_task_new(downloader, &req, &cbs, &ctx);
assert(task != NULL);
+ assert(libvlc_downloader_submit(downloader, task) == 0);
/* wait for the first (partial-accept) callback, then for auto-pause */
vlc_sem_wait(&ctx.first_buffer_sem);
@@ -486,6 +505,7 @@ static void test_partial_read(libvlc_instance_t *vlc)
size_t expected_residual = ctx.first_len - (ctx.first_len / 2);
assert(ctx.second_len == expected_residual);
+ libvlc_downloader_task_release(task);
libvlc_downloader_destroy(downloader);
libvlc_media_release(media);
}
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/d7e1cd1ef080ae72b9dfd15913470708b416baa6...80e8286e7eaaf5a165c834731104868df3522018
--
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/d7e1cd1ef080ae72b9dfd15913470708b416baa6...80e8286e7eaaf5a165c834731104868df3522018
You're receiving this email because of your account on code.videolan.org. Manage all notifications: https://code.videolan.org/-/profile/notifications | Help: https://code.videolan.org/help
More information about the vlc-commits
mailing list