[FFmpeg-devel] [PATCH 2/2] avcodec/ac3enc: Avoid needlessly copying encoded packets around

[Andreas Rheinhardt]

James Almer:
> On 3/29/2021 2:20 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 3/29/2021 2:12 PM, Andreas Rheinhardt wrote:
>>>> AC-3 and EAC-3 are codecs whose packet sizes are known in advance,
>>>> so one can use the min_size parameter of ff_alloc_packet2() to
>>>> allocate exactly this amount. This avoids a memcpy later in
>>>> av_packet_make_refcounted() in encode_simple_internal().
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>>> ---
>>>> Is there actually a reason not to use av_new_packet() (or
>>>> ff_get_encode_buffer()) directly?
>>>
>>> I'm waiting for the bump when avcodec_encode_*() are removed before
>>> making most encoders DR1, and thus use ff_get_encode_buffer(). And that
>>> includes this one.
>>>
>>> This patch LGTM, but it may not be worth applying if it's going to be
>>> changed again very soon.
>>>
>>
>> What are your plans for encoders that don't know the final size in
>> advance and overallocate?
> 
> Keep using av_packet_shrink() like now.
> 

But only a few encoders only use av_shrink_packet(); most just use
ff_alloc_packet2() with min_size == 0 (in which case the returned packet
isn't refcounted) and just set the packet's size. The
av_packet_make_refcounted() in encode_simple_internal() then allocates
the final buffer (and zeroes the padding).
So do you intend to return these oversized buffers to the user or do you
intend to use ff_get_encode_buffer() for allocating the packet's buffer
when it isn't refcounted?

>>
>>>>
>>>>    libavcodec/ac3enc.c | 3 ++-
>>>>    1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
>>>> index 4cfd0afe12..fc4d45651d 100644
>>>> --- a/libavcodec/ac3enc.c
>>>> +++ b/libavcodec/ac3enc.c
>>>> @@ -1759,7 +1759,8 @@ int
>>>> ff_ac3_encode_frame_common_end(AVCodecContext *avctx, AVPacket *avpkt,
>>>>          ac3_quantize_mantissas(s);
>>>>    -    if ((ret = ff_alloc_packet2(avctx, avpkt, s->frame_size, 0))
>>>> < 0)
>>>> +    ret = ff_alloc_packet2(avctx, avpkt, s->frame_size,
>>>> s->frame_size);
>>>> +    if (ret < 0)
>>>>            return ret;
>>>>        ac3_output_frame(s, avpkt->data);
>>>>   
>>>
>>> _______________________________________________
>>> 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".
>>
>> _______________________________________________
>> 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".
>>
> 
> _______________________________________________
> 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".