Tristan Matthews pushed to branch master at VideoLAN / VLC


Commits:
68a9ddde by Alaric Senat at 2023-01-26T15:55:06+00:00
transcode: make pcr forwarding optional

This patch make it possible to disable the PCR forwarding
synchronisation algorithm as it is still prone to bugs and will likely
fail on wrong encoder outputs.

The PCR forwarding is still enabled by default.

Refs #27730

- - - - -
ec5e344a by Alaric Senat at 2023-01-26T15:55:06+00:00
transcode: pcr_sync: switch assert to runtime error

This assertion was too severe and deserve actual runtime handling.

The `pcr_helper` should just fail in case of inconsistent
input/output. This can happen in various case such as bogus encoder or
simply internal error in the `pcr_helper` code.

This patch is making sure the `pcr_helper` user can disable it in case
of failure.

Refs #27730

- - - - -
2c67e7af by Alaric Senat at 2023-01-26T15:55:06+00:00
test: pcr_sync: check IO mismatch error

- - - - -
d80e9d0c by Alaric Senat at 2023-01-26T15:55:06+00:00
transcode: disable pcr forwarding on error

`SignalLeavingFrame` returning an error usually means that either the
decoder outputs an inconsistent stream timestamp-wise or that the PCR
sync algorithm had a mistake in its implementation.

Both case shouldn't ignore the error, from that point, the PCR
output should be considered flawed and PCR output from transcode
should be stopped with a proper error message.

Refs #27730

- - - - -


5 changed files:

- modules/stream_out/transcode/pcr_helper.c
- modules/stream_out/transcode/pcr_helper.h
- modules/stream_out/transcode/transcode.c
- modules/stream_out/transcode/transcode.h
- test/modules/stream_out/pcr_sync.c


Changes:

=====================================
modules/stream_out/transcode/pcr_helper.c
=====================================
@@ -155,10 +155,14 @@ int 
transcode_track_pcr_helper_SignalEnteringFrame(transcode_track_pcr_helper_t
     return VLC_SUCCESS;
 }
 
