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

[James Almer]

On 3/29/2021 2:35 PM, Andreas Rheinhardt wrote:
> 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?

If we want to ensure that buffers allocated with ff_get_encode_buffer() 
will be the exact required size on encoders that currently overallocate 
with ff_alloc_packet2() and min_size == 0, then yes, we can keep calling 
ff_alloc_packet2() to get the non refcounted packet with pkt->data 
pointing to the internal buffer, then effectively allocate the final 
buffer with ff_get_encode_buffer() once we know the actual packet size, 
and memcpy the data.
But we could consider to instead propagate those oversized packets and 
save ourselves the memcpy, unless the overallocation is a really bad 
worst case scenario, like in cfhdenc.

> 
>>>
>>>>>
>>>>>     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".
> 
> _______________________________________________
> 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".
>