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

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

[Dale Curtis]

[-- Attachment #1: Type: text/plain, Size: 284 bytes --]

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(-)

[-- Attachment #2: no_reinit.patch --]
[-- Type: application/octet-stream, Size: 2071 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[Dale Curtis]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]

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(-)
>
>

[-- Attachment #2: no_reinit_v2.patch --]
[-- Type: application/octet-stream, Size: 2105 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[Anton Khirnov]

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?
> 
> 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.

-- 
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".

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

[Dale Curtis]

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.


>
> --
> 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".

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

[Dale Curtis]

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".

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

[Dale Curtis]

[-- Attachment #1: Type: text/plain, Size: 3384 bytes --]

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

> 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.
>

Fixed. Thanks.


>
>
>>
>>
>>>
>>> --
>>> 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".
>>>
>>

[-- Attachment #2: no_reinit_v3.patch --]
[-- Type: application/octet-stream, Size: 2124 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

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

[Dale Curtis]

Bump for this one. Thanks!

- dale

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

> On Wed, Jul 31, 2024 at 2:29 PM Dale Curtis <dalecurtis@chromium.org>
> wrote:
>
>> 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.
>>
>
> Fixed. Thanks.
>
>
>>
>>
>>>
>>>
>>>>
>>>> --
>>>> 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".