-vlc_tick_t 
transcode_track_pcr_helper_SignalLeavingFrame(transcode_track_pcr_helper_t 
*pcr_helper,
-                                                         const vlc_frame_t 
*frame)
+int transcode_track_pcr_helper_SignalLeavingFrame(transcode_track_pcr_helper_t 
*pcr_helper,
+                                                  const vlc_frame_t *frame,
+                                                  vlc_tick_t *pcr)
 {
-    assert(!vlc_list_is_empty(&pcr_helper->delayed_frames_data));
+    *pcr = VLC_TICK_INVALID;
+
+    if (vlc_list_is_empty(&pcr_helper->delayed_frames_data))
+        return VLC_EGENERIC;
 
     pcr_helper->last_dts_output = frame->i_dts;
 
@@ -166,7 +170,6 @@ vlc_tick_t 
transcode_track_pcr_helper_SignalLeavingFrame(transcode_track_pcr_hel
     const vlc_tick_t output_media_time =
         pcr_helper->input_media_time - pcr_helper->held_media_time;
 
-    vlc_tick_t pcr = VLC_TICK_INVALID;
     delayed_frame_data_t *frame_data;
     vlc_list_foreach(frame_data, &pcr_helper->delayed_frames_data, node)
     {
@@ -176,11 +179,14 @@ vlc_tick_t 
transcode_track_pcr_helper_SignalLeavingFrame(transcode_track_pcr_hel
         const vlc_tick_t current_pcr =
             transcode_track_pcr_helper_GetFramePCR(pcr_helper, 
frame_data->dts);
         if (current_pcr != VLC_TICK_INVALID)
-            pcr = current_pcr;
+            *pcr = current_pcr;
 
         vlc_list_remove(&frame_data->node);
         free(frame_data);
     }
-    return (pcr == VLC_TICK_INVALID) ? VLC_TICK_INVALID
-                                     : __MIN(frame->i_dts, pcr);
+
+    if (*pcr != VLC_TICK_INVALID)
+        *pcr = __MIN(frame->i_dts, *pcr);
+
+    return VLC_SUCCESS;
 }


=====================================
modules/stream_out/transcode/pcr_helper.h
=====================================
@@ -80,11 +80,14 @@ int 
transcode_track_pcr_helper_SignalEnteringFrame(transcode_track_pcr_helper_t
  * \note This can be called multiple times to handle multiple following PCR's.
  * \note If the frame output has seen its timestamp altered, this helper will 
synthetise a new PCR.
  *
- * \param frame The leaving frame (will be treated as a single frame).
+ * \param[in] frame The leaving frame (will be treated as a single frame).
+ * \param[out] pcr The PCR value following the frame or VLC_TICK_INVALID if no 
PCR is following.
  *
- * \return The PCR value following the frame or VLC_TICK_INVALID if no PCR is 
following.
+ * \retval VLC_SUCCESS On success (even if no PCR is to be forwarded).
+ * \retval VLC_EGENERIC On fatal frame mismatch error.
  */
-vlc_tick_t 
transcode_track_pcr_helper_SignalLeavingFrame(transcode_track_pcr_helper_t *,
-                                                         const vlc_frame_t 
*frame);
+int transcode_track_pcr_helper_SignalLeavingFrame(transcode_track_pcr_helper_t 
*,
+                                                  const vlc_frame_t *frame,
+                                                  vlc_tick_t *pcr);
 
 #endif


=====================================
modules/stream_out/transcode/transcode.c
=====================================
@@ -131,6 +131,9 @@
 #define POOL_TEXT N_("Picture pool size")
 #define POOL_LONGTEXT N_( "Defines how many pictures we allow to be in pool "\
     "between decoder/encoder threads when threads > 0" )
+#define FORWARD_PCR_TEXT N_( "Forward PCR" )
+#define FORWARD_PCR_LONGTEXT N_( \
+    "Enable PCR events forwarding to the next stream." )
 
 
 /* Note: Skip adding translated accompanying labels - too technical, not worth 
it */
@@ -212,6 +215,8 @@ vlc_module_begin ()
     add_integer( SOUT_CFG_PREFIX "pool-size", 10, POOL_TEXT, POOL_LONGTEXT )
         change_integer_range( 1, 1000 )
     add_obsolete_bool( SOUT_CFG_PREFIX "high-priority" ) // Since 4.0.0
+    add_bool( SOUT_CFG_PREFIX "forward-pcr", true, FORWARD_PCR_TEXT,
+              FORWARD_PCR_LONGTEXT )
 
 vlc_module_end ()
 
@@ -221,7 +226,7 @@ static const char *const ppsz_sout_options[] = {
     "deinterlace-module", "threads", "aenc", "acodec", "ab", "alang",
     "afilter", "samplerate", "channels", "senc", "scodec", "soverlay",
     "sfilter", "high-priority", "maxwidth", "maxheight", "pool-size",
-    NULL
+    "forward-pcr", NULL
 };
 
 /*****************************************************************************
@@ -386,11 +391,20 @@ static int Open( vlc_object_t *p_this )
     config_ChainParse( p_stream, SOUT_CFG_PREFIX, ppsz_sout_options,
                    p_stream->p_cfg );
 
-    p_sys->pcr_sync = vlc_pcr_sync_New();
-    if( unlikely( p_sys->pcr_sync == NULL ) )
+    p_sys->pcr_forwarding_enabled =
+        var_GetBool( p_stream, SOUT_CFG_PREFIX "forward-pcr" );
+    if( p_sys->pcr_forwarding_enabled )
     {
-        free( p_sys );
-        return VLC_ENOMEM;
+        p_sys->pcr_sync = vlc_pcr_sync_New();
+        if( unlikely(p_sys->pcr_sync == NULL) )
+        {
+            free( p_sys );
+            return VLC_ENOMEM;
+        }
+    }
+    else
+    {
+        p_sys->pcr_sync = NULL;
     }
     p_sys->first_pcr_sent = false;
     p_sys->pcr_sync_has_input = false;
@@ -488,7 +502,8 @@ static void Close( vlc_object_t * p_this )
 
     transcode_encoder_config_clean( &p_sys->senc_cfg );
 
-    vlc_pcr_sync_Delete( p_sys->pcr_sync );
+    if( p_sys->pcr_sync != NULL )
+        vlc_pcr_sync_Delete( p_sys->pcr_sync );
 
     free( p_sys );
 }
@@ -661,14 +676,20 @@ static void *Add( sout_stream_t *p_stream, const 
es_format_t *p_fmt )
     if(!success)
         goto error;
 
-    if( id->b_transcode )
+    if( !id->b_transcode )
+    {
+        id->pcr_helper = NULL;
+        return id;
+    }
+
+    ++p_sys->transcoded_stream_nb;
+
+    if( p_sys->pcr_forwarding_enabled )
     {
         // TODO properly estimate the delay
         id->pcr_helper = transcode_track_pcr_helper_New( p_sys->pcr_sync, 
VLC_TICK_FROM_SEC( 4 ) );
         if( unlikely( id->pcr_helper == NULL ) )
             goto error;
-
-        ++p_sys->transcoded_stream_nb;
     }
     else
     {
@@ -721,7 +742,8 @@ static void Del( sout_stream_t *p_stream, void *_id )
         default:
             break;
         }
-        transcode_track_pcr_helper_Delete( id->pcr_helper );
+        if( id->pcr_helper != NULL )
+            transcode_track_pcr_helper_Delete( id->pcr_helper );
         --p_sys->transcoded_stream_nb;
     }
     else
@@ -748,11 +770,11 @@ static int Send( sout_stream_t *p_stream, void *_id, 
block_t *p_buffer )
             goto error;
     }
 
-    if( p_buffer != NULL )
+    sout_stream_sys_t *sys = p_stream->p_sys;
+    if( p_buffer != NULL && sys->pcr_forwarding_enabled )
     {
         assert( p_buffer->p_next == NULL );
 
-        sout_stream_sys_t *sys = p_stream->p_sys;
         if( !sys->pcr_sync_has_input )
             sys->pcr_sync_has_input = true;
 
@@ -789,8 +811,19 @@ static int Send( sout_stream_t *p_stream, void *_id, 
block_t *p_buffer )
         block_t *next = it->p_next;
         it->p_next = NULL;
 
-        const vlc_tick_t pcr =
-            transcode_track_pcr_helper_SignalLeavingFrame( id->pcr_helper, it 
);
+        vlc_tick_t pcr = VLC_TICK_INVALID;
+        if( sys->pcr_forwarding_enabled )
+        {
+            const int status = transcode_track_pcr_helper_SignalLeavingFrame(
+                id->pcr_helper, it, &pcr );
+            if( status != VLC_SUCCESS )
+            {
+                msg_Err( p_stream,
+                         "Failed to match transcode input with encoder output. 
"
+                         "Disabling PCR forwarding..." );
+                sys->pcr_forwarding_enabled = false;
+            }
+        }
 
         if( sout_StreamIdSend( p_stream->p_next, id->downstream_id, it ) != 
VLC_SUCCESS )
         {
@@ -819,6 +852,10 @@ error:
 static void SetPCR( sout_stream_t *stream, vlc_tick_t pcr )
 {
     sout_stream_sys_t *sys = stream->p_sys;
+
+    if( !sys->pcr_forwarding_enabled )
+        return;
+
     if( sys->transcoded_stream_nb == 0)
     {
         sout_StreamSetPCR( stream->p_next, pcr );


=====================================
modules/stream_out/transcode/transcode.h
=====================================
@@ -65,6 +65,7 @@ typedef struct
     /* Spu's video */
     sout_stream_id_sys_t *id_video;
 
+    bool pcr_forwarding_enabled;
     vlc_pcr_sync_t *pcr_sync;
     bool first_pcr_sent;
     bool pcr_sync_has_input;


=====================================
test/modules/stream_out/pcr_sync.c
=====================================
@@ -350,8 +350,10 @@ static void test_PCRHelperSimple(vlc_pcr_sync_t *sync,
     assert(vlc_pcr_sync_SignalPCR(sync, 10u) == VLC_SUCCESS);
 
     const vlc_frame_t out_frame = {.i_dts = 2000u, .i_length = 10u};
-    const vlc_tick_t pcr =
-        transcode_track_pcr_helper_SignalLeavingFrame(pcr_helper, &out_frame);
+    vlc_tick_t pcr;
+    const int status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame, &pcr);
+    assert(status == VLC_SUCCESS);
     assert(pcr == 10u);
 }
 
