[FFmpeg-devel] [PATCH] avpacket: ABI bump additions

[Lynne]

3 Jun 2021, 06:58 by dev at lynne.ee:

> Apr 26, 2021, 03:27 by andreas.rheinhardt at outlook.com:
>
>> Lynne:
>>
>>> From 097aed2ac33dda0bb2052d8b0402711ce95079ba Mon Sep 17 00:00:00 2001
>>> From: Lynne <dev at lynne.ee>
>>> Date: Sat, 23 Jan 2021 19:56:18 +0100
>>> Subject: [PATCH] avpacket: ABI bump additions
>>>
>>> ---
>>>  libavcodec/avpacket.c |  5 +++++
>>>  libavcodec/packet.h   | 21 +++++++++++++++++++++
>>>  2 files changed, 26 insertions(+)
>>>
>>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>>> index e32c467586..03b73b3b53 100644
>>> --- a/libavcodec/avpacket.c
>>> +++ b/libavcodec/avpacket.c
>>> @@ -382,6 +382,10 @@ int av_packet_copy_props(AVPacket *dst, const AVPacket *src)
>>>  dst->flags                = src->flags;
>>>  dst->stream_index         = src->stream_index;
>>>  
>>> +    i = av_buffer_replace(&dst->opaque_ref, src->opaque_ref);
>>> +    if (i < 0)
>>> +        return i;
>>>
>>
>> 1. Don't use i here; add a new variable.
>> 2. Up until now, av_packet_ref() and av_packet_copy_props() treat the
>> destination packet as uninitialized and make no attempt at unreferencing
>> its content; yet you try to reuse opaque_ref. Even worse, you might
>> return potentially dangerous packets: If the properties were
>> uninitialized and av_packet_copy_props() failed, then the caller were
>> not allowed to unreference the packet even when the non-properties were
>> set to sane values. The easiest way to fix this is to move setting
>> opaque ref to the place after initializing side_data below and either
>> set dst->opaque_ref to NULL before av_buffer_replace() or to not use
>> av_buffer_replace(). It may also be best to unref it again if copying
>> side data fails.
>>
>>> +
>>>  dst->side_data            = NULL;
>>>  dst->side_data_elems      = 0;
>>>  for (i = 0; i < src->side_data_elems; i++) {
>>> @@ -403,6 +407,7 @@ int av_packet_copy_props(AVPacket *dst, const AVPacket *src)
>>>  void av_packet_unref(AVPacket *pkt)
>>>  {
>>>  av_packet_free_side_data(pkt);
>>> +    av_buffer_unref(&pkt->opaque_ref);
>>>  av_buffer_unref(&pkt->buf);
>>>  get_packet_defaults(pkt);
>>>  }
>>> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
>>> index fad8341c12..c29ad18a2b 100644
>>> --- a/libavcodec/packet.h
>>> +++ b/libavcodec/packet.h
>>> @@ -383,6 +383,27 @@ typedef struct AVPacket {
>>>  int64_t duration;
>>>  
>>>  int64_t pos;                            ///< byte position in stream, -1 if unknown
>>> +
>>> +    /**
>>> +     * for some private data of the user
>>> +     */
>>> +    void *opaque;
>>>
>>
>> The corresponding AVFrame field is copied when copying props.
>>
>
> Fixed both, thanks. Also copied the time_base field and made av_init_packet()
> initialize all fields.
> Patch attached. Sorry it's taking me so long to work on this and the Vulkan ABI changes.
>

Since no one yet has objected, will push this in 3 days unless someone does.