[FFmpeg-devel] [PATCH 2/3 v2] avcodec/encode: restructure the old encode API

[James Almer]

On 3/26/2020 5:57 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-03-16 22:30:01)
>> Following the same logic as 061a0c14bb, this commit turns the old encode API
>> into a wrapper for the new one.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> This could be squashed with the previous commit, like it was done in 061a0c14bb,
>> but i figured it would be easier to review this way.
>>
>>  libavcodec/encode.c   | 364 +++++++++++++-----------------------------
>>  libavcodec/internal.h |   1 +
>>  libavcodec/utils.c    |   8 +
>>  3 files changed, 116 insertions(+), 257 deletions(-)
>>
>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
>> index cdea1c6c1e..0fdb9e2df2 100644
>> --- a/libavcodec/encode.c
>> +++ b/libavcodec/encode.c
>> @@ -610,3 +361,102 @@ int attribute_align_arg avcodec_receive_packet(AVCodecContext *avctx, AVPacket *
>>  
>>      return 0;
>>  }
>> +
>> +static int compat_encode(AVCodecContext *avctx, AVPacket *avpkt,
>> +                         int *got_packet, const AVFrame *frame)
>> +{
>> +    AVCodecInternal *avci = avctx->internal;
>> +    AVPacket user_pkt;
>> +    int ret;
>> +
>> +    *got_packet = 0;
>> +
>> +    if (frame && avctx->codec->type == AVMEDIA_TYPE_VIDEO) {
>> +        if (frame->format == AV_PIX_FMT_NONE)
>> +            av_log(avctx, AV_LOG_WARNING, "AVFrame.format is not set\n");
>> +        if (frame->width == 0 || frame->height == 0)
>> +            av_log(avctx, AV_LOG_WARNING, "AVFrame.width or height is not set\n");
>> +    }
>> +
>> +    ret = avcodec_send_frame(avctx, frame);
>> +    if (ret == AVERROR_EOF)
>> +        ret = 0;
>> +    else if (ret == AVERROR(EAGAIN)) {
>> +        /* we fully drain all the output in each encode call, so this should not
>> +         * ever happen */
>> +        return AVERROR_BUG;
>> +    } else if (ret < 0)
>> +        return ret;
>> +
>> +    av_packet_move_ref(&user_pkt, avpkt);
>> +    while (ret >= 0) {
>> +        ret = avcodec_receive_packet(avctx, avpkt);
>> +        if (ret < 0) {
>> +            if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
>> +                ret = 0;
>> +            goto finish;
>> +        }
>> +
>> +        if (avpkt != avci->compat_encode_packet) {
>> +            if (avpkt->data && user_pkt.data) {
>> +                if (user_pkt.size >= avpkt->size) {
>> +                    memcpy(user_pkt.data, avpkt->data, avpkt->size);
>> +                    av_buffer_unref(&avpkt->buf);
>> +                    avpkt->buf  = user_pkt.buf;
>> +                    avpkt->data = user_pkt.data;
>> +                    av_init_packet(&user_pkt);
>> +                } else {
>> +                    av_log(avctx, AV_LOG_ERROR, "Provided packet is too small, needs to be %d\n", avpkt->size);
>> +                    ret = AVERROR(EINVAL);
>> +                    goto finish;
> 
> Shouldn't the packet be unreffed somewhere in this block?

Yeah. Fixed locally.

> 
>> +                }
>> +            }
>> +
>> +            *got_packet = 1;
>> +            avpkt = avci->compat_encode_packet;
>> +        } else {
>> +            if (!avci->compat_decode_warned) {
>> +                av_log(avctx, AV_LOG_WARNING, "The deprecated avcodec_encode_* "
>> +                       "API cannot return all the packets for this encoder. "
>> +                       "Some packets will be dropped. Update your code to the "
>> +                       "new encoding API to fix this.\n");
>> +                avci->compat_decode_warned = 1;
>> +            }
> 
> Same.

avpkt is avci->compat_encode_packet in this block. It will either be
unreffed next time avcodec_receive_packet() is called with it, or by
avcodec_close(). No need to unref it here explicitly, but i can do it if
you prefer that.

> 
> Beyond those two apparent leaks, looks good.

I'll send an updated set with the first two patches only. Now we just
need nvenc, amfenc and vaapi_enc ported, and then we can apply everything.