Re: [FFmpeg-devel] [PATCH] Check codec_whitelist before reinitializing AVCtx.priv_data.

[Dale Curtis <dalecurtis@chromium.org>]

On Wed, Jul 31, 2024 at 2:10 PM Dale Curtis <dalecurtis@chromium.org> wrote:

> On Wed, Jul 31, 2024 at 4:32 AM Anton Khirnov <anton@khirnov.net> wrote:
>
>> Quoting Dale Curtis (2024-07-31 01:14:13)
>> > I realized there are a couple more allocations that can be skipped here
>> > when a codec is not on the allow list. Here's the updated patch.
>> >
>> > - dale
>> >
>> > On Mon, Jul 29, 2024 at 10:19 AM Dale Curtis <dalecurtis@chromium.org>
>> > wrote:
>> >
>> > > This ensures that if a codec isn't on codec_whitelist, its VUI
>> > > information can still be populated during find_stream_info()
>> > > via parsers.
>> > >
>> > > Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
>> > > ---
>> > >  libavcodec/avcodec.c | 12 ++++++------
>> > >  1 file changed, 6 insertions(+), 6 deletions(-)
>> > >
>> > >
>> >
>> > From f87042d77d13c4c45f4b800146dc16347c1007d4 Mon Sep 17 00:00:00 2001
>> > From: Dale Curtis <dalecurtis@chromium.org>
>> > Date: Tue, 30 Jul 2024 23:12:21 +0000
>> > Subject: [PATCH] Check codec_whitelist before reinitializing
>> AVCtx.priv_data.
>> >
>> > This ensures that if a codec isn't on codec_whitelist, its VUI
>> > information can still be populated during find_stream_info()
>> > via parsers.
>>
>> Can you elaborate on this?
>>
>
> The current code reinitializes the private data structures then checks the
> whitelist. If the private data section had already been filled out, it ends
> up being overwritten causing find_stream_info to drop side channel data.
>
>
>> >
>> > Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
>> > ---
>> >  libavcodec/avcodec.c | 11 +++++------
>> >  1 file changed, 5 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
>> > index 214dca4566..1f9b3eb360 100644
>> > --- a/libavcodec/avcodec.c
>> > +++ b/libavcodec/avcodec.c
>> > @@ -174,6 +174,11 @@ int attribute_align_arg
>> avcodec_open2(AVCodecContext *avctx, const AVCodec *code
>> >      if (avctx->extradata_size < 0 || avctx->extradata_size >=
>> FF_MAX_EXTRADATA_SIZE)
>> >          return AVERROR(EINVAL);
>> >
>> > +    if (avctx->codec_whitelist && av_match_list(codec->name,
>> avctx->codec_whitelist, ',') <= 0) {
>> > +        av_log(avctx, AV_LOG_ERROR, "Codec (%s) not on whitelist
>> \'%s\'\n", codec->name, avctx->codec_whitelist);
>> > +        return AVERROR(EINVAL);
>> > +    }
>>
>> I think this will break the case where the whitelist is provided in the
>> options dictionary, as it's not applied yet at this point.
>>
>
> I just copied the existing code a few lines up, so it seems ancillary to
> this patch, but I can fix it in this one too if you want.
>

Nevermind, I see what you're saying -- yes this does break that case.
Thanks for catching. I'll send a fix shortly.


>
>
>>
>> --
>> Anton Khirnov
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".