[FFmpeg-devel] [PATCH 4/4] ffmpeg: pass decoded or filtered AVFrame to output stream initialization

[Jan Ekström]

On Tue, Sep 15, 2020 at 9:12 PM Jan Ekström <jeebjp at gmail.com> wrote:
>
> On Tue, Sep 15, 2020 at 7:39 PM James Almer <jamrial at gmail.com> wrote:
> >
> > On 9/15/2020 1:21 PM, Jan Ekström wrote:
> > > On Tue, Sep 15, 2020 at 5:28 PM Jan Ekström <jeebjp at gmail.com> wrote:
> > >>
> > >>
> > >> Further looking into this, it seems to come from filtering (?) (diff attached)
> > >>
> > >> decoded AVFrame: 128x128, pix_fmt: rgba, range: pc
> > >> while...
> > >> AVFrame to be encoded: 128x128, pix_fmt: bgra, range: tv
> > >>
> > >> Jan
> > >
> > > For the record, the culprit seems to be this piece of logic within
> > > libavfilter/vf_scale.c:
> > > http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavfilter/vf_scale.c;h=58eee967440657798f84383ec6f79e8a05c3ece0;hb=HEAD#l747
> > >
> > > Only input has range defined, out_full never gets set to nonzero.
> > > Thus,
> > > out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG
> > > leads to AVCOL_RANGE_MPEG.
> >
> > Would changing that to out->color_range = scale->out_range help, or
> > break things too much?
> >
>
> I was informed that swscale doesn't say that RGB is full range, so I
> moved on to fix that since that is utilized as out_full here. Patch
> follows.
>
> After doing that, FATE still passed but...
>
> [swscaler @ 0x62f000000400] sws_init_context: srcRange: 1, dstRange: 1
> [auto_scaler_0 @ 0x611000007980] w:128 h:128 fmt:rgba sar:2835/2835 ->
> w:128 h:128 fmt:bgra sar:1/1 flags:0x4
> scale_frame: out_full: 0
> vf_scale AVFrame: 128x128, pix_fmt: bgra, range: tv
>
> I am very confused at this point :P
>
> Jan
>
> ------>8--------
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index 9ca378bd3b..59d4d5e873 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1200,8 +1200,11 @@ av_cold int sws_init_context(SwsContext *c,
> SwsFilter *srcFilter,
>
>      unscaled = (srcW == dstW && srcH == dstH);
>
> -    c->srcRange |= handle_jpeg(&c->srcFormat);
> -    c->dstRange |= handle_jpeg(&c->dstFormat);
> +    c->srcRange |= isAnyRGB(c->srcFormat) ? 1 : handle_jpeg(&c->srcFormat);
> +    c->dstRange |= isAnyRGB(c->dstFormat) ? 1 : handle_jpeg(&c->dstFormat);
> +
> +    av_log(c, AV_LOG_VERBOSE, "%s: srcRange: %d, dstRange: %d\n",
> +           __FUNCTION__, c->srcRange, c->dstRange);
>
>      if(srcFormat!=c->srcFormat || dstFormat!=c->dstFormat)
>          av_log(c, AV_LOG_WARNING, "deprecated pixel format used, make
> sure you did set range correctly\n");

OK, I found the silent override in sws_setColorspaceDetails. If this
is indeed how swscale notes what the input/output range is, maybe it
should be something a la the following? (maybe just without the
duplication by making it a function).

@@ -876,10 +914,23 @@ int sws_setColorspaceDetails(struct SwsContext
*c, const int inv_table[4],
     desc_dst = av_pix_fmt_desc_get(c->dstFormat);
     desc_src = av_pix_fmt_desc_get(c->srcFormat);

-    if(!isYUV(c->dstFormat) && !isGray(c->dstFormat))
-        dstRange = 0;
-    if(!isYUV(c->srcFormat) && !isGray(c->srcFormat))
-        srcRange = 0;
+    if ((isAnyRGB(c->dstFormat) || handle_jpeg(&c->dstFormat)) && !dstRange) {
+        av_log(c, AV_LOG_WARNING,
+               "Destination pixel format '%s' is considered full range, "
+               "but an attempt was made to set the conversion to limited "
+               "range. Enforcing full range.\n",
+               (desc_dst && desc_dst->name) ? desc_dst->name : "unknown");
+        dstRange = 1;
+    }
+
+    if ((isAnyRGB(c->srcFormat) || handle_jpeg(&c->srcFormat)) && !srcRange) {
+        av_log(c, AV_LOG_WARNING,
+               "Source pixel format '%s' is considered full range, "
+               "but an attempt was made to set the conversion to limited "
+               "range. Enforcing full range.\n",
+               (desc_dst && desc_dst->name) ? desc_dst->name : "unknown");
+        srcRange = 1;
+    }

     if (c->srcRange != srcRange ||
         c->dstRange != dstRange ||

Alternatively, if the swscale value is not the input/output range
exactly, maybe we should switch to something a la the following?

static int decide_color_range(ScaleContext *scale, AVFrame *in_frame,
                              AVFrame *out_frame)
{
    const AVPixFmtDescriptor *in_desc  = av_pix_fmt_desc_get(in_frame->format);
    const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(out_frame->format);
    if (!in_desc || !out_desc) {
        return AVERROR_INVALIDDATA;
    }

    int output_is_yuvj = \
        (out_desc->name && !strncmp(out_desc->name, "yuvj", 4));

    // first decide on the range. start by setting it to the
    // scale context's value.
    out_frame->color_range = scale->out_range;
    if (out_frame->color_range != AVCOL_RANGE_UNSPECIFIED)
        // we have a range specified for output, off ye go
        goto early_exit;

    // output range is unspecified, time to do some logic
    if (!!(in_desc->flags & AV_PIX_FMT_FLAG_RGB) ==
        !!(out_desc->flags & AV_PIX_FMT_FLAG_RGB)) {
        // if both input and output match in type, utilize the input range
        // (prefer parameter, although if that override is not available,
        // utilize the AVFrame's range)
        out_frame->color_range = (scale->in_range == AVCOL_RANGE_UNSPECIFIED) ?
            in_frame->color_range : scale->in_range;

        // if we got a value, run with it.
        if (out_frame->color_range != AVCOL_RANGE_UNSPECIFIED)
            goto early_exit;
    }

    // alright, if we are here neither the input or output knows what it
    // wants, but we know what will happen.
    // - If it's YCbCr, it is going to be full range for J formats and
    //   limited range for non-J.
    // - If it's RGB, it's going to be full range.
    out_frame->color_range = (out_desc->flags & AV_PIX_FMT_FLAG_RGB) ?
        AVCOL_RANGE_JPEG :
        (output_is_yuvj ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG);

early_exit:

    return 0;
}