@@ -380,11 +382,14 @@ test_PCRHelperMultipleTracks(vlc_pcr_sync_t *sync,
             assert(vlc_pcr_sync_SignalPCR(sync, 101) == VLC_SUCCESS);
     }
 
+    vlc_tick_t pcr;
     // Unqueue the audio frames first to mimic fast decoding
     for (vlc_tick_t dts = 0; dts < 200; dts += 10)
     {
         const vlc_frame_t a_frame = {.i_dts = dts + 1, .i_length = 10u};
-        const vlc_tick_t pcr = 
transcode_track_pcr_helper_SignalLeavingFrame(audio, &a_frame);
+        const int status = transcode_track_pcr_helper_SignalLeavingFrame(
+            audio, &a_frame, &pcr);
+        assert(status == VLC_SUCCESS);
         assert(pcr == VLC_TICK_INVALID);
     }
 
@@ -392,7 +397,9 @@ test_PCRHelperMultipleTracks(vlc_pcr_sync_t *sync,
     {
         const vlc_frame_t v_frame = {.i_dts = (dts * 2) - 10 + 1,
                                      .i_length = 20u};
-        const vlc_tick_t pcr = 
transcode_track_pcr_helper_SignalLeavingFrame(video, &v_frame);
+        const int status = transcode_track_pcr_helper_SignalLeavingFrame(
+            video, &v_frame, &pcr);
+        assert(status == VLC_SUCCESS);
         if (dts != 50)
             assert(pcr == VLC_TICK_INVALID);
         else
@@ -416,11 +423,15 @@ test_PCRHelperSplitFrameOutput(vlc_pcr_sync_t *sync,
 
     const vlc_frame_t out_frame1 = {.i_dts = 2000u, .i_length = 5u};
     const vlc_frame_t out_frame2 = {.i_dts = 2005u, .i_length = 5u};
-    vlc_tick_t pcr =
-        transcode_track_pcr_helper_SignalLeavingFrame(pcr_helper, &out_frame1);
+    vlc_tick_t pcr;
+    int status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame1, &pcr);
+    assert(status == VLC_SUCCESS);
     assert(pcr == VLC_TICK_INVALID);
-    pcr =
-        transcode_track_pcr_helper_SignalLeavingFrame(pcr_helper, &out_frame2);
+
+    status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame2, &pcr);
+    assert(status == VLC_SUCCESS);
     assert(pcr == 10u);
 }
 
