[FFmpeg-devel] [PATCH 1/3] avcodec/avcodec: Add codec_tags array to AVCodec

[Michael Niedermayer]

On Tue, Jan 21, 2020 at 04:39:10PM -0300, James Almer wrote:
> On 1/21/2020 4:30 PM, Michael Niedermayer wrote:
> > On Tue, Jan 21, 2020 at 07:48:38PM +0100, Anton Khirnov wrote:
> >> Quoting Michael Niedermayer (2019-12-30 00:38:17)
> >>> This allows the fuzzer to target meaningfull codec tags instead
> >>> of hunting the 4gb space, which it seems to have problems with.
> >>>
> >>> Suggested-by: James
> >>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>> ---
> >>>  libavcodec/avcodec.h | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >>> index 119b32dc1f..b0c6a8f2e3 100644
> >>> --- a/libavcodec/avcodec.h
> >>> +++ b/libavcodec/avcodec.h
> >>> @@ -3634,6 +3634,12 @@ typedef struct AVCodec {
> >>>       * The user can only access this field via avcodec_get_hw_config().
> >>>       */
> >>>      const struct AVCodecHWConfigInternal **hw_configs;
> >>> +
> >>> +    /**
> >>> +     * List of supported codec_tags, terminated by CODEC_TAGS_END.
> >>> +     */
> >>> +    const uint32_t *codec_tags;
> >>> +#define CODEC_TAGS_END -1
> >>
> >> Is this supposed to be public or for fuzzer use only?
> >> If the latter, then CODEC_TAGS_END doesn't need to live in a public
> >> header.
> > 
> > the codec_tag field is public. So eventually the list of valid codec tags
> > should become too.
> 
> It's at the end of AVCodec, which is where the fields that must not be
> used or accessed from outside of lavc reside. They are documented as
> "can be removed at any time without warning and without bumping
> version". So basically, it's not public.

yes, it could either be moved up or a public function to access it
added when its decided to make it public


> 
> Since you are accessing them directly from the fuzzer anyway, might as
> well just assume the -1 termination and not bother adding a public
> define for it. If we ever want this public then it can be added, perhaps
> after a better design is found.

replacing a named constant by a litteral repeated -1 is making the code
worse. (maintaince and understanding wise)

So from the smell of this thread i dont think we will agree on a public
API and that also wasnt suggested nor is it needed ATM.

Thus i suggest we keep the codec_tags as in the original patch in the
private part and put the CODEC_TAGS_END  with a FF prefix in 
internal.h

That way it can be used in the fuzzer and we can in the future decide
if this or a different API could/should become public.

is that ok with everyone?

Thanks


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User
questions about the command line tools should be sent to the ffmpeg-user ML.
And questions about how to use libav* should be sent to the libav-user ML.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200121/ad6f48ef/attachment.sig>