[FFmpeg-devel] [PATCH 15/15] fftools/sync_queue: make sure non-limiting streams are not used as queue head
James Almer
jamrial at gmail.com
Wed May 24 15:01:47 EEST 2023
On 5/23/2023 10:58 AM, Anton Khirnov wrote:
> A non-limiting stream could mistakenly end up being the queue head,
> which would then produce incorrect synchronization, seen e.g. in
> fate-matroska-flac-extradata-update for certain number of frame threads
> (e.g. 5).
>
> Found-By: James Almer
Strictly speaking, it was FATE.
> ---
> fftools/sync_queue.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/fftools/sync_queue.c b/fftools/sync_queue.c
> index c0f33e9235..bc107ba4fe 100644
> --- a/fftools/sync_queue.c
> +++ b/fftools/sync_queue.c
> @@ -217,17 +217,26 @@ static void finish_stream(SyncQueue *sq, unsigned int stream_idx)
>
> static void queue_head_update(SyncQueue *sq)
> {
> + av_assert0(sq->have_limiting);
> +
> if (sq->head_stream < 0) {
> + unsigned first_limiting = UINT_MAX;
> +
> /* wait for one timestamp in each stream before determining
> * the queue head */
> for (unsigned int i = 0; i < sq->nb_streams; i++) {
> SyncQueueStream *st = &sq->streams[i];
> - if (st->limiting && st->head_ts == AV_NOPTS_VALUE)
> + if (!st->limiting)
> + continue;
> + if (st->head_ts == AV_NOPTS_VALUE)
> return;
> + if (first_limiting == UINT_MAX)
> + first_limiting = i;
> }
>
> // placeholder value, correct one will be found below
> - sq->head_stream = 0;
> + av_assert0(first_limiting < UINT_MAX);
> + sq->head_stream = first_limiting;
> }
>
> for (unsigned int i = 0; i < sq->nb_streams; i++) {
Can confirm it fixes the issue.
More information about the ffmpeg-devel
mailing list