[FFmpeg-devel] [PATCH 01/50] avcodec/packet: deprecate av_init_packet()

Andreas Rheinhardt andreas.rheinhardt at gmail.com
Mon Feb 8 17:16:41 EET 2021


James Almer:
> Once removed, sizeof(AVPacket) will stop being a part of the public ABI.
> 
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/avpacket.c  | 23 +++++++++++++++--------
>  libavcodec/packet.h    | 23 +++++++++++++++++++----
>  libavcodec/version.h   |  3 +++
>  libavformat/avformat.h |  4 ++++
>  4 files changed, 41 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index e4ba403cf6..ae0cbfb9f9 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -32,6 +32,7 @@
>  #include "packet.h"
>  #include "packet_internal.h"
>  
> +#if FF_API_INIT_PACKET
>  void av_init_packet(AVPacket *pkt)
>  {
>      pkt->pts                  = AV_NOPTS_VALUE;
> @@ -49,6 +50,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      pkt->side_data            = NULL;
>      pkt->side_data_elems      = 0;
>  }
> +#endif
> +
> +static void get_packet_defaults(AVPacket *pkt)
> +{
> +    memset(pkt, 0, sizeof(*pkt));
> +
> +    pkt->pts             = AV_NOPTS_VALUE;
> +    pkt->dts             = AV_NOPTS_VALUE;
> +    pkt->pos             = -1;
> +}
>  
>  AVPacket *av_packet_alloc(void)
>  {
> @@ -56,7 +67,7 @@ AVPacket *av_packet_alloc(void)
>      if (!pkt)
>          return pkt;
>  
> -    av_init_packet(pkt);
> +    get_packet_defaults(pkt);
>  
>      return pkt;
>  }
> @@ -92,7 +103,7 @@ int av_new_packet(AVPacket *pkt, int size)
>      if (ret < 0)
>          return ret;
>  
> -    av_init_packet(pkt);
> +    get_packet_defaults(pkt);
>      pkt->buf      = buf;
>      pkt->data     = buf->data;
>      pkt->size     = size;
> @@ -607,9 +618,7 @@ void av_packet_unref(AVPacket *pkt)
>  {
>      av_packet_free_side_data(pkt);
>      av_buffer_unref(&pkt->buf);
> -    av_init_packet(pkt);
> -    pkt->data = NULL;
> -    pkt->size = 0;
> +    get_packet_defaults(pkt);
>  }
>  
>  int av_packet_ref(AVPacket *dst, const AVPacket *src)
> @@ -664,9 +673,7 @@ AVPacket *av_packet_clone(const AVPacket *src)
>  void av_packet_move_ref(AVPacket *dst, AVPacket *src)
>  {
>      *dst = *src;
> -    av_init_packet(src);
> -    src->data = NULL;
> -    src->size = 0;
> +    get_packet_defaults(src);
>  }
>  
>  int av_packet_make_refcounted(AVPacket *pkt)
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index b9d4c9c2c8..c442b6a6eb 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -319,10 +319,6 @@ typedef struct AVPacketSideData {
>   * packets, with no compressed data, containing only side data
>   * (e.g. to update some stream parameters at the end of encoding).
>   *
> - * AVPacket is one of the few structs in FFmpeg, whose size is a part of public
> - * ABI. Thus it may be allocated on stack and no new fields can be added to it
> - * without libavcodec and libavformat major bump.
> - *
>   * The semantics of data ownership depends on the buf field.
>   * If it is set, the packet data is dynamically allocated and is
>   * valid indefinitely until a call to av_packet_unref() reduces the
> @@ -334,6 +330,12 @@ typedef struct AVPacketSideData {
>   * The side data is always allocated with av_malloc(), copied by
>   * av_packet_ref() and freed by av_packet_unref().
>   *
> + * sizeof(AVPacket) being a part of the public ABI is deprecated. once
> + * av_init_packet() is removed, new packets will only be able to be allocated
> + * with av_packet_alloc(), and new fields may be added to the end of the struct
> + * with a minor bump.
> + *
> + * @see av_packet_alloc
>   * @see av_packet_ref
>   * @see av_packet_unref
>   */
> @@ -394,7 +396,11 @@ typedef struct AVPacket {
>  } AVPacket;
>  
>  typedef struct AVPacketList {
> +#if FF_API_INIT_PACKET
>      AVPacket pkt;
> +#else
> +    AVPacket *pkt;
> +#endif
>      struct AVPacketList *next;
>  } AVPacketList;

As long as the packet-list functions use this structure, there will be
an unnecessary allocation for every packet list entry. This is
unnecessary even if one wants to make sizeof(AVPacket) not part of the
ABI any more: Just move the next pointer to the front.
Of course this will make this structure useless to someone who doesn't
have functions that deal with AVPacketList entries. But given that there
are no public functions dedicated to this task at all and given that
anyone can write a packet list like the above without problems, this
structure should be removed from the public API altogether.
(If the packet list helper functions were to be made public, the above
would need to be revised. But I don't think that adding a packet list
API that forces us to use a linked list should be made public.)

>  
> @@ -460,6 +466,7 @@ AVPacket *av_packet_clone(const AVPacket *src);
>   */
>  void av_packet_free(AVPacket **pkt);
>  
> +#if FF_API_INIT_PACKET
>  /**
>   * Initialize optional fields of a packet with default values.
>   *
> @@ -467,8 +474,16 @@ void av_packet_free(AVPacket **pkt);
>   * initialized separately.
>   *
>   * @param pkt packet
> + *
> + * @see av_packet_alloc
> + * @see av_packet_unref
> + *
> + * @deprecated This function is deprecated. Once it's removed,
> +               sizeof(AVPacket) will not be a part of the ABI anymore.
>   */
> +attribute_deprecated
>  void av_init_packet(AVPacket *pkt);
> +#endif
>  
>  /**
>   * Allocate the payload of a packet and initialize its fields with
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 3cac8c5f30..247568ec7f 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -153,5 +153,8 @@
>  #ifndef FF_API_DEBUG_MV
>  #define FF_API_DEBUG_MV          (LIBAVCODEC_VERSION_MAJOR < 60)
>  #endif
> +#ifndef FF_API_INIT_PACKET
> +#define FF_API_INIT_PACKET         (LIBAVCODEC_VERSION_MAJOR < 60)
> +#endif
>  
>  #endif /* AVCODEC_VERSION_H */
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 523cf34d55..d2d31e1deb 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -950,7 +950,11 @@ typedef struct AVStream {
>       * decoding: set by libavformat, must not be modified by the caller.
>       * encoding: unused
>       */
> +#if FF_API_INIT_PACKET
>      AVPacket attached_pic;
> +#else
> +    AVPacket *attached_pic;
> +#endif
>  
>      /**
>       * An array of side data that applies to the whole stream (i.e. the
> 



More information about the ffmpeg-devel mailing list