2 Aug 2021, 05:03 by andreas.rheinha...@outlook.com:
> Lynne:
>
>> 8 Jul 2021, 21:20 by andreas.rheinha...@outlook.com:
>>
>>> Lynne:
>>>
>>>> Apr 26, 2021, 03:27 by andreas.rheinha...@outlook.com:
>>>>
>>>>> Lynne:
>>>>>
>>>>>> From 097aed2ac33dda0bb2052d8b0402711ce95079ba Mon Sep 17 00:00:00 2001
>>>>>> From: Lynne <d...@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.
>>>>
>>>
>>> Your new version is still not correct: If copying side data fails, the
>>> function returns without initializing opaque_ref at all, thereby making
>>> it dangerous to unref the packet (which e.g. av_packet_ref() does).
>>>
>>> - Andreas
>>>
>>
>> Fixed, and made sure to unref it if copying side data fails.
>>
> You indeed addressed the concerns I had; I still don't like the idea of
> having per-packet timebases (instead preferring Marton's approach), but
> it seems that we two are alone in this. So go ahead.
>
> - Andreas
>
Thanks for reviewing it.
Pushed!
_______________________________________________
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".