@@ -446,13 +457,41 @@ test_PCRHelperSplitFrameInput(vlc_pcr_sync_t *sync,
     assert(dropped_frame_pcr == VLC_TICK_INVALID);
 
     const vlc_frame_t out_frame1 = {.i_dts = 1u, .i_length = 20u};
-    vlc_tick_t pcr =
-        transcode_track_pcr_helper_SignalLeavingFrame(pcr_helper, &out_frame1);
+    vlc_tick_t pcr;
+    int status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame1, &pcr);
+    assert(status == VLC_SUCCESS);
     // PCR should be re-synthetized.
     assert(pcr == 1u);
     const vlc_frame_t out_frame2 = {.i_dts = 21u, .i_length = 10u};
-    pcr =
-        transcode_track_pcr_helper_SignalLeavingFrame(pcr_helper, &out_frame2);
+    status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame2, &pcr);
+    assert(status == VLC_SUCCESS);
+    assert(pcr == VLC_TICK_INVALID);
+}
+
+static void test_PCRHelperIOMismatch(vlc_pcr_sync_t *sync,
+                                     transcode_track_pcr_helper_t *pcr_helper)
+{
+    const vlc_frame_t in_frame = {.i_dts = 1u, .i_length = 10u};
+    vlc_tick_t dropped_frame_pcr;
+    transcode_track_pcr_helper_SignalEnteringFrame(
+        pcr_helper, &in_frame, &dropped_frame_pcr);
+    assert(dropped_frame_pcr == VLC_TICK_INVALID);
+
+    assert(vlc_pcr_sync_SignalPCR(sync, 10u) == VLC_SUCCESS);
+
+    const vlc_frame_t out_frame1 = {.i_dts = 2000u, .i_length = 10u};
+    const vlc_frame_t out_frame2 = {.i_dts = 2010u, .i_length = 10u};
+    vlc_tick_t pcr;
+    int status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame1, &pcr);
+    assert(status == VLC_SUCCESS);
+    assert(pcr == 10u);
+
+    status = transcode_track_pcr_helper_SignalLeavingFrame(
+        pcr_helper, &out_frame2, &pcr);
+    assert(status == VLC_EGENERIC);
     assert(pcr == VLC_TICK_INVALID);
 }
 
@@ -474,4 +513,5 @@ int main()
     test_PCRHelper(test_PCRHelperMultipleTracks);
     test_PCRHelper(test_PCRHelperSplitFrameOutput);
     test_PCRHelper(test_PCRHelperSplitFrameInput);
+    test_PCRHelper(test_PCRHelperIOMismatch);
 }



View it on GitLab: 
https://code.videolan.org/videolan/vlc/-/compare/a17b8d5be1dc3b0845401be8fffbd17877fa04e5...d80e9d0c057dfcd223aea94f83fb6ee7c668c16c

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


VideoLAN code repository instance
_______________________________________________
vlc-commits mailing list
vlc-commits@videolan.org
https://mailman.videolan.org/listinfo/vlc-commits

Reply via email to