[FFmpeg-devel] [PATCH 1/3 v2] avcodec/encode: restructure the core encoding code

[James Almer]

On 3/26/2020 5:43 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-03-16 22:30:00)
>> This commit follows the same logic as 061a0c14bb, but for the encode API: The
>> new public encoding API will no longer be a wrapper around the old deprecated
>> one, and the internal API used by the encoders now consists of a single
>> receive_packet() callback that pulls frames as required.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
> 
> Generally looks ok, only some minor comments below.
> 
>>  libavcodec/avcodec.h  |  12 +-
>>  libavcodec/decode.c   |   1 -
>>  libavcodec/encode.c   | 285 ++++++++++++++++++++++++++++++++----------
>>  libavcodec/encode.h   |  39 ++++++
>>  libavcodec/internal.h |   7 +-
>>  libavcodec/utils.c    |  10 +-
>>  6 files changed, 277 insertions(+), 77 deletions(-)
>>  create mode 100644 libavcodec/encode.h
>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
>> index 9ed2cf0f59..cdea1c6c1e 100644
>> --- a/libavcodec/encode.c
>> +++ b/libavcodec/encode.c
>> @@ -26,6 +26,7 @@
>>  #include "libavutil/samplefmt.h"
>>  
>>  #include "avcodec.h"
>> +#include "encode.h"
>>  #include "frame_thread_encoder.h"
>>  #include "internal.h"
>>  
>> @@ -80,12 +81,9 @@ int ff_alloc_packet(AVPacket *avpkt, int size)
>>   */
>>  static int pad_last_frame(AVCodecContext *s, AVFrame **dst, const AVFrame *src)
>>  {
>> -    AVFrame *frame = NULL;
>> +    AVFrame *frame = *dst;
>>      int ret;
>>  
>> -    if (!(frame = av_frame_alloc()))
>> -        return AVERROR(ENOMEM);
>> -
>>      frame->format         = src->format;
>>      frame->channel_layout = src->channel_layout;
>>      frame->channels       = src->channels;
>> @@ -106,12 +104,10 @@ static int pad_last_frame(AVCodecContext *s, AVFrame **dst, const AVFrame *src)
> 
> Doesn't need to be a double pointer anymore.

I know, but i left it as is to keep the changes to
avcodec_encode_audio2() to a minimum.

Although admittedly, since avcodec_encode_audio2() is being completely
rewritten in the following patch, it hardly matters. So changed locally.

> 
>> +static int encode_send_packet_internal(AVCodecContext *avctx, const AVFrame *src)
> 
> Should this be named send_frame?

Yes, changed locally.

Thanks.