[FFmpeg-devel] [PATCH 1/2] avcodec, avformat: deprecate anything related to side data merging

James Almer jamrial at gmail.com
Thu Mar 16 16:07:44 EET 2017


On 3/16/2017 1:20 AM, wm4 wrote:
> This patch deprecates anything that has to do with merging/splitting
> side data. Automatic side data merging (and splitting), as well as all
> API symbols involved in it, are removed completely.
> 
> Two FF_API_ defines are dedicated to deprecating API symbols related to
> this: FF_API_MERGE_SD_API removes av_packet_split/merge_side_data in
> libavcodec, and FF_API_LAVF_KEEPSIDE_FLAG deprecates
> AVFMT_FLAG_KEEP_SIDE_DATA in libavformat.
> 
> Since it was claimed that changing the default from merging side data to
> not doing it is an ABI change, there are two additional FF_API_ defines,
> which stop using the side data merging/splitting by default (and remove
> any code in avformat/avcodec doing this): FF_API_MERGE_SD in libavcodec,
> and FF_API_LAVF_MERGE_SD in libavformat.
> 
> It is very much intended that FF_API_MERGE_SD and FF_API_LAVF_MERGE_SD
> are quickly defined to 0 in the next ABI bump, while the API symbols are
> retained for a longer time for the sake of compatibility.
> AVFMT_FLAG_KEEP_SIDE_DATA will (very much intentionally) do nothing for
> most of the time it will still be defined. Keep in mind that no code
> exists that actually tries to unset this flag for any reason, nor does
> such code need to exist. Code setting this flag explicitly will work as
> before. Thus it's ok for AVFMT_FLAG_KEEP_SIDE_DATA to do nothing once
> side data merging has been removed from libavformat.
> 
> In order to avoid that anyone in the future does this incorrectly, here
> is a small guide how to update the internal code on bumps:
> 
> - next ABI bump (probably soon):
>   - define FF_API_LAVF_MERGE_SD to 0, and remove all code covered by it
>   - define FF_API_MERGE_SD to 0, and remove all code covered by it
> - next API bump (typically two years in the future or so):
>   - define FF_API_LAVF_MERGE_SD to 0, and remove all code covered by it
>   - define FF_API_MERGE_SD_API to 0, and remove all code covered by it

I assume you meant to write FF_API_LAVF_KEEPSIDE_FLAG in the API bump part.

