Re: [FFmpeg-devel] [PATCH 05/25] avfilter/buffersrc: add color_space/range parameters

From: Anton Khirnov <anton@khirnov.net>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Niklas Haas <git@haasn.dev>
Subject: Re: [FFmpeg-devel] [PATCH 05/25] avfilter/buffersrc: add color_space/range parameters
Date: Wed, 06 Dec 2023 16:10:03 +0100
Message-ID: <170187540394.8914.18137862049617540717@lain.khirnov.net> (raw)
In-Reply-To: <20231109122534.124157-6-ffmpeg@haasn.xyz>

Quoting Niklas Haas (2023-11-09 13:19:37)
> @@ -328,6 +341,30 @@ static const AVOption buffer_options[] = {
>      { "pixel_aspect",  "sample aspect ratio",    OFFSET(pixel_aspect),     AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, DBL_MAX, V },
>      { "time_base",     NULL,                     OFFSET(time_base),        AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, DBL_MAX, V },
>      { "frame_rate",    NULL,                     OFFSET(frame_rate),       AV_OPT_TYPE_RATIONAL, { .dbl = 0 }, 0, DBL_MAX, V },
> +    { "colorspace", "select colorspace", OFFSET(color_space), AV_OPT_TYPE_INT, {.i64=AVCOL_SPC_UNSPECIFIED}, 0, AVCOL_SPC_NB-1, V, "colorspace"},
> +    {   "gbr",         NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_SPC_RGB},               INT_MIN, INT_MAX, V, "colorspace"},
            ^^^
        is that intentional?

> @@ -432,6 +471,15 @@ static int query_formats(AVFilterContext *ctx)
>          if ((ret = ff_add_format         (&formats, c->pix_fmt)) < 0 ||
>              (ret = ff_set_common_formats (ctx     , formats   )) < 0)
>              return ret;
> +        /* force specific colorspace/range downstream only for ordinary YUV */
> +        if (ff_fmt_is_regular_yuv(c->pix_fmt)) {

What will this do when the colorspace is not set, as it will not be with
existing callers.

> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
> index 3b248b37cd..4357a7bbfb 100644
> --- a/libavfilter/buffersrc.h
> +++ b/libavfilter/buffersrc.h
> @@ -105,6 +105,12 @@ typedef struct AVBufferSrcParameters {
>       */
>      AVBufferRef *hw_frames_ctx;
>  
> +    /**
> +     * Video only, the YUV colorspace and range
> +     */
> +    enum AVColorSpace color_space;
> +    enum AVColorRange color_range;

This has to go at the end of the struct, otherwise you're breaking ABI.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".