Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-02-12 16:02:21)
>> av_packet_ref() mostly treated the destination packet dst as uninitialized,
>> i.e. the destination fields were simply overwritten. But if the source
>> packet was not reference-counted, dst->buf was treated as if it pointed
>> to an already allocated buffer (if != NULL) to be reallocated to the
>> desired size.
>>
>> The documentation did not explicitly state whether the dst will be treated
>> as uninitialized, but it stated that if the source packet is not refcounted,
>> a new buffer in dst will be allocated. This and the fact that the side-data
>> as well as the codepath taken in case src is refcounted always treated the
>> packet as uninitialized means that dst should always be treated as
>> uninitialized for the sake of consistency. And this behaviour has been
>> explicitly documented.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com>
>> ---
>> libavcodec/avcodec.h | 2 +-
>> libavcodec/avpacket.c | 1 +
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index dc5807324f..982a545dc6 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -4600,7 +4600,7 @@ void av_packet_free_side_data(AVPacket *pkt);
>> *
>> * @see av_packet_unref
>> *
>> - * @param dst Destination packet
>> + * @param dst Destination packet. Will be treated as initially
>> uninitialized.
>
> The 'initially' sounds weird to me. How about "Will be completely
> overwritten"?
>
Changed.
>> * @param src Source packet
>> *
>> * @return 0 on success, a negative AVERROR on error.
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index 74845efcd2..0d9ddeee07 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -614,6 +614,7 @@ int av_packet_ref(AVPacket *dst, const AVPacket *src)
>> return ret;
>>
>> if (!src->buf) {
>> + dst->buf = NULL;
>> ret = packet_alloc(&dst->buf, src->size);
>> if (ret < 0)
>> goto fail;
>> --
>> 2.20.1
>
> weak sugestion - perhaps it'd be clearer to just add a call to
> av_init_packet() to the beginning of that function
It's unnecessary on success, but I have added it on failure in [1].
>
> Also, we might want to finally forbid non-refcounted packets completely.
>
Then you might want to take a look at [2].
- Andreas
[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-March/259083.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-December/253662.html
_______________________________________________
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".
