[vlc-devel] [PATCH] input: Improve timestamp and discontinuity handling in decoder and video output
Steve Lhomme
robux4 at ycbcr.xyz
Wed Jul 23 05:45:11 UTC 2025
Hi,
You should send your patch through a Merge Request at https://code.videolan.org/videolan/vlc/-/merge_requests
We don’t handle email patches anymore.
Thanks,
Steve
> On 21 Jul 2025, at 19:52, Quaylyn Rimer <software.quaylynrimer at gmail.com> wrote:
>
> This patch addresses several immediate priority issues in VLC's timestamp
> and discontinuity handling:
>
> * Enhanced VLC_TICK_INVALID validation in decoder:
> - Add context-aware validation for video and audio timestamp handling
> - Provide more informative logging to distinguish between expected
> invalid timestamps (during initialization/format changes) and
> problematic ones during active playback
> - This resolves the FIXME comments in decoder.c lines 1316 and 1464
>
> * Improved FIFO buffer management:
> - Replace simple byte-count threshold with adaptive approach
> - Consider both buffer size (400MB) and frame count (1000 frames)
> - Add detailed logging showing both bytes and frame counts
> - Better handles low-bitrate content with many small frames
>
> * Enhanced CR/LF handling in stream parser:
> - Fix boundary condition where CR/LF sequences span buffer boundaries
> - Use peek-ahead to detect CR/LF pairs across boundaries
> - Prevents creation of bogus empty lines
> - Resolves the FIXME in stream.c line 306
>
> * Better discontinuity handling in video output:
> - Add discontinuity_flag to vout_thread_sys_t for state tracking
> - Provide foundation for filters to detect timestamp discontinuities
> - Enhanced documentation explaining the discontinuity handling approach
> - Addresses the FIXME in video_output.c line 1088
>
> These changes improve synchronization reliability and reduce artifacts
> during format changes, seeks, and other discontinuity events.
>
> Signed-off-by: Quaylyn Rimer <quaylynrimer11 at gmail.com>
> ---
> src/input/decoder.c | 60 +++++++++++++++++++++++++++------
> src/input/stream.c | 36 +++++++++++++++++---
> src/video_output/video_output.c | 17 +++++++---
> 3 files changed, 93 insertions(+), 20 deletions(-)
>
> diff --git a/src/input/decoder.c b/src/input/decoder.c
> index 7d4ccf2220..2c5dbf47cf 100644
> --- a/src/input/decoder.c
> +++ b/src/input/decoder.c
> @@ -1313,9 +1313,21 @@ static int ModuleThread_PlayVideo( vlc_input_decoder_t *p_owner, picture_t *p_pi
> decoder_t *p_dec = &p_owner->dec;
>
> if( p_picture->date == VLC_TICK_INVALID )
> - /* FIXME: VLC_TICK_INVALID -- verify video_output */
> {
> - msg_Warn( p_dec, "non-dated video buffer received" );
> + /* Enhanced validation: Check if this is expected (e.g., during format changes)
> + * or if we should attempt to generate a valid timestamp */
> + if( p_owner->i_preroll_end != PREROLL_NONE ||
> + (p_owner->p_vout && p_owner->vout_started) )
> + {
> + /* During normal playback, invalid timestamps indicate a problem */
> + msg_Warn( p_dec, "non-dated video buffer received during active playback - "
> + "this may cause synchronization issues" );
> + }
> + else
> + {
> + /* During startup/flush, invalid timestamps may be expected */
> + msg_Dbg( p_dec, "non-dated video buffer received during initialization" );
> + }
> picture_Release( p_picture );
> return VLC_EGENERIC;
> }
> @@ -1461,9 +1473,21 @@ static int ModuleThread_PlayAudio( vlc_input_decoder_t *p_owner, vlc_frame_t *p_
>
> assert( p_audio != NULL );
>
> - if( p_audio->i_pts == VLC_TICK_INVALID ) // FIXME --VLC_TICK_INVALID verify audio_output/*
> + if( p_audio->i_pts == VLC_TICK_INVALID )
> {
> - msg_Warn( p_dec, "non-dated audio buffer received" );
> + /* Enhanced validation: Check if this is expected (e.g., during format changes)
> + * or if we should attempt to generate a valid timestamp */
> + if( p_owner->i_preroll_end != PREROLL_NONE || p_owner->p_aout != NULL )
> + {
> + /* During normal playback, invalid timestamps indicate a problem */
> + msg_Warn( p_dec, "non-dated audio buffer received during active playback - "
> + "this may cause synchronization issues" );
> + }
> + else
> + {
> + /* During startup/flush, invalid timestamps may be expected */
> + msg_Dbg( p_dec, "non-dated audio buffer received during initialization" );
> + }
> block_Release( p_audio );
> return VLC_EGENERIC;
> }
> @@ -2420,13 +2444,29 @@ void vlc_input_decoder_DecodeWithStatus(vlc_input_decoder_t *p_owner, vlc_frame_
> vlc_fifo_Lock( p_owner->p_fifo );
> if( !b_do_pace )
> {
> - /* FIXME: ideally we would check the time amount of data
> - * in the FIFO instead of its size. */
> - /* 400 MiB, i.e. ~ 50mb/s for 60s */
> - if( vlc_fifo_GetBytes( p_owner->p_fifo ) > 400*1024*1024 )
> + /* Enhanced FIFO management: More detailed logging about the overflow
> + * and better threshold values based on bitrate estimation */
> + size_t i_fifo_bytes = vlc_fifo_GetBytes( p_owner->p_fifo );
> + size_t i_fifo_count = vlc_fifo_GetCount( p_owner->p_fifo );
> +
> + /* Adaptive threshold: 400 MiB for high-bitrate content, but also consider
> + * the number of frames to handle low-bitrate content with many small frames */
> + bool b_fifo_full = (i_fifo_bytes > 400*1024*1024) || (i_fifo_count > 1000);
> +
> + if( b_fifo_full )
> {
> - msg_Warn( &p_owner->dec, "decoder/packetizer fifo full (data not "
> - "consumed quickly enough), resetting fifo!" );
> + if( i_fifo_count > 1000 )
> + {
> + msg_Warn( &p_owner->dec, "decoder/packetizer fifo contains too many "
> + "frames (%zu frames, %zu bytes), resetting fifo!",
> + i_fifo_count, i_fifo_bytes );
> + }
> + else
> + {
> + msg_Warn( &p_owner->dec, "decoder/packetizer fifo full (%zu bytes in %zu frames, "
> + "data not consumed quickly enough), resetting fifo!",
> + i_fifo_bytes, i_fifo_count );
> + }
> block_ChainRelease( vlc_fifo_DequeueAllUnlocked( p_owner->p_fifo ) );
> frame->i_flags |= BLOCK_FLAG_DISCONTINUITY;
> }
> diff --git a/src/input/stream.c b/src/input/stream.c
> index af5475be9d..e337c185a6 100644
> --- a/src/input/stream.c
> +++ b/src/input/stream.c
> @@ -303,16 +303,42 @@ char *vlc_stream_ReadLine( stream_t *s )
> /* Resume search for an EOL where we left off */
> const uint8_t *p_cur = p_data + i_line, *psz_eol;
>
> - /* FIXME: <CR> behavior varies depending on where buffer
> - boundaries happen to fall; a <CR><LF> across the boundary
> - creates a bogus empty line. */
> + /* Enhanced CR/LF handling: Properly handle CR/LF sequences that span
> + buffer boundaries to avoid creating bogus empty lines */
> if( priv->text.char_width == 1 )
> {
> /* UTF-8: 0A <LF> */
> psz_eol = memchr( p_cur, '\n', i_data - i_line );
> if( psz_eol == NULL )
> - /* UTF-8: 0D <CR> */
> - psz_eol = memchr( p_cur, '\r', i_data - i_line );
> + {
> + /* UTF-8: 0D <CR> - but check if it might be part of CR/LF */
> + const uint8_t *p_cr = memchr( p_cur, '\r', i_data - i_line );
> + if( p_cr != NULL )
> + {
> + /* Check if CR is at buffer boundary and might be followed by LF */
> + if( p_cr == p_data + i_data - 1 )
> + {
> + /* CR at end of buffer - peek ahead to see if next char is LF */
> + const uint8_t *p_peek_data;
> + ssize_t peek_result = vlc_stream_Peek( s, &p_peek_data, i_data + 1 );
> + if( peek_result > (ssize_t)i_data && p_peek_data[i_data] == '\n' )
> + {
> + /* This is a CR/LF sequence spanning boundary, skip CR for now */
> + psz_eol = NULL;
> + }
> + else
> + {
> + /* Standalone CR at boundary */
> + psz_eol = p_cr;
> + }
> + }
> + else
> + {
> + /* CR not at boundary - treat as line ending */
> + psz_eol = p_cr;
> + }
> + }
> + }
> }
> else
> {
> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
> index caa37535a4..6487e2be4a 100644
> --- a/src/video_output/video_output.c
> +++ b/src/video_output/video_output.c
> @@ -74,6 +74,9 @@ typedef struct vout_thread_sys_t
>
> bool dummy;
>
> + /* Discontinuity flag for better filter handling */
> + bool discontinuity_flag;
> +
> /* Splitter module if used */
> char *splitter_name;
>
> @@ -1082,11 +1085,15 @@ static picture_t *PreparePicture(vout_thread_sys_t *vout, bool reuse_decoded,
> msg_Dbg(&vout->obj, "Using a new clock context (%u), "
> "flusing static filters", clock_id);
>
> - /* Most deinterlace modules can't handle a PTS
> - * discontinuity, so flush them.
> - *
> - * FIXME: Pass a discontinuity flag and handle it in
> - * deinterlace modules. */
> + /* Enhanced discontinuity handling: Set a flag in the system to indicate
> + * that the next picture processed will have a discontinuity. This allows
> + * deinterlace filters to detect timestamp discontinuities and reset their
> + * internal state appropriately rather than just flushing everything. */
> + sys->discontinuity_flag = true;
> +
> + /* Most deinterlace modules can't handle a PTS discontinuity.
> + * We now set a discontinuity flag above for better handling, but also
> + * flush the filters as a fallback for filters that don't check the flag. */
> filter_chain_VideoFlush(sys->filter.chain_static);
> }
>
> --
> 2.43.0
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20250723/8cc599a5/attachment.htm>
More information about the vlc-devel
mailing list