[FFmpeg-devel] [PATCH 1/6] avcodec/avpacket: add av_packet_make_ref()

[James Almer]

On 3/25/2018 2:54 PM, Mark Thompson wrote:
> On 25/03/18 05:03, James Almer wrote:
>> It works as a drop in replacement for the deprecated av_dup_packet(),
>> to ensure a packet is reference counted.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> Better name welcome.
>>
>>  libavcodec/avcodec.h  | 18 +++++++++++++++++-
>>  libavcodec/avpacket.c | 18 ++++++++++++++++++
>>  2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 495242faf0..eb3fe4e428 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -4360,7 +4360,7 @@ int av_packet_from_data(AVPacket *pkt, uint8_t *data, int size);
>>   * @warning This is a hack - the packet memory allocation stuff is broken. The
>>   * packet is allocated if it was not really allocated.
>>   *
>> - * @deprecated Use av_packet_ref
>> + * @deprecated Use av_packet_ref or av_packet_make_ref
>>   */
>>  attribute_deprecated
>>  int av_dup_packet(AVPacket *pkt);
>> @@ -4531,6 +4531,22 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src);
>>   */
>>  int av_packet_copy_props(AVPacket *dst, const AVPacket *src);
>>  
>> +/**
>> + * Ensure the data described by a given packet is reference counted.
>> + *
>> + * @note This function does no ensure that the reference will be writable.
> 
> "does not ensure"

Fixed.

> 
>> + *       Use av_packet_make_writable instead for that purpose.
>> + *
>> + * @see av_packet_ref
>> + * @see av_packet_make_writable
>> + *
>> + * @param pkt packet whose data should be made reference counted.
>> + *
>> + * @return 0 on success, a negative AVERROR on error. On failure, the
>> + *         packet is unchanged.
>> + */
>> +int av_packet_make_ref(AVPacket *pkt);
> 
> "make_ref" seems pretty confusing as a name - it sounds like it will always make a new reference somehow.
> 
> Given the first line of the description, "ensure_refcounted"?

av_packet_ensure_refcounted() seems a tad too long, IMO, especially for
a function doing something as simple as this one as replacement for one
called av_dup_packet().

But if others agree I'll go with it.

> 
>> +
>>  /**
>>   * Create a writable reference for the data described by a given packet,
>>   * avoiding data copy if possible.
>> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
>> index 7faa082395..a6d2e6eb74 100644
>> --- a/libavcodec/avpacket.c
>> +++ b/libavcodec/avpacket.c
>> @@ -655,6 +655,24 @@ void av_packet_move_ref(AVPacket *dst, AVPacket *src)
>>      src->size = 0;
>>  }
>>  
>> +int av_packet_make_ref(AVPacket *pkt)
>> +{
>> +    int ret;
>> +
>> +    if (pkt->buf)
>> +        return 0;
>> +
>> +    ret = packet_alloc(&pkt->buf, pkt->size);
>> +    if (ret < 0)
>> +        return ret;
>> +    if (pkt->size)
>> +        memcpy(pkt->buf->data, pkt->data, pkt->size);
>> +
>> +    pkt->data = pkt->buf->data;
>> +
>> +    return 0;
>> +}
>> +
>>  int av_packet_make_writable(AVPacket *pkt)
>>  {
>>      AVBufferRef *buf = NULL;
>>
> 
> Looks like sensible thing to have.
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>