[FFmpeg-devel] [PATCH] avcodec/avpacket: ensure the packet is writable in av_shrink_packet()

[James Almer]

On 3/25/2018 9:14 AM, wm4 wrote:
> On Sat, 24 Mar 2018 22:39:45 -0300
> James Almer <jamrial at gmail.com> wrote:
> 
>> On 3/24/2018 6:46 PM, wm4 wrote:
>>> On Sat, 24 Mar 2018 18:11:53 -0300
>>> James Almer <jamrial at gmail.com> wrote:
>>>   
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>> This is a good time to deprecate this function and introduce a
>>>> replacement using the correct av_packet namespace and this time
>>>> returning an int.
>>>> What would be better
>>>>
>>>> int av_packet_shrink(AVPacket *pkt, int size);
>>>>
>>>> Or
>>>>  
>>>   
>>>> int av_packet_resize(AVPacket *pkt, int size);  
>>>
>>> Seems better.
>>>   
>>>>
>>>> The latter would be a combination of both the current shrink and grow
>>>> functions.
>>>>
>>>>  libavcodec/avpacket.c | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>>>> index 0693ca6f62..7faa082395 100644
>>>> --- a/libavcodec/avpacket.c
>>>> +++ b/libavcodec/avpacket.c
>>>> @@ -100,9 +100,12 @@ int av_new_packet(AVPacket *pkt, int size)
>>>>  
>>>>  void av_shrink_packet(AVPacket *pkt, int size)
>>>>  {
>>>> +    int packet_is_writable;
>>>>      if (pkt->size <= size)
>>>>          return;
>>>>      pkt->size = size;
>>>> +    packet_is_writable = !av_packet_make_writable(pkt);
>>>> +    av_assert0(packet_is_writable);
>>>>      memset(pkt->data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>>>  }
>>>>    
>>>
>>> LGTM  
>>
>> Ok, but i just realized that i can't apply this without first writing an
>> internal variant specifically for the encoders that makes sure pkt->data
>> != avctx->internal->byte_buffer before trying to do anything, otherwise
>> the supposed benefits of that weird internal buffer code in
>> ff_alloc_packet2() would be lost.
>>
>> Will look at that later.
> 
> Didn't look at it again, but this mechanism might be broken anyway
> since the new encode API must return refcounted packets.
> 
> Maybe the new dynamic sized buffer pool would help.

Good point. Still waiting for suggestions about how to introduce that in
the relevant thread, for that matter :p