[FFmpeg-devel] [PATCH 17/39] avcodec/libxavs: Avoid overallocating, copying packet data

[Andreas Rheinhardt]

James Almer:
> On 5/21/2021 6:17 AM, Andreas Rheinhardt wrote:
>> When the packet size is known in advance like here, one can avoid
>> an intermediate buffer for the packet data; also, there is no reason
>> to add AV_INPUT_BUFFER_MIN_SIZE to the packet size any more, as the
>> actually needed packet size can be easily calculated: It is three bytes
>> more than the raw nal size per NALU.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>> ---
>>   libavcodec/libxavs.c | 20 +++++++++++++-------
>>   1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/libavcodec/libxavs.c b/libavcodec/libxavs.c
>> index 253f4bde6a..ea53c49d38 100644
>> --- a/libavcodec/libxavs.c
>> +++ b/libavcodec/libxavs.c
>> @@ -27,6 +27,7 @@
>>   #include <float.h>
>>   #include <xavs.h>
>>   #include "avcodec.h"
>> +#include "encode.h"
>>   #include "internal.h"
>>   #include "packet_internal.h"
>>   #include "libavutil/internal.h"
>> @@ -85,18 +86,20 @@ static int encode_nals(AVCodecContext *ctx,
>> AVPacket *pkt,
>>                          xavs_nal_t *nals, int nnal)
>>   {
>>       XavsContext *x4 = ctx->priv_data;
>> -    uint8_t *p;
>> -    int i, s, ret, size = x4->sei_size + AV_INPUT_BUFFER_MIN_SIZE;
>> +    int64_t size = x4->sei_size;
>> +    uint8_t *p, *p_end;
>> +    int i, s, ret;
>>         if (!nnal)
>>           return 0;
>>         for (i = 0; i < nnal; i++)
>> -        size += nals[i].i_payload;
>> +        size += 3U + nals[i].i_payload;
>>   -    if ((ret = ff_alloc_packet2(ctx, pkt, size, 0)) < 0)
>> +    if ((ret = ff_get_encode_buffer(ctx, pkt, size, 0)) < 0)
>>           return ret;
>>       p = pkt->data;
>> +    p_end = pkt->data + size;
>>         /* Write the SEI as part of the first frame. */
>>       if (x4->sei_size > 0 && nnal > 0) {
>> @@ -106,12 +109,14 @@ static int encode_nals(AVCodecContext *ctx,
>> AVPacket *pkt,
>>       }
>>         for (i = 0; i < nnal; i++) {
>> +        int size = p_end - p;
>>           s = xavs_nal_encode(p, &size, 1, nals + i);
>>           if (s < 0)
>>               return -1;
>> +        if (s != 3U + nals[i].i_payload)
>> +            return AVERROR_BUG;
> 
> AVERROR_EXTERNAL. This is not a bug in our code or something we can fix
> if it fails, so AVERROR_BUG is not correct (Neither would an assert for
> the same reason).
> 

xavs's documentation is quite sparse, to say the least. It only says
this (see https://github.com/pandastream/xavs/blob/master/xavs.h#L432):

/* xavs_nal_encode:
 *      encode a nal into a buffer, setting the size.
 *      if b_annexeb then a long synch work is added
 *      XXX: it currently doesn't check for overflow */
int xavs_nal_encode (void *, int *, int b_annexeb, xavs_nal_t * nal);

So I chose AVERROR_BUG, because if this fails, then it might be due to
me misunderstanding the xavs API. Obviously I don't believe that this
will happen: It seems that development of xavs stopped years ago.

>>           p += s;
>>       }
>> -    pkt->size = p - pkt->data;
>>         return 1;
>>   }
>> @@ -150,7 +155,7 @@ static int XAVS_frame(AVCodecContext *avctx,
>> AVPacket *pkt,
>>         if (!ret) {
>>           if (!frame && !(x4->end_of_stream)) {
>> -            if ((ret = ff_alloc_packet2(avctx, pkt, 4, 0)) < 0)
>> +            if ((ret = ff_get_encode_buffer(avctx, pkt, 4, 0)) < 0)
>>                   return ret;
>>                 pkt->data[0] = 0x0;
>> @@ -425,7 +430,8 @@ const AVCodec ff_libxavs_encoder = {
>>       .init           = XAVS_init,
>>       .encode2        = XAVS_frame,
>>       .close          = XAVS_close,
>> -    .capabilities   = AV_CODEC_CAP_DELAY | AV_CODEC_CAP_OTHER_THREADS,
>> +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY |
>> +                      AV_CODEC_CAP_OTHER_THREADS,
>>       .caps_internal  = FF_CODEC_CAP_AUTO_THREADS,
>>       .pix_fmts       = (const enum AVPixelFormat[]) {
>> AV_PIX_FMT_YUV420P, AV_PIX_FMT_NONE },
>>       .priv_class     = &xavs_class,
>>
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".