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

[Mark Thompson]

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"

> + *       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"?

> +
>  /**
>   * 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