[FFmpeg-devel] [PATCH 1/3] avcodec/avpacket: add av_packet_resize()

[James Almer]

On 3/11/2021 1:11 PM, Marton Balint wrote:
> 
> 
> On Thu, 11 Mar 2021, James Almer wrote:
> 
>> This function acts as a replacement for both av_grow_packet() and
>> av_shrink_packet(), the latter which is now deprecated and will be 
>> removed as
>> it does not correctly handle non-writable packets.
> 
> I don't think this is a good idea, av_shrink_packet cannot fail, 
> av_grow_packet can. By using the same function you are losing the 
> information if the end result should be checked or not.

I'm not sure i follow. av_shrink_packet() is not being changed at all, 
just deprecated, scheduled for removal, and its use discouraged.
Maybe i should have split this in two, one to add av_packet_resize() and 
one to deprecate av_shrink_packet(), to avoid confusion.

In any case, the fact av_shrink_packet() cannot fail is the reason I'm 
getting rid of it. It's zeroing the padding without bothering to check 
if the packet is writable at all. And we can't have it attempt to make 
it writable because it can't then report if it failed to reallocate the 
buffer. So this patch here deprecates it for being a function that 
predates reference counted buffers and is not able to properly handle 
them, and adds a replacement for it that also supersedes 
av_grow_packet() while at it.

> 
> Regards,
> Marton
> 
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> libavcodec/avpacket.c | 19 +++++++++++++++----
>> libavcodec/packet.h   | 16 ++++++++++++++++
>> libavcodec/version.h  |  3 +++
>> 3 files changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index 32cb71fcf0..7d0dbadbed 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -100,6 +100,7 @@ int av_new_packet(AVPacket *pkt, int size)
>>     return 0;
>> }
>>
>> +#if FF_API_SHRINK_PACKET
>> void av_shrink_packet(AVPacket *pkt, int size)
>> {
>>     if (pkt->size <= size)
>> @@ -107,16 +108,26 @@ void av_shrink_packet(AVPacket *pkt, int size)
>>     pkt->size = size;
>>     memset(pkt->data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>> }
>> +#endif
>>
>> int av_grow_packet(AVPacket *pkt, int grow_by)
>> {
>> -    int new_size;
>>     av_assert0((unsigned)pkt->size <= INT_MAX - 
>> AV_INPUT_BUFFER_PADDING_SIZE);
>>     if ((unsigned)grow_by >
>>         INT_MAX - (pkt->size + AV_INPUT_BUFFER_PADDING_SIZE))
>>         return AVERROR(ENOMEM);
>>
>> -    new_size = pkt->size + grow_by + AV_INPUT_BUFFER_PADDING_SIZE;
>> +    return av_packet_resize(pkt, pkt->size + grow_by);
>> +}
>> +
>> +int av_packet_resize(AVPacket *pkt, int size)
>> +{
>> +    int new_size;
>> +
>> +    if (size < 0 || size > INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE)
>> +        return AVERROR(EINVAL);
>> +
>> +    new_size = size + AV_INPUT_BUFFER_PADDING_SIZE;
>>     if (pkt->buf) {
>>         size_t data_offset;
>>         uint8_t *old_data = pkt->data;
>> @@ -143,10 +154,10 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
>>         if (!pkt->buf)
>>             return AVERROR(ENOMEM);
>>         if (pkt->size > 0)
>> -            memcpy(pkt->buf->data, pkt->data, pkt->size);
>> +            memcpy(pkt->buf->data, pkt->data, FFMIN(pkt->size, size));
>>         pkt->data = pkt->buf->data;
>>     }
>> -    pkt->size += grow_by;
>> +    pkt->size = size;
>>     memset(pkt->data + pkt->size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
>>
>>     return 0;
>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
>> index 3d9013d783..1720d973f5 100644
>> --- a/libavcodec/packet.h
>> +++ b/libavcodec/packet.h
>> @@ -484,13 +484,29 @@ void av_init_packet(AVPacket *pkt);
>>  */
>> int av_new_packet(AVPacket *pkt, int size);
>>
>> +#if FF_API_SHRINK_PACKET
>> /**
>>  * Reduce packet size, correctly zeroing padding
>>  *
>>  * @param pkt packet
>>  * @param size new size
>> + *
>> + * @deprecated Use av_packet_resize
>>  */
>> +attribute_deprecated
>> void av_shrink_packet(AVPacket *pkt, int size);
>> +#endif
>> +
>> +/**
>> + * Resize the payload of a packet, correctly zeroing padding and 
>> avoiding data
>> + * copy if possible.
>> + *
>> + * @param pkt packet
>> + * @param size new size
>> + *
>> + * @return 0 on success, a negative AVERROR on error
>> + */
>> +int av_packet_resize(AVPacket *pkt, int size);
>>
>> /**
>>  * Increase packet size, correctly zeroing padding
>> diff --git a/libavcodec/version.h b/libavcodec/version.h
>> index 3124ec8061..6c362b43e2 100644
>> --- a/libavcodec/version.h
>> +++ b/libavcodec/version.h
>> @@ -162,5 +162,8 @@
>> #ifndef FF_API_GET_FRAME_CLASS
>> #define FF_API_GET_FRAME_CLASS     (LIBAVCODEC_VERSION_MAJOR < 60)
>> #endif
>> +#ifndef FF_API_SHRINK_PACKET
>> +#define FF_API_SHRINK_PACKET       (LIBAVCODEC_VERSION_MAJOR < 60)
>> +#endif
>>
>> #endif /* AVCODEC_VERSION_H */
>> -- 
>> 2.30.2
>>
>> _______________________________________________
>> 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".