[FFmpeg-devel] [PATCH] Delay FF_API_AVFRAME_COLORSPACE until the next libavutil soname bump

[Hendrik Leppkes]

Am 28.07.2014 04:30 schrieb "Michael Niedermayer" <michaelni at gmx.at>:
>
> This works around ABI issues with applications which depend on libavutil
> internal values like sizeof(AVFrame)
> One such application is VLC 2.1.4 as well as 2.1.5
>
> Found-by: Andreas Cadhalpun <andreas.cadhalpun at googlemail.com>
> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> ---
>  libavcodec/utils.c  |    8 ++++----
>  libavutil/frame.c   |    8 ++++----
>  libavutil/frame.h   |   20 ++++++++++++++++++++
>  libavutil/version.h |    2 +-
>  4 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 6071380..26baa94 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -780,15 +780,15 @@ int ff_init_buffer_info(AVCodecContext *avctx,
AVFrame *frame)
>      }
>      frame->reordered_opaque = avctx->reordered_opaque;
>
> +    if (av_frame_get_colorspace(frame) == AVCOL_SPC_UNSPECIFIED)
> +        av_frame_set_colorspace(frame, avctx->colorspace);
> +    if (av_frame_get_color_range(frame) == AVCOL_RANGE_UNSPECIFIED)
> +        av_frame_set_color_range(frame, avctx->color_range);
>  #if FF_API_AVFRAME_COLORSPACE
>      if (frame->color_primaries == AVCOL_PRI_UNSPECIFIED)
>          frame->color_primaries = avctx->color_primaries;
>      if (frame->color_trc == AVCOL_TRC_UNSPECIFIED)
>          frame->color_trc = avctx->color_trc;
> -    if (av_frame_get_colorspace(frame) == AVCOL_SPC_UNSPECIFIED)
> -        av_frame_set_colorspace(frame, avctx->colorspace);
> -    if (av_frame_get_color_range(frame) == AVCOL_RANGE_UNSPECIFIED)
> -        av_frame_set_color_range(frame, avctx->color_range);
>      if (frame->chroma_location == AVCHROMA_LOC_UNSPECIFIED)
>          frame->chroma_location = avctx->chroma_sample_location;
>  #endif
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index fdfbc46..f18f9c1 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -104,11 +104,11 @@ static void get_frame_defaults(AVFrame *frame)
>      frame->sample_aspect_ratio = (AVRational){ 0, 1 };
>      frame->format              = -1; /* unknown */
>      frame->extended_data       = frame->data;
> +    frame->colorspace          = AVCOL_SPC_UNSPECIFIED;
> +    frame->color_range         = AVCOL_RANGE_UNSPECIFIED;
>  #if FF_API_AVFRAME_COLORSPACE
>      frame->color_primaries     = AVCOL_PRI_UNSPECIFIED;
>      frame->color_trc           = AVCOL_TRC_UNSPECIFIED;
> -    frame->colorspace          = AVCOL_SPC_UNSPECIFIED;
> -    frame->color_range         = AVCOL_RANGE_UNSPECIFIED;
>      frame->chroma_location     = AVCHROMA_LOC_UNSPECIFIED;
>  #endif
>  }
> @@ -482,11 +482,11 @@ int av_frame_copy_props(AVFrame *dst, const AVFrame
*src)
>      dst->display_picture_number = src->display_picture_number;
>      dst->flags                  = src->flags;
>      dst->decode_error_flags     = src->decode_error_flags;
> +    dst->colorspace             = src->colorspace;
> +    dst->color_range            = src->color_range;
>  #if FF_API_AVFRAME_COLORSPACE
>      dst->color_primaries        = src->color_primaries;
>      dst->color_trc              = src->color_trc;
> -    dst->colorspace             = src->colorspace;
> -    dst->color_range            = src->color_range;
>      dst->chroma_location        = src->chroma_location;
>  #endif
>
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index a39c8d0..93d27ad 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -526,6 +526,26 @@ typedef struct AVFrame {
>       */
>      int pkt_size;
>
> +#if !FF_API_AVFRAME_COLORSPACE
> +    /**
> +     * MPEG vs JPEG YUV range.
> +     * It must be accessed using av_frame_get_color_range() and
> +     * av_frame_set_color_range().
> +     * - encoding: Set by user
> +     * - decoding: Set by libavcodec
> +     */
> +    enum AVColorRange color_range;
> +
> +    /**
> +     * YUV colorspace type.
> +     * It must be accessed using av_frame_get_colorspace() and
> +     * av_frame_set_colorspace().
> +     * - encoding: Set by user
> +     * - decoding: Set by libavcodec
> +     */
> +    enum AVColorSpace colorspace;
> +#endif
> +
>      /**
>       * Not to be accessed directly from outside libavutil
>       */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 6d8d6f0..1deb6e4 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -138,7 +138,7 @@
>  #define FF_API_OPT_TYPE_METADATA        (LIBAVUTIL_VERSION_MAJOR < 54)
>  #endif
>  #ifndef FF_API_AVFRAME_COLORSPACE
> -#define FF_API_AVFRAME_COLORSPACE       (LIBAVUTIL_VERSION_MAJOR >= 52)
> +#define FF_API_AVFRAME_COLORSPACE       (LIBAVUTIL_VERSION_MAJOR >= 53)
>  #endif
>
>
> --
> 1.7.9.5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This is stupid. If we cannot even add new fields to structs which are
defined to have their size not part of the ABI, we might as well give up on
improving the API.

I'm against this. Broken user apps shouldn't dictate everything we do.