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

[Andreas Rheinhardt]

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.

> +
> +    /**
> +     * AVBufferRef for free use by the API user. FFmpeg will never check the
> +     * contents of the buffer ref. FFmpeg calls av_buffer_unref() on it when
> +     * the packet is unreferenced. av_packet_copy_props() calls create a new
> +     * reference with av_buffer_ref() for the target packet's opaque_ref field.
> +     *
> +     * This is unrelated to the opaque field, although it serves a similar
> +     * purpose.
> +     */
> +    AVBufferRef *opaque_ref;
> +
> +    /**
> +     * Time base of the packet's timestamps.
> +     */
> +    AVRational time_base;
>  } AVPacket;
>  
>  #if FF_API_INIT_PACKET