[FFmpeg-devel] [PATCH 1/7] lavc: factor out encoder init/validation from avcodec_open2()

[Anton Khirnov]

Quoting James Almer (2021-03-10 16:17:35)
> On 3/10/2021 9:03 AM, Anton Khirnov wrote:
> > avcodec_open2() is massive, splitting it makes it more readable.
> > 
> > Also, add a missing error code to ticks_per_frame sanity check.
> > ---
> >   libavcodec/encode.c | 157 +++++++++++++++++++++++++++++++++++++++++
> >   libavcodec/encode.h |   6 ++
> >   libavcodec/utils.c  | 166 +-------------------------------------------
> >   3 files changed, 166 insertions(+), 163 deletions(-)
> > 
> > diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> > index 282337e453..bbf03d62fc 100644
> > --- a/libavcodec/encode.c
> > +++ b/libavcodec/encode.c
> > @@ -462,3 +462,160 @@ int attribute_align_arg avcodec_encode_video2(AVCodecContext *avctx,
> >       return ret;
> >   }
> >   #endif
> > +
> > +int ff_encode_preinit(AVCodecContext *avctx)
> 
> nit: Would prefer if this and ff_decode_preinit() could stay in the same 
> file as avcodec_open2() as static functions. This includes moving 
> decode_bsfs_init() there, too.
> decode.c and encode.c seem to me that they should contain functions used 
> during decoding and encoding, and not initialization.

For the record: discussed this on IRC and James dropped his objection to
the move

> 
> That being said, not related to this set and not really a priority, but 
> avcodec_open2() is not a "utility" function as much as a core lavc 
> function. av_get_bits_per_sample() for example is a util, as are 
> av_get_audio_frame_duration() and avcodec_align_dimensions2(). So 
> perhaps it, avcodec_alloc_context3() and avcodec_free_context() should 
> be together (options.c is also not exactly the best name for the file 
> currently hosting the latter two, so maybe it could be renamed to 
> avcodec.c while at it).

I would certainly be in favor of that. utils.c is way too huge.

-- 
Anton Khirnov