[FFmpeg-devel] lavu/attributes: add ff_ prefix to attribute_deprecated

Otherwise it might conflict with caller-defined identifiers.

ff because it is not supposed to be used by external callers.
---
 doc/developer.texi                     |  2 +-
 doc/print_options.c                    |  4 ++--
 libavcodec/avcodec.h                   |  4 ++--
 libavcodec/avfft.h                     | 30 +++++++++++++-------------
 libavcodec/packet.h                    |  4 ++--
 libavcodec/vdpau.h                     |  8 +++----
 libavfilter/avfilter.h                 |  4 ++--
 libavformat/avformat.h                 | 12 +++++------
 libavutil/attributes.h                 | 12 ++++++++---
 libavutil/frame.h                      | 12 +++++------
 libavutil/hdr_dynamic_vivid_metadata.h | 12 +++++------
 libavutil/version.h                    |  1 +
 12 files changed, 56 insertions(+), 49 deletions(-)

Anton Khirnov:
> Otherwise it might conflict with caller-defined identifiers.
> 
> ff because it is not supposed to be used by external callers.
> ---

This is public, so it should be av_attribute_deprecated.
For the same reason it is actually an API break to no longer provide the
un-prefixed version.
Is this just a general precaution or did you run into an actual conflict?

>  doc/developer.texi                     |  2 +-
>  doc/print_options.c                    |  4 ++--
>  libavcodec/avcodec.h                   |  4 ++--
>  libavcodec/avfft.h                     | 30 +++++++++++++-------------
>  libavcodec/packet.h                    |  4 ++--
>  libavcodec/vdpau.h                     |  8 +++----
>  libavfilter/avfilter.h                 |  4 ++--
>  libavformat/avformat.h                 | 12 +++++------
>  libavutil/attributes.h                 | 12 ++++++++---
>  libavutil/frame.h                      | 12 +++++------
>  libavutil/hdr_dynamic_vivid_metadata.h | 12 +++++------
>  libavutil/version.h                    |  1 +
>  12 files changed, 56 insertions(+), 49 deletions(-)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index c86bb5820c..141139df8c 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -522,7 +522,7 @@ removing the API - this is described in @ref{Major version bumps}.
>  
>  To deprecate an API you should signal to our users that they should stop using
>  it. E.g. if you intend to remove struct members or functions, you should mark
> -them with @code{attribute_deprecated}. When this cannot be done, it may be
> +them with @code{ff_attribute_deprecated}. When this cannot be done, it may be
>  possible to detect the use of the deprecated API at runtime and print a warning
>  (though take care not to print it too often). You should also document the
>  deprecation (and the replacement, if applicable) in the relevant Doxygen
> diff --git a/doc/print_options.c b/doc/print_options.c
> index fd1cf10dc1..89df2928b8 100644
> --- a/doc/print_options.c
> +++ b/doc/print_options.c
> @@ -35,8 +35,8 @@
>  #include "libavutil/opt.h"
>  
>  /* Forcibly turn off deprecation warnings, which just add noise here. */
> -#undef attribute_deprecated
> -#define attribute_deprecated
> +#undef ff_attribute_deprecated
> +#define ff_attribute_deprecated
>  
>  #include "libavcodec/options_table.h"
>  
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 21fc74707f..0c1c4eec41 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -572,7 +572,7 @@ typedef struct AVCodecContext {
>       * - encoding: Set AVCodecContext.framerate instead
>       *
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int ticks_per_frame;
>  #endif
>  
> @@ -2370,7 +2370,7 @@ int avcodec_open2(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **op
>   * multiple times is not supported anymore -- use multiple codec contexts
>   * instead.
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  int avcodec_close(AVCodecContext *avctx);
>  #endif
>  
> diff --git a/libavcodec/avfft.h b/libavcodec/avfft.h
> index e3a0da1eb9..1cf1cd6df1 100644
> --- a/libavcodec/avfft.h
> +++ b/libavcodec/avfft.h
> @@ -50,14 +50,14 @@ typedef struct FFTContext FFTContext;
>   * @param inverse         if 0 perform the forward transform, if 1 perform the inverse
>   * @deprecated use av_tx_init from libavutil/tx.h with a type of AV_TX_FLOAT_FFT
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  FFTContext *av_fft_init(int nbits, int inverse);
>  
>  /**
>   * Do the permutation needed BEFORE calling ff_fft_calc().
>   * @deprecated without replacement
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_fft_permute(FFTContext *s, FFTComplex *z);
>  
>  /**
> @@ -65,25 +65,25 @@ void av_fft_permute(FFTContext *s, FFTComplex *z);
>   * input data must be permuted before. No 1.0/sqrt(n) normalization is done.
>   * @deprecated use the av_tx_fn value returned by av_tx_init, which also does permutation
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_fft_calc(FFTContext *s, FFTComplex *z);
>  
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_fft_end(FFTContext *s);
>  
>  /**
>   * @deprecated use av_tx_init from libavutil/tx.h with a type of AV_TX_FLOAT_MDCT,
>   * with a flag of AV_TX_FULL_IMDCT for a replacement to av_imdct_calc.
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  FFTContext *av_mdct_init(int nbits, int inverse, double scale);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_imdct_calc(FFTContext *s, FFTSample *output, const FFTSample *input);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_imdct_half(FFTContext *s, FFTSample *output, const FFTSample *input);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_mdct_calc(FFTContext *s, FFTSample *output, const FFTSample *input);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_mdct_end(FFTContext *s);
>  
>  /* Real Discrete Fourier Transform */
> @@ -104,11 +104,11 @@ typedef struct RDFTContext RDFTContext;
>   *
>   * @deprecated use av_tx_init from libavutil/tx.h with a type of AV_TX_FLOAT_RDFT
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  RDFTContext *av_rdft_init(int nbits, enum RDFTransformType trans);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_rdft_calc(RDFTContext *s, FFTSample *data);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_rdft_end(RDFTContext *s);
>  
>  /* Discrete Cosine Transform */
> @@ -134,11 +134,11 @@ enum DCTTransformType {
>   *
>   * @deprecated use av_tx_init from libavutil/tx.h with an appropriate type of AV_TX_FLOAT_DCT
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  DCTContext *av_dct_init(int nbits, enum DCTTransformType type);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_dct_calc(DCTContext *s, FFTSample *data);
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_dct_end (DCTContext *s);
>  
>  /**
> diff --git a/libavcodec/packet.h b/libavcodec/packet.h
> index b0ba3baea2..969622456d 100644
> --- a/libavcodec/packet.h
> +++ b/libavcodec/packet.h
> @@ -567,7 +567,7 @@ typedef struct AVPacket {
>  } AVPacket;
>  
>  #if FF_API_INIT_PACKET
> -attribute_deprecated
> +ff_attribute_deprecated
>  typedef struct AVPacketList {
>      AVPacket pkt;
>      struct AVPacketList *next;
> @@ -649,7 +649,7 @@ void av_packet_free(AVPacket **pkt);
>   * @deprecated This function is deprecated. Once it's removed,
>                 sizeof(AVPacket) will not be a part of the ABI anymore.
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_init_packet(AVPacket *pkt);
>  #endif
>  
> diff --git a/libavcodec/vdpau.h b/libavcodec/vdpau.h
> index 8021c25761..9b68b5a123 100644
> --- a/libavcodec/vdpau.h
> +++ b/libavcodec/vdpau.h
> @@ -100,18 +100,18 @@ typedef struct AVVDPAUContext {
>   * Allows extending the struct without breaking API/ABI
>   * @deprecated use av_vdpau_bind_context() instead
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  AVVDPAUContext *av_alloc_vdpaucontext(void);
>  
>  /**
>   * @deprecated render2 is public and can be accessed directly
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  AVVDPAU_Render2 av_vdpau_hwaccel_get_render2(const AVVDPAUContext *);
>  /**
>   * @deprecated render2 is public and can be accessed directly
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  void av_vdpau_hwaccel_set_render2(AVVDPAUContext *, AVVDPAU_Render2);
>  #endif
>  
> @@ -162,7 +162,7 @@ int av_vdpau_get_surface_parameters(AVCodecContext *avctx, VdpChromaType *type,
>   * @return Newly-allocated AVVDPAUContext or NULL on failure.
>   * @deprecated use av_vdpau_bind_context() instead
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  AVVDPAUContext *av_vdpau_alloc_context(void);
>  #endif
>  
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index a34e61f23c..593abf647f 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -680,13 +680,13 @@ int avfilter_link(AVFilterContext *src, unsigned srcpad,
>  /**
>   * @deprecated this function should never be called by users
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  void avfilter_link_free(AVFilterLink **link);
>  
>  /**
>   * @deprecated this function should never be called by users
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  int avfilter_config_links(AVFilterContext *filter);
>  #endif
>  
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index de40397676..7c847842d0 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -862,7 +862,7 @@ typedef struct AVStream {
>       * @deprecated use AVStream's @ref AVCodecParameters.coded_side_data
>       *             "codecpar side data".
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      AVPacketSideData *side_data;
>      /**
>       * The number of elements in the AVStream.side_data array.
> @@ -870,7 +870,7 @@ typedef struct AVStream {
>       * @deprecated use AVStream's @ref AVCodecParameters.nb_coded_side_data
>       *             "codecpar side data".
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int            nb_side_data;
>  #endif
>  
> @@ -1891,7 +1891,7 @@ void av_format_inject_global_side_data(AVFormatContext *s);
>   * @return AVFMT_DURATION_FROM_PTS, AVFMT_DURATION_FROM_STREAM, or AVFMT_DURATION_FROM_BITRATE.
>   * @deprecated duration_estimation_method is public and can be read directly.
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  enum AVDurationEstimationMethod av_fmt_ctx_get_duration_estimation_method(const AVFormatContext* ctx);
>  #endif
>  
> @@ -2088,7 +2088,7 @@ int avformat_stream_group_add_stream(AVStreamGroup *stg, AVStream *st);
>   * @deprecated use av_packet_side_data_add() with the stream's
>   *             @ref AVCodecParameters.coded_side_data "codecpar side data"
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
>                              uint8_t *data, size_t size);
>  
> @@ -2103,7 +2103,7 @@ int av_stream_add_side_data(AVStream *st, enum AVPacketSideDataType type,
>   * @deprecated use av_packet_side_data_new() with the stream's
>   *             @ref AVCodecParameters.coded_side_data "codecpar side data"
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  uint8_t *av_stream_new_side_data(AVStream *stream,
>                                   enum AVPacketSideDataType type, size_t size);
>  /**
> @@ -2118,7 +2118,7 @@ uint8_t *av_stream_new_side_data(AVStream *stream,
>   * @deprecated use av_packet_side_data_get() with the stream's
>   *             @ref AVCodecParameters.coded_side_data "codecpar side data"
>   */
> -attribute_deprecated
> +ff_attribute_deprecated
>  uint8_t *av_stream_get_side_data(const AVStream *stream,
>                                   enum AVPacketSideDataType type, size_t *size);
>  #endif
> diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> index 04c615c952..3723683a31 100644
> --- a/libavutil/attributes.h
> +++ b/libavutil/attributes.h
> @@ -26,6 +26,8 @@
>  #ifndef AVUTIL_ATTRIBUTES_H
>  #define AVUTIL_ATTRIBUTES_H
>  
> +#include "version.h"
> +
>  #ifdef __GNUC__
>  #    define AV_GCC_VERSION_AT_LEAST(x,y) (__GNUC__ > (x) || __GNUC__ == (x) && __GNUC_MINOR__ >= (y))
>  #    define AV_GCC_VERSION_AT_MOST(x,y)  (__GNUC__ < (x) || __GNUC__ == (x) && __GNUC_MINOR__ <= (y))
> @@ -97,11 +99,15 @@
>  #endif
>  
>  #if AV_GCC_VERSION_AT_LEAST(3,1)
> -#    define attribute_deprecated __attribute__((deprecated))
> +#    define ff_attribute_deprecated __attribute__((deprecated))
>  #elif defined(_MSC_VER)
> -#    define attribute_deprecated __declspec(deprecated)
> +#    define ff_attribute_deprecated __declspec(deprecated)
>  #else
> -#    define attribute_deprecated
> +#    define ff_attribute_deprecated
> +#endif
> +
> +#if FF_API_ATTRIBUTE_DEPRECATED
> +#define attribute_deprecated ff_attribute_deprecated
>  #endif
>  
>  /**
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index b94687941d..f7ffbaa57f 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -432,7 +432,7 @@ typedef struct AVFrame {
>       *
>       * @deprecated Use AV_FRAME_FLAG_KEY instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int key_frame;
>  #endif
>  
> @@ -513,7 +513,7 @@ typedef struct AVFrame {
>       *
>       * @deprecated Use AV_FRAME_FLAG_INTERLACED instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int interlaced_frame;
>  
>      /**
> @@ -521,7 +521,7 @@ typedef struct AVFrame {
>       *
>       * @deprecated Use AV_FRAME_FLAG_TOP_FIELD_FIRST instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int top_field_first;
>  #endif
>  
> @@ -529,7 +529,7 @@ typedef struct AVFrame {
>      /**
>       * Tell user application that palette has changed from previous frame.
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int palette_has_changed;
>  #endif
>  
> @@ -646,7 +646,7 @@ typedef struct AVFrame {
>       * @deprecated use AV_CODEC_FLAG_COPY_OPAQUE to pass through arbitrary user
>       *             data from packets to frames
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int64_t pkt_pos;
>  #endif
>  
> @@ -680,7 +680,7 @@ typedef struct AVFrame {
>       * @deprecated use AV_CODEC_FLAG_COPY_OPAQUE to pass through arbitrary user
>       *             data from packets to frames
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int pkt_size;
>  #endif
>  
> diff --git a/libavutil/hdr_dynamic_vivid_metadata.h b/libavutil/hdr_dynamic_vivid_metadata.h
> index 4524a81557..8842b203a7 100644
> --- a/libavutil/hdr_dynamic_vivid_metadata.h
> +++ b/libavutil/hdr_dynamic_vivid_metadata.h
> @@ -174,7 +174,7 @@ typedef struct AVHDRVividColorToneMappingParams {
>       * of 0 to 3, inclusive.
>       * @deprecated Use three_spline instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      int three_Spline_TH_mode;
>  
>      /**
> @@ -182,7 +182,7 @@ typedef struct AVHDRVividColorToneMappingParams {
>       * and in multiples of 1.0/255.
>       * @deprecated Use three_spline instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      AVRational three_Spline_TH_enable_MB;
>  
>      /**
> @@ -191,7 +191,7 @@ typedef struct AVHDRVividColorToneMappingParams {
>       * and in multiples of 1.0/4095.
>       * @deprecated Use three_spline instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      AVRational three_Spline_TH_enable;
>  
>      /**
> @@ -200,7 +200,7 @@ typedef struct AVHDRVividColorToneMappingParams {
>       * and in multiples of 0.25/1023.
>       * @deprecated Use three_spline instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      AVRational three_Spline_TH_Delta1;
>  
>      /**
> @@ -209,7 +209,7 @@ typedef struct AVHDRVividColorToneMappingParams {
>       * and in multiples of 0.25/1023.
>       * @deprecated Use three_spline instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      AVRational three_Spline_TH_Delta2;
>  
>      /**
> @@ -218,7 +218,7 @@ typedef struct AVHDRVividColorToneMappingParams {
>       * and in multiples of 1.0/255.
>       * @deprecated Use three_spline instead
>       */
> -    attribute_deprecated
> +    ff_attribute_deprecated
>      AVRational three_Spline_enable_Strength;
>  #endif
>  
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 09f8cdc292..757ae6c082 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -111,6 +111,7 @@
>  #define FF_API_FRAME_KEY                (LIBAVUTIL_VERSION_MAJOR < 60)
>  #define FF_API_PALETTE_HAS_CHANGED      (LIBAVUTIL_VERSION_MAJOR < 60)
>  #define FF_API_VULKAN_CONTIGUOUS_MEMORY (LIBAVUTIL_VERSION_MAJOR < 60)
> +#define FF_API_ATTRIBUTE_DEPRECATED     (LIBAVUTIL_VERSION_MAJOR < 60)
>  
>  /**
>   * @}

Quoting Andreas Rheinhardt (2024-03-12 09:56:46)
> Anton Khirnov:
> > Otherwise it might conflict with caller-defined identifiers.
> > 
> > ff because it is not supposed to be used by external callers.
> > ---
> 
> This is public, so it should be av_attribute_deprecated.

As the commit message says, this is not supposed to be used by external
callers. Or do you see any reason why it should be public?

> For the same reason it is actually an API break to no longer provide the
> un-prefixed version.

I am still providing the unprefixed version under a deprecation guard.

> Is this just a general precaution or did you run into an actual conflict?

The former.

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-12 09:56:46)
>> Anton Khirnov:
>>> Otherwise it might conflict with caller-defined identifiers.
>>>
>>> ff because it is not supposed to be used by external callers.
>>> ---
>>
>> This is public, so it should be av_attribute_deprecated.
> 
> As the commit message says, this is not supposed to be used by external
> callers. Or do you see any reason why it should be public?
> 

It is public given that it is declared in a public header. That is what
matters.
Furthermore, we have used the FF_ and AV_ prefixes for public macros and
the av_ prefix for public symbols, but IIRC we do not use the ff_ prefix
for public symbols at all, therefore I'd like us not to reserve another
namespace.

>> For the same reason it is actually an API break to no longer provide the
>> un-prefixed version.
> 
> I am still providing the unprefixed version under a deprecation guard.
> 

I overlooked that.

>> Is this just a general precaution or did you run into an actual conflict?
> 
> The former.
>

Quoting Andreas Rheinhardt (2024-03-12 10:21:52)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2024-03-12 09:56:46)
> >> Anton Khirnov:
> >>> Otherwise it might conflict with caller-defined identifiers.
> >>>
> >>> ff because it is not supposed to be used by external callers.
> >>> ---
> >>
> >> This is public, so it should be av_attribute_deprecated.
> > 
> > As the commit message says, this is not supposed to be used by external
> > callers. Or do you see any reason why it should be public?
> > 
> 
> It is public given that it is declared in a public header. That is what
> matters.

I agree that it is (implicitly) public now, but the question is whether
it should be. IMO it should not. Do you disagree?

> Furthermore, we have used the FF_ and AV_ prefixes for public macros and
> the av_ prefix for public symbols, but IIRC we do not use the ff_ prefix
> for public symbols at all, therefore I'd like us not to reserve another
> namespace.

We do reserve the ff namespace for private identifiers that need to be
exposed, e.g. in public headers or when linking statically.

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-12 10:21:52)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2024-03-12 09:56:46)
>>>> Anton Khirnov:
>>>>> Otherwise it might conflict with caller-defined identifiers.
>>>>>
>>>>> ff because it is not supposed to be used by external callers.
>>>>> ---
>>>>
>>>> This is public, so it should be av_attribute_deprecated.
>>>
>>> As the commit message says, this is not supposed to be used by external
>>> callers. Or do you see any reason why it should be public?
>>>
>>
>> It is public given that it is declared in a public header. That is what
>> matters.
> 
> I agree that it is (implicitly) public now, but the question is whether
> it should be. IMO it should not. Do you disagree?
> 

It needs to be available in a public header given that deprecations are
supposed to convey information to the public. If you are asking whether
we should add a note like the one for the FF_API_ defines: I don't have
formed an opinion on that yet.

>> Furthermore, we have used the FF_ and AV_ prefixes for public macros and
>> the av_ prefix for public symbols, but IIRC we do not use the ff_ prefix
>> for public symbols at all, therefore I'd like us not to reserve another
>> namespace.
> 
> We do reserve the ff namespace for private identifiers that need to be
> exposed, e.g. in public headers or when linking statically.
> 

We reserve the FF_ namespace in public headers, not the ff_ namespace.
If I am not mistaken we never use the latter namespace in public for
stuff without linkage (i.e. a project could use the ff_ namespace for
macros/enums without conflicting with FFmpeg).

- Andreas