[vlc-devel] [PATCH] splitter: use a different thread for each display
Rémi Denis-Courmont
remi at remlab.net
Mon Jan 20 09:04:21 CET 2020
Hi,
First, we need to decide how higher video than display frame rate should be handled in general. Current display plugins seem to assume that the core will drop frames automatically and aggressively, but that's probably not quite what actually happens.
Le 20 janvier 2020 09:18:25 GMT+02:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
>Hi,
>
>Upping this answer, should the OpenGL display execute the
>callbacks in its own thread instead then?
>
>Regards,
>--
>Alexandre Janniaux
>Videolabs
>
>On Wed, Dec 11, 2019 at 01:12:46PM +0200, Rémi Denis-Courmont wrote:
>> If you want thread barriers, use thread barriers, not semaphore.
>That's super inefficient, ironically for a performance-focused patch.
>>
>> It's also highly questionable idea anyway. This looks like a bug in
>the GL display, as this can't work anyway with high frame rate. Plus
>making multiple threads contend for internal resources of the display
>backend is likely to make performance worse rather than better.
>>
>> Le 11 décembre 2019 12:32:10 GMT+02:00, Alexandre Janniaux
><ajanni at videolabs.io> a écrit :
>> >Display modules are expected to present the image to the user from
>the
>> >display callback. Some display can achieve it asynchronously but
>others
>> >having VSYNC mechanisms like OpenGL are forcing the display to sleep
>> >until the next VSYNC. In a general way, similar display with a
>limited
>> >number of render buffer will wait until a new buffer gets released
>and
>> >available for processing.
>> >
>> >While it is correct for one display, the behaviour will make
>splitter
>> >managed display miss the screen frame deadline and generate
>stuttering
>> >with more than one output. It stems from display operations being
>> >executed sequentially for each output.
>> >
>> >Instead, parallelize and synchronize each display's prepare+display
>so
>> >that display taking VSYNC into account don't accumulate sleeping
>time.
>> >---
>> > modules/video_output/splitter.c | 132
>+++++++++++++++++++++++++++++---
>> > 1 file changed, 122 insertions(+), 10 deletions(-)
>> >
>> >diff --git a/modules/video_output/splitter.c
>> >b/modules/video_output/splitter.c
>> >index 8d6532ceb4..195bf2c50c 100644
>> >--- a/modules/video_output/splitter.c
>> >+++ b/modules/video_output/splitter.c
>> >@@ -34,23 +34,77 @@
>> > #include <vlc_codec.h>
>> > #include <vlc_vout_display.h>
>> > #include <vlc_video_splitter.h>
>> >+#include <vlc_threads.h>
>> >
>> > struct vlc_vidsplit_part {
>> > vout_window_t *window;
>> > vout_display_t *display;
>> >+ picture_t *picture;
>> >+ vlc_tick_t date;
>> > vlc_sem_t lock;
>> > unsigned width;
>> > unsigned height;
>> > };
>> >
>> >+struct vlc_vidsplit_thread
>> >+{
>> >+ vout_display_t *vd;
>> >+ struct vlc_vidsplit_part *part;
>> >+ vlc_thread_t thread;
>> >+};
>> >+
>> > struct vout_display_sys_t {
>> > video_splitter_t splitter;
>> > vlc_mutex_t lock;
>> >
>> > picture_t **pictures;
>> > struct vlc_vidsplit_part *parts;
>> >+ struct vlc_vidsplit_thread *threads;
>> >+
>> >+ vlc_sem_t prepare_wait;
>> >+ vlc_sem_t prepare_done;
>> >+ vlc_sem_t display_wait;
>> >+ vlc_sem_t display_done;
>> > };
>> >
>> >+static void* vlc_vidsplit_ThreadDisplay(void *data)
>> >+{
>> >+ struct vlc_vidsplit_thread *vd_thread = data;
>> >+ vout_display_t *vd = vd_thread->vd;
>> >+ vout_display_sys_t *sys = vd->sys;
>> >+ struct vlc_vidsplit_part *part = vd_thread->part;
>> >+
>> >+ for (;;)
>> >+ {
>> >+ vlc_sem_wait(&sys->prepare_wait);
>> >+
>> >+ /* We might need to stop the prepare just after the barrier
>> >+ * and we were not supposed to take the semaphore token. */
>> >+ if (part->display == NULL)
>> >+ {
>> >+ /* If we don't have a display, we can early exist. */
>> >+ vlc_sem_post(&sys->prepare_done);
>> >+ continue;
>> >+ }
>> >+
>> >+ part->picture = vout_display_Prepare(part->display,
>> >+ part->picture, NULL,
>> >+ part->date);
>> >+
>> >+ /* notify readiness */
>> >+ vlc_sem_post(&sys->prepare_done);
>> >+
>> >+ vlc_sem_wait(&sys->display_wait);
>> >+ if (part->picture)
>> >+ vout_display_Display(part->display, part->picture);
>> >+
>> >+ /* notify that image has been displayed */
>> >+ vlc_sem_post(&sys->display_done);
>> >+ }
>> >+
>> >+ return NULL;
>> >+}
>> >+
>> > static void vlc_vidsplit_Prepare(vout_display_t *vd, picture_t
>*pic,
>> > subpicture_t *subpic, vlc_tick_t
>date)
>> > {
>> >@@ -69,26 +123,61 @@ static void vlc_vidsplit_Prepare(vout_display_t
>> >*vd, picture_t *pic,
>> > }
>> > vlc_mutex_unlock(&sys->lock);
>> >
>> >+ /* After here, part are locked until all vout display has
>finished
>> >+ * displaying the picture. See vlc_vidsplit_Display. */
>> >+ for (int i = 0; i < sys->splitter.i_output; i++)
>> >+ vlc_sem_wait(&sys->parts[i].lock);
>> >+
>> >+ /* Now that all display are waiting, prepare their state and
>> >remove unused
>> >+ * pictures from killed display. */
>> > for (int i = 0; i < sys->splitter.i_output; i++) {
>> > struct vlc_vidsplit_part *part = &sys->parts[i];
>> >-
>> >- vlc_sem_wait(&part->lock);
>> >- sys->pictures[i] = vout_display_Prepare(part->display,
>> >- sys->pictures[i],
>> >NULL, date);
>> >+ part->picture = sys->pictures[i];
>> >+ part->date = date;
>> >+
>> >+ if (part->display == NULL)
>> >+ {
>> >+ /* The display died, cleanup. */
>> >+ part->picture = NULL;
>> >+ if (sys->pictures[i] != NULL)
>> >+ {
>> >+ picture_Release(sys->pictures[i]);
>> >+ sys->pictures[i] = NULL;
>> >+ }
>> >+ }
>> > }
>> >+
>> >+ /* Start preparing each vout display. */
>> >+ for (int i = 0; i < sys->splitter.i_output; ++i)
>> >+ if (sys->parts[i].display != NULL)
>> >+ vlc_sem_post(&sys->prepare_wait);
>> >+
>> >+ /* Wait for each vout display to have finished. */
>> >+ for (int i = 0; i < sys->splitter.i_output; ++i)
>> >+ if (sys->parts[i].display != NULL)
>> >+ vlc_sem_wait(&sys->prepare_done);
>> >+
>> >+ /* The time to prepare all vout display is the preparation time
>> >for the
>> >+ * slower of all display, which matches the core requirement.
>*/
>> > }
>> >
>> >static void vlc_vidsplit_Display(vout_display_t *vd, picture_t
>> >*picture)
>> > {
>> > vout_display_sys_t *sys = vd->sys;
>> >
>> >- for (int i = 0; i < sys->splitter.i_output; i++) {
>> >- struct vlc_vidsplit_part *part = &sys->parts[i];
>> >+ /* Request each output to display the picture. */
>> >+ for (int i = 0; i < sys->splitter.i_output; i++)
>> >+ if (sys->parts[i].display != NULL)
>> >+ vlc_sem_post(&sys->display_wait);
>> >
>> >- if (sys->pictures[i] != NULL)
>> >- vout_display_Display(part->display, sys->pictures[i]);
>> >- vlc_sem_post(&part->lock);
>> >- }
>> >+ /* Wait until every output has displayed a picture. */
>> >+ for (int i = 0; i < sys->splitter.i_output; i++)
>> >+ if (sys->parts[i].display != NULL)
>> >+ vlc_sem_wait(&sys->display_done);
>> >+
>> >+ /* Release parts lock, we can't read sys->parts[i] after that.
>*/
>> >+ for (int i = 0; i < sys->splitter.i_output; i++)
>> >+ vlc_sem_post(&sys->parts[i].lock);
>> >
>> > (void) picture;
>> > }
>> >@@ -115,6 +204,7 @@ static void vlc_vidsplit_Close(vout_display_t
>*vd)
>> >
>> > for (int i = 0; i < n; i++) {
>> > struct vlc_vidsplit_part *part = &sys->parts[i];
>> >+ struct vlc_vidsplit_thread *thread = &sys->threads[i];
>> > vout_display_t *display;
>> >
>> > vlc_sem_wait(&part->lock);
>> >@@ -122,6 +212,9 @@ static void vlc_vidsplit_Close(vout_display_t
>*vd)
>> > part->display = NULL;
>> > vlc_sem_post(&part->lock);
>> >
>> >+ vlc_cancel(thread->thread);
>> >+ vlc_join(thread->thread, NULL);
>> >+
>> > if (display != NULL)
>> > vout_display_Delete(display);
>> >
>> >@@ -259,12 +352,19 @@ static int vlc_vidsplit_Open(vout_display_t
>*vd,
>> > * sizeof (*sys->pictures));
>> > sys->parts = vlc_obj_malloc(obj,
>> > splitter->i_output * sizeof
>(*sys->parts));
>> >+ sys->threads = vlc_obj_malloc(obj,
>> >+ splitter->i_output * sizeof
>> >(*sys->threads));
>> > if (unlikely(sys->pictures == NULL || sys->parts == NULL)) {
>> > splitter->i_output = 0;
>> > vlc_vidsplit_Close(vd);
>> > return VLC_ENOMEM;
>> > }
>> >
>> >+ vlc_sem_init(&sys->prepare_wait, 0);
>> >+ vlc_sem_init(&sys->prepare_done, 0);
>> >+ vlc_sem_init(&sys->display_wait, 0);
>> >+ vlc_sem_init(&sys->display_done, 0);
>> >+
>> > for (int i = 0; i < splitter->i_output; i++) {
>> > const video_splitter_output_t *output =
>&splitter->p_output[i];
>> > vout_display_cfg_t vdcfg = {
>> >@@ -305,6 +405,18 @@ static int vlc_vidsplit_Open(vout_display_t
>*vd,
>> > part->display = display;
>> > vout_display_SetSize(display, part->width, part->height);
>> > vlc_sem_post(&part->lock);
>> >+
>> >+ sys->threads[i].vd = vd;
>> >+ sys->threads[i].part = part;
>> >+
>> >+ int ret_clone = vlc_clone(&sys->threads[i].thread,
>> >+ vlc_vidsplit_ThreadDisplay,
>> >+ &sys->threads[i],
>> >+ VLC_THREAD_PRIORITY_VIDEO);
>> >+
>> >+ if (ret_clone != VLC_SUCCESS) {
>> >+ /* TODO: Abort and cleanup */
>> >+ }
>> > }
>> >
>> > vd->prepare = vlc_vidsplit_Prepare;
>> >--
>> >2.24.1
>> >
>> >_______________________________________________
>> >vlc-devel mailing list
>> >To unsubscribe or modify your subscription options:
>> >https://mailman.videolan.org/listinfo/vlc-devel
>>
>> --
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
>excuser ma brièveté.
>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel
--
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200120/46e03ca9/attachment.html>
More information about the vlc-devel
mailing list