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.
I'd argue that a deprecation is actually a change.
> 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.
>
Yet you are not documenting that av_packet_resize can't fail if it is
shrinking a packet known to be writable; ergo all unchecked uses of this
function in the second and third patch are API abuse.
>>
>> Regards,
>> Marton
>>
>>>
>>> Signed-off-by: James Almer <jamr...@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@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
