[vlc-commits] [Git][videolan/vlc][master] 4 commits: test: video_output: fix typo

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Sun Oct 2 17:42:28 UTC 2022



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
3d61b755 by Alexandre Janniaux at 2022-10-02T17:28:17+00:00
test: video_output: fix typo

- - - - -
621491b8 by Alexandre Janniaux at 2022-10-02T17:28:17+00:00
video_output: move variable in its usage scope

- - - - -
059b41d9 by Alexandre Janniaux at 2022-10-02T17:28:17+00:00
video_output: check picture != NULL before release

In the UpdateCurrentPicture function, it looks like the picture will
never be NULL because we check at the beginning of the function whether
it's NULL or not and only set it at the end.

However, UpdateCurrentPicture calls PreparePicture to make progress, and
this last function can call ChangeFilters, leading to the call to
FilterFlush. After this sequence, FilterFlush will set the picture
syys->displayed.current to NULL and the code will unwind back to the
UpdateCurrentPicture call, ready to release the picture.

PreparePicture should probably be made so that it doesn't reset the
current state, but it's far simpler to first fix the problem with a
check after PreparePicture, guarding picture_Release from NULL pointers
and then test the behaviour so that refactor doesn't break it.

- - - - -
a50390b5 by Alexandre Janniaux at 2022-10-02T17:28:17+00:00
test: input_decoder: send preliminary pictures

Send initial pictures to start-up the whole input_decoder state and
ensure it won't deadlock the es_out.

At the same time, it seems that updating the vout state from the decoder
(decoder_UpdateVideoOutput) is racy with the work of the video output
itself, so ensure the update is done once at the beginning for now. A
subsequent test will be provided once this race is fixed.

- - - - -


3 changed files:

- src/video_output/video_output.c
- test/src/input/decoder/input_decoder_scenarios.c
- test/src/video_output/video_output.c


Changes:

=====================================
src/video_output/video_output.c
=====================================
@@ -973,7 +973,7 @@ static picture_t *PreparePicture(vout_thread_sys_t *vout, bool reuse_decoded,
                         continue;
                     }
                 }
-                vlc_video_context *pic_vctx = picture_GetVideoContext(decoded);
+
                 if (!VideoFormatIsCropArEqual(&decoded->format, &sys->filter.src_fmt))
                 {
                     // we received an aspect ratio change
@@ -982,6 +982,7 @@ static picture_t *PreparePicture(vout_thread_sys_t *vout, bool reuse_decoded,
                     video_format_Copy(&sys->filter.src_fmt, &decoded->format);
                     if (sys->filter.src_vctx)
                         vlc_video_context_Release(sys->filter.src_vctx);
+                    vlc_video_context *pic_vctx = picture_GetVideoContext(decoded);
                     sys->filter.src_vctx = pic_vctx ? vlc_video_context_Hold(pic_vctx) : NULL;
 
                     ChangeFilters(vout);
@@ -1442,8 +1443,12 @@ static bool UpdateCurrentPicture(vout_thread_sys_t *sys)
     picture_t *next = PreparePicture(sys, false, false);
     if (next == NULL)
         return false;
+    /* We might have reset the current picture when preparing the next one,
+     * because filters had to be changed. In this case, avoid releasing the
+     * picture since it will lead to null pointer dereference errors. */
+    if (sys->displayed.current != NULL)
+        picture_Release(sys->displayed.current);
 
-    picture_Release(sys->displayed.current);
     sys->displayed.current = next;
 
     return true;


=====================================
test/src/input/decoder/input_decoder_scenarios.c
=====================================
@@ -33,6 +33,7 @@
 #include <vlc_player.h>
 #include <vlc_interface.h>
 #include <vlc_codec.h>
+#include <vlc_vout_display.h>
 
 #include "input_decoder.h"
 
@@ -44,6 +45,7 @@ static struct scenario_data
     struct vlc_video_context *decoder_vctx;
     bool skip_decoder;
     bool stream_out_sent;
+    size_t decoder_image_sent;
 } scenario_data;
 
 static void decoder_fixed_size(decoder_t *dec, vlc_fourcc_t chroma,
@@ -116,8 +118,27 @@ static int decoder_decode_check_flush_video(decoder_t *dec, picture_t *pic)
         return VLC_SUCCESS;
     }
 
-    int ret = decoder_UpdateVideoOutput(dec, NULL);
-    assert(ret == VLC_SUCCESS);
+    /* Only update the output format the first time. */
+    if (scenario_data.decoder_image_sent == 0)
+    {
+        int ret = decoder_UpdateVideoOutput(dec, NULL);
+        assert(ret == VLC_SUCCESS);
+    }
+
+    /* Workaround: the input decoder needs multiple frame (at least 2
+     * currently) to start correctly. We're not testing this, but we're
+     * testing the flush here, so provide additional picture at the
+     * beginning of the test to avoid issues with this.
+     * They must not be linked to any picture context. */
+    if (scenario_data.decoder_image_sent < 3)
+    {
+        msg_Info(dec, "Queueing workaround picture number %zu", scenario_data.decoder_image_sent);
+        decoder_QueueVideo(dec, pic);
+        scenario_data.decoder_image_sent++;
+        return VLC_SUCCESS;
+    }
+
+    /* Now the input decoder should be completely started. */
 
     struct picture_watcher_context context1 = {
         .context.destroy = context_destroy,
@@ -187,6 +208,10 @@ static void display_prepare_signal(vout_display_t *vd, picture_t *pic)
 {
     (void)vd;
 
+    if (pic->context == NULL)
+        return;
+
+    msg_Info(vd, "Signal that the frame has been prepared from display");
     struct picture_watcher_context *watcher =
         container_of(pic->context, struct picture_watcher_context, context);
     vlc_sem_post(&watcher->wait_prepare);
@@ -294,6 +319,7 @@ void input_decoder_scenario_init(void)
     scenario_data.decoder_vctx = NULL;
     scenario_data.skip_decoder = false;
     scenario_data.stream_out_sent = false;
+    scenario_data.decoder_image_sent = 0;
     vlc_sem_init(&scenario_data.wait_stop, 0);
     vlc_sem_init(&scenario_data.display_prepare_signal, 0);
     vlc_sem_init(&scenario_data.wait_ready_to_flush, 0);


=====================================
test/src/video_output/video_output.c
=====================================
@@ -238,7 +238,7 @@ static int OpenIntf(vlc_object_t *obj)
     msg_Info(intf, "Starting tests");
     while (current_scenario < vout_scenarios_count)
     {
-        msg_Info(intf, " - Running transcode scenario %zu", current_scenario);
+        msg_Info(intf, " - Running vout scenario %zu", current_scenario);
         play_scenario(intf, &vout_scenarios[current_scenario]);
         current_scenario++;
     }



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/0b13cf8b2c58f3e99909c426c2186ac5dfff01fe...a50390b5ed607308512aa324fcdf22d27a7e449a

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/0b13cf8b2c58f3e99909c426c2186ac5dfff01fe...a50390b5ed607308512aa324fcdf22d27a7e449a
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