> 
> This forces anyone who actually wants packet side data to temporarily
> use deprecated API to get it all. If you ask me, this is batshit fucked
> up crazy, but it's how we roll. Making AVFMT_FLAG_KEEP_SIDE_DATA to be
> set by default was rejected as an ABI change, so I'm going all the way
> to get rid of this once and for all.
> ---
>  doc/APIchanges              |  5 +++++
>  libavcodec/avcodec.h        |  4 ++++
>  libavcodec/avpacket.c       |  4 ++++
>  libavcodec/utils.c          | 16 ++++++++++++++++
>  libavcodec/version.h        |  9 ++++++++-
>  libavformat/avformat.h      |  4 +++-
>  libavformat/mux.c           |  6 ++++++
>  libavformat/options_table.h |  2 ++
>  libavformat/utils.c         |  2 ++
>  libavformat/version.h       |  8 +++++++-
>  10 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index dc36a6bea7..acb67d38e4 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,11 @@ libavutil:     2015-08-28
>  
>  API changes, most recent first:
>  
> +2017-03-16 - xxxxxxx - lavf 57.66.105, lavc 57.83.101 - avformat.h, avcodec.h
> +  Deprecate AVFMT_FLAG_KEEP_SIDE_DATA. It will be ignored after the next major
> +  bump, and libavformat will behave as if it were always set.
> +  Deprecate av_packet_merge_side_data() and av_packet_split_side_data().
> +
>  2017-02-10 - xxxxxxx - lavu 55.48.100 / 55.33.0 - spherical.h
>    Add AV_SPHERICAL_EQUIRECTANGULAR_TILE, av_spherical_tile_bounds(),
>    and projection-specific properties (bound_left, bound_top, bound_right,
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index e32f57983c..e279dd59ee 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -4575,9 +4575,13 @@ int av_packet_shrink_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>  uint8_t* av_packet_get_side_data(AVPacket *pkt, enum AVPacketSideDataType type,
>                                   int *size);
>  
> +#if FF_API_MERGE_SD_API
> +attribute_deprecated
>  int av_packet_merge_side_data(AVPacket *pkt);
>  
> +attribute_deprecated
>  int av_packet_split_side_data(AVPacket *pkt);
> +#endif
>  
>  const char *av_packet_side_data_name(enum AVPacketSideDataType type);
>  
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index 60269aa63d..6af3bb68f1 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -379,6 +379,8 @@ const char *av_packet_side_data_name(enum AVPacketSideDataType type)
>      return NULL;
>  }
>  
> +#if FF_API_MERGE_SD_API
> +
>  #define FF_MERGE_MARKER 0x8c4d9d108e25e9feULL
>  
>  int av_packet_merge_side_data(AVPacket *pkt){
> @@ -460,6 +462,8 @@ int av_packet_split_side_data(AVPacket *pkt){
>      return 0;
>  }
>  
> +#endif
> +
>  uint8_t *av_packet_pack_dictionary(AVDictionary *dict, int *size)
>  {
>      AVDictionaryEntry *t = NULL;
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 4d1b63222f..456e707c8f 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -2263,7 +2263,9 @@ int attribute_align_arg avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi
>  
>      if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size ||
>          (avctx->active_thread_type & FF_THREAD_FRAME)) {
> +#if FF_API_MERGE_SD
>          int did_split = av_packet_split_side_data(&tmp);
> +#endif

You should do

#if FF_API_MERGE_SD
FF_DISABLE_DEPRECATION_WARNINGS
        int did_split = av_packet_split_side_data(&tmp);
FF_ENABLE_DEPRECATION_WARNINGS
#endif

To remove the "deprecated" warning spam during compilation. Same with every
other call to any of the deprecated functions.

What about the ffmpeg.c calls to av_packet_split_side_data()? You should
also wrap them with one of these new FF_API defines.

>          ret = apply_param_change(avctx, &tmp);
>          if (ret < 0)
>              goto fail;
> @@ -2295,11 +2297,13 @@ fail:
>          emms_c(); //needed to avoid an emms_c() call before every return;
>  
>          avctx->internal->pkt = NULL;
> +#if FF_API_MERGE_SD
>          if (did_split) {
>              av_packet_free_side_data(&tmp);
>              if(ret == tmp.size)
>                  ret = avpkt->size;
>          }
> +#endif
>          if (picture->flags & AV_FRAME_FLAG_DISCARD) {
>              *got_picture_ptr = 0;
>          }
> @@ -2369,7 +2373,9 @@ int attribute_align_arg avcodec_decode_audio4(AVCodecContext *avctx,
>          uint8_t discard_reason = 0;
>          // copy to ensure we do not change avpkt
>          AVPacket tmp = *avpkt;
> +#if FF_API_MERGE_SD
>          int did_split = av_packet_split_side_data(&tmp);
> +#endif
>          ret = apply_param_change(avctx, &tmp);
>          if (ret < 0)
>              goto fail;
> @@ -2481,11 +2487,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
>          }
>  fail:
>          avctx->internal->pkt = NULL;
> +#if FF_API_MERGE_SD
>          if (did_split) {
>              av_packet_free_side_data(&tmp);
>              if(ret == tmp.size)
>                  ret = avpkt->size;
>          }
> +#endif
>  
>          if (ret >= 0 && *got_frame_ptr) {
>              if (!avctx->refcounted_frames) {
> @@ -2682,6 +2690,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>      if ((avctx->codec->capabilities & AV_CODEC_CAP_DELAY) || avpkt->size) {
>          AVPacket pkt_recoded;
>          AVPacket tmp = *avpkt;
> +#if FF_API_MERGE_SD
>          int did_split = av_packet_split_side_data(&tmp);
>          //apply_param_change(avctx, &tmp);
>  
> @@ -2694,6 +2703,7 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>              memset(tmp.data + tmp.size, 0,
>                     FFMIN(avpkt->size - tmp.size, AV_INPUT_BUFFER_PADDING_SIZE));
>          }
> +#endif
>  
>          pkt_recoded = tmp;
>          ret = recode_subtitle(avctx, &pkt_recoded, &tmp);
> @@ -2753,11 +2763,13 @@ int avcodec_decode_subtitle2(AVCodecContext *avctx, AVSubtitle *sub,
>              avctx->internal->pkt = NULL;
>          }
>  
> +#if FF_API_MERGE_SD
>          if (did_split) {
>              av_packet_free_side_data(&tmp);
>              if(ret == tmp.size)
>                  ret = avpkt->size;
>          }
> +#endif
>  
>          if (*got_sub_ptr)
>              avctx->frame_number++;
> @@ -2873,12 +2885,16 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>      if (avctx->codec->send_packet) {
>          if (avpkt) {
>              AVPacket tmp = *avpkt;
> +#if FF_API_MERGE_SD
>              int did_split = av_packet_split_side_data(&tmp);
> +#endif
>              ret = apply_param_change(avctx, &tmp);
>              if (ret >= 0)
>                  ret = avctx->codec->send_packet(avctx, &tmp);
> +#if FF_API_MERGE_SD
>              if (did_split)
>                  av_packet_free_side_data(&tmp);
> +#endif
>              return ret;
>          } else {
>              return avctx->codec->send_packet(avctx, NULL);
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 3ed5a718d4..ec8651f086 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  
>  #define LIBAVCODEC_VERSION_MAJOR  57
>  #define LIBAVCODEC_VERSION_MINOR  83
> -#define LIBAVCODEC_VERSION_MICRO 100
> +#define LIBAVCODEC_VERSION_MICRO 101
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> @@ -157,6 +157,9 @@
>  #ifndef FF_API_VAAPI_CONTEXT
>  #define FF_API_VAAPI_CONTEXT     (LIBAVCODEC_VERSION_MAJOR < 58)
>  #endif
> +#ifndef FF_API_MERGE_SD
> +#define FF_API_MERGE_SD          (LIBAVCODEC_VERSION_MAJOR < 58)
> +#endif
>  #ifndef FF_API_AVCTX_TIMEBASE
>  #define FF_API_AVCTX_TIMEBASE    (LIBAVCODEC_VERSION_MAJOR < 59)
>  #endif
> @@ -229,5 +232,9 @@
>  #ifndef FF_API_STRUCT_VAAPI_CONTEXT
>  #define FF_API_STRUCT_VAAPI_CONTEXT (LIBAVCODEC_VERSION_MAJOR < 59)
>  #endif
> +#ifndef FF_API_MERGE_SD_API
> +#define FF_API_MERGE_SD_API      (LIBAVCODEC_VERSION_MAJOR < 59)
> +#endif
> +
>  
>  #endif /* AVCODEC_VERSION_H */
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 4c1b18e002..4ab217dc17 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1468,7 +1468,9 @@ typedef struct AVFormatContext {
>  #define AVFMT_FLAG_MP4A_LATM    0x8000 ///< Enable RTP MP4A-LATM payload
>  #define AVFMT_FLAG_SORT_DTS    0x10000 ///< try to interleave outputted packets by dts (using this flag can slow demuxing down)
>  #define AVFMT_FLAG_PRIV_OPT    0x20000 ///< Enable use of private options by delaying codec open (this could be made default once all code is converted)
> -#define AVFMT_FLAG_KEEP_SIDE_DATA 0x40000 ///< Don't merge side data but keep it separate.
> +#if FF_API_LAVF_KEEPSIDE_FLAG
> +#define AVFMT_FLAG_KEEP_SIDE_DATA 0x40000 ///< Don't merge side data but keep it separate. Deprecated, will be the default.
> +#endif
>  #define AVFMT_FLAG_FAST_SEEK   0x80000 ///< Enable fast, but inaccurate seeks for some formats
>  #define AVFMT_FLAG_SHORTEST   0x100000 ///< Stop muxing when the shortest stream stops.
>  #define AVFMT_FLAG_AUTO_BSF   0x200000 ///< Wait for packet data before writing a header, and add bitstream filters as requested by the muxer
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index e500531789..7de5d03fec 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -752,7 +752,9 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
>          }
>      }
>  
> +#if FF_API_LAVF_MERGE_SD
>      did_split = av_packet_split_side_data(pkt);
> +#endif
>  
>      if (!s->internal->header_written) {
>          ret = s->internal->write_header_ret ? s->internal->write_header_ret : write_header_internal(s);
> @@ -777,8 +779,10 @@ static int write_packet(AVFormatContext *s, AVPacket *pkt)
>      }
>  
>  fail:
> +#if FF_API_LAVF_MERGE_SD
>      if (did_split)
>          av_packet_merge_side_data(pkt);
> +#endif
>  
>      if (ret < 0) {
>          pkt->pts = pts_backup;
> @@ -875,8 +879,10 @@ static int do_packet_auto_bsf(AVFormatContext *s, AVPacket *pkt) {
>          }
>      }
>  
> +#if FF_API_LAVF_MERGE_SD
>      if (st->internal->nb_bsfcs)
>          av_packet_split_side_data(pkt);
> +#endif
>  
>      for (i = 0; i < st->internal->nb_bsfcs; i++) {
>          AVBSFContext *ctx = st->internal->bsfcs[i];
> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
> index a537dda95e..0c1915d6d4 100644
> --- a/libavformat/options_table.h
> +++ b/libavformat/options_table.h
> @@ -48,7 +48,9 @@ static const AVOption avformat_options[] = {
>  {"igndts", "ignore dts", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_IGNDTS }, INT_MIN, INT_MAX, D, "fflags"},
>  {"discardcorrupt", "discard corrupted frames", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_DISCARD_CORRUPT }, INT_MIN, INT_MAX, D, "fflags"},
>  {"sortdts", "try to interleave outputted packets by dts", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_SORT_DTS }, INT_MIN, INT_MAX, D, "fflags"},
> +#if FF_API_LAVF_KEEPSIDE_FLAG
>  {"keepside", "don't merge side data", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_KEEP_SIDE_DATA }, INT_MIN, INT_MAX, D, "fflags"},
> +#endif
>  {"fastseek", "fast but inaccurate seeks", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_FAST_SEEK }, INT_MIN, INT_MAX, D, "fflags"},
>  {"latm", "enable RTP MP4A-LATM payload", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_MP4A_LATM }, INT_MIN, INT_MAX, E, "fflags"},
>  {"nobuffer", "reduce the latency introduced by optional buffering", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_NOBUFFER }, 0, INT_MAX, D, "fflags"},
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 37d7024465..a4b4466851 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -1675,8 +1675,10 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              st->inject_global_side_data = 0;
>          }
>  
> +#if FF_API_LAVF_MERGE_SD
>          if (!(s->flags & AVFMT_FLAG_KEEP_SIDE_DATA))
>              av_packet_merge_side_data(pkt);
> +#endif
>      }
>  
>      av_opt_get_dict_val(s, "metadata", AV_OPT_SEARCH_CHILDREN, &metadata);
> diff --git a/libavformat/version.h b/libavformat/version.h
> index dc689d45fb..bfc42e3f15 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -33,7 +33,7 @@
>  // Also please add any ticket numbers that you believe might be affected here
>  #define LIBAVFORMAT_VERSION_MAJOR  57
>  #define LIBAVFORMAT_VERSION_MINOR  66
> -#define LIBAVFORMAT_VERSION_MICRO 104
> +#define LIBAVFORMAT_VERSION_MICRO 105
>  
>  #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
>                                                 LIBAVFORMAT_VERSION_MINOR, \
> @@ -88,6 +88,12 @@
>  #ifndef FF_API_HLS_WRAP
>  #define FF_API_HLS_WRAP                 (LIBAVFORMAT_VERSION_MAJOR < 58)
>  #endif
> +#ifndef FF_API_LAVF_MERGE_SD
> +#define FF_API_LAVF_MERGE_SD            (LIBAVFORMAT_VERSION_MAJOR < 58)
> +#endif
> +#ifndef FF_API_LAVF_KEEPSIDE_FLAG
> +#define FF_API_LAVF_KEEPSIDE_FLAG       (LIBAVFORMAT_VERSION_MAJOR < 58)

This one should probably be set as "< 59", much like FF_API_MERGE_SD_API.

> +#endif
>  
>  
>  #ifndef FF_API_R_FRAME_RATE
> 



More information about the ffmpeg-devel mailing list