From patchwork Sat Oct 28 14:41:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Niklas Haas X-Patchwork-Id: 44410 Delivered-To: ffmpegpatchwork2@gmail.com Received: by 2002:a05:6a20:dd83:b0:15d:8365:d4b8 with SMTP id kw3csp498156pzb; Sat, 28 Oct 2023 07:45:39 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEODmvIVQ5et8pmfY/qjjNd22WIrfz5C0E65vx6V8IVg7r5DhMGga6pVXx/O7p3sYK70i2f X-Received: by 2002:a05:6512:b9c:b0:507:9861:2be9 with SMTP id b28-20020a0565120b9c00b0050798612be9mr4041454lfv.6.1698504339436; Sat, 28 Oct 2023 07:45:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698504339; cv=none; d=google.com; s=arc-20160816; b=EPOk4IFIisAEawL4p0xUfzFiv49jMC/cNE7eehIj8jUcJvNtBn7H3oa2DUmCwhaxZ+ 17+V+KzBe1u6BInwFB3P1aO+w95fQB3UqVjNegjS6Qj/++VJ4SvuutzE9JN8mB4fYF3t M0m/7Jwh5PTP68Y+8GX2yTJG0te7KkUJVSkwxlgcDpe1M6DWCrp32UFGJ+LKESGOmduz Eo905HA2LYMC7pBemPbf7qgXOqzlu9gJkORh74lvmjnL9AHl4LEY/lSByYIkJgKAZNOI rnauCx1ZAlz2zpb5eDVTB3Hc72udOMP0D9RpGplMC48gWaULKJjgYtoM4BocnrahijIW Sbiw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:reply-to :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:mime-version:references:in-reply-to :message-id:date:to:from:dkim-signature:delivered-to; bh=wQc46/83WPaep0QVh2BMUqxoWVFEQhnkvOfhORk5++Q=; fh=xmAeKtysnShNOmkhiJmYkS30uw4Fu2hvBJ7qlIwukxQ=; b=z+4YIKUaBeZQjOaiWkWxRdnrdPpKFNrbAaHhvRynwmOwsYrnxrvFF3PHJamrPOMG+o 9kSrUIG8nUiWQJsn1AxAOMbPQZnddhnnud4is22uLfKnWFwd5WIIKJzZF34ufdSPy14N 9J+DrdblQRMery3fNwTwFOAEu3RI0zacPvpqztESYRZAjmwAMGTGZRVxhWBnWwiE3p1L YZSeWcjOjxXnuQBEEa6XvAE8ZxLwmcZLUdqEgeT8EmGFv+iLGjiKBcWRIwAuyjnDEMeI eQPqiBZknZHBZ4Kz4hIn8dQKs1WUlx3+p2aqETII4VnJsg+xYSQd4MMpXqmDk8iZ5uDO Kxmg== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@haasn.xyz header.s=mail header.b=smN378Zs; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id i4-20020a05640242c400b00540346c84b4si1794630edc.542.2023.10.28.07.45.39; Sat, 28 Oct 2023 07:45:39 -0700 (PDT) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@haasn.xyz header.s=mail header.b=smN378Zs; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A9FAA68CBF0; Sat, 28 Oct 2023 17:44:48 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 7B8A568CBCB for ; Sat, 28 Oct 2023 17:44:40 +0300 (EEST) Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id 66CB44BB07; Sat, 28 Oct 2023 16:44:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1698504276; bh=NLV1tG6YIyW2ynchJR8f79MSRSGIESICMpGmP2xOuU4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=smN378ZsVdv4ZJtqbR6bBa61cUaPoIfzQlvDdvjxGMEMrlQNe0wNQxT+D0dCngaok vuutlmkWaIFT3+pvDuHwGf6wACRniArKz0p9zmtL1io2zcAhB35u3YhXa7LSWti5XF 8XjRHODnV1JfHbdsgLBuRb7euYijfB38o5WvHBC0= From: Niklas Haas To: ffmpeg-devel@ffmpeg.org Date: Sat, 28 Oct 2023 16:41:13 +0200 Message-ID: <20231028144430.60538-7-ffmpeg@haasn.xyz> X-Mailer: git-send-email 2.42.0 In-Reply-To: <20231028144430.60538-1-ffmpeg@haasn.xyz> References: <20231028144430.60538-1-ffmpeg@haasn.xyz> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH v2 06/10] avfilter/vf_scale: properly respect to input colorimetry X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Niklas Haas Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" X-TUID: 6wdMaVXzQ5GF From: Niklas Haas This code, as written, skips sws_init_context if scale->in_range was not set, even if scale->in_frame_range is set to something. So this would hit the 'no sws context' fast path in scale_frame and skip color range conversion even where technically required. This had the effect of, in practice, effectively applying limited/full range conversion if and only if you set e.g. a nonzero yuv color matrix, which is obviously undesirable behavior. Second, the assignment of out->color_range inside the branch would fail to properly propagate tags for any actually applied conversion, for example on conversion to a forced full range format. Solve both of these problems and make the code much easier to understand and follow by doing the following: 1. Always initialize sws context on get_props 2. Always use sws_getColorspaceDetails + sws_setColorspaceDetails to fix the conversion matrices per-frame. 3. Rather than testing if the context exists, do the no-op test after settling the above values and deciding what conversion to actually perform. --- libavfilter/vf_scale.c | 186 +++++++++++++++++------------------------ 1 file changed, 76 insertions(+), 110 deletions(-) diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c index 23335cef4b..3d58de0494 100644 --- a/libavfilter/vf_scale.c +++ b/libavfilter/vf_scale.c @@ -143,7 +143,6 @@ typedef struct ScaleContext { char *out_color_matrix; int in_range; - int in_frame_range; int out_range; int out_h_chr_pos; @@ -356,8 +355,6 @@ static av_cold int init(AVFilterContext *ctx) if (!threads) av_opt_set_int(scale->sws_opts, "threads", ff_filter_get_nb_threads(ctx), 0); - scale->in_frame_range = AVCOL_RANGE_UNSPECIFIED; - return 0; } @@ -520,6 +517,7 @@ static int config_props(AVFilterLink *outlink) const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format); const AVPixFmtDescriptor *outdesc = av_pix_fmt_desc_get(outfmt); ScaleContext *scale = ctx->priv; + struct SwsContext **swscs[3] = {&scale->sws, &scale->isws[0], &scale->isws[1]}; uint8_t *flags_val = NULL; int ret; @@ -552,65 +550,46 @@ static int config_props(AVFilterLink *outlink) if (scale->isws[1]) sws_freeContext(scale->isws[1]); scale->isws[0] = scale->isws[1] = scale->sws = NULL; - if (inlink0->w == outlink->w && - inlink0->h == outlink->h && - !scale->out_color_matrix && - scale->in_range == scale->out_range && - inlink0->format == outlink->format) - ; - else { - struct SwsContext **swscs[3] = {&scale->sws, &scale->isws[0], &scale->isws[1]}; - int i; - - for (i = 0; i < 3; i++) { - int in_v_chr_pos = scale->in_v_chr_pos, out_v_chr_pos = scale->out_v_chr_pos; - struct SwsContext *const s = sws_alloc_context(); - if (!s) - return AVERROR(ENOMEM); - *swscs[i] = s; - - ret = av_opt_copy(s, scale->sws_opts); - if (ret < 0) - return ret; - av_opt_set_int(s, "srcw", inlink0 ->w, 0); - av_opt_set_int(s, "srch", inlink0 ->h >> !!i, 0); - av_opt_set_int(s, "src_format", inlink0->format, 0); - av_opt_set_int(s, "dstw", outlink->w, 0); - av_opt_set_int(s, "dsth", outlink->h >> !!i, 0); - av_opt_set_int(s, "dst_format", outfmt, 0); - if (scale->in_range != AVCOL_RANGE_UNSPECIFIED) - av_opt_set_int(s, "src_range", - scale->in_range == AVCOL_RANGE_JPEG, 0); - else if (scale->in_frame_range != AVCOL_RANGE_UNSPECIFIED) - av_opt_set_int(s, "src_range", - scale->in_frame_range == AVCOL_RANGE_JPEG, 0); - if (scale->out_range != AVCOL_RANGE_UNSPECIFIED) - av_opt_set_int(s, "dst_range", - scale->out_range == AVCOL_RANGE_JPEG, 0); - - /* Override chroma location default settings to have the correct - * chroma positions. MPEG chroma positions are used by convention. - * Note that this works for both MPEG-1/JPEG and MPEG-2/4 chroma - * locations, since they share a vertical alignment */ - if (desc->log2_chroma_h == 1 && scale->in_v_chr_pos == -513) { - in_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192; - } - - if (outdesc->log2_chroma_h == 1 && scale->out_v_chr_pos == -513) { - out_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192; - } - - av_opt_set_int(s, "src_h_chr_pos", scale->in_h_chr_pos, 0); - av_opt_set_int(s, "src_v_chr_pos", in_v_chr_pos, 0); - av_opt_set_int(s, "dst_h_chr_pos", scale->out_h_chr_pos, 0); - av_opt_set_int(s, "dst_v_chr_pos", out_v_chr_pos, 0); - - if ((ret = sws_init_context(s, NULL, NULL)) < 0) - return ret; - if (!scale->interlaced) - break; + for (int i = 0; i < 3; i++) { + int in_v_chr_pos = scale->in_v_chr_pos, out_v_chr_pos = scale->out_v_chr_pos; + struct SwsContext *const s = sws_alloc_context(); + if (!s) + return AVERROR(ENOMEM); + *swscs[i] = s; + + ret = av_opt_copy(s, scale->sws_opts); + if (ret < 0) + return ret; + + av_opt_set_int(s, "srcw", inlink0 ->w, 0); + av_opt_set_int(s, "srch", inlink0 ->h >> !!i, 0); + av_opt_set_int(s, "src_format", inlink0->format, 0); + av_opt_set_int(s, "dstw", outlink->w, 0); + av_opt_set_int(s, "dsth", outlink->h >> !!i, 0); + av_opt_set_int(s, "dst_format", outfmt, 0); + + /* Override chroma location default settings to have the correct + * chroma positions. MPEG chroma positions are used by convention. + * Note that this works for both MPEG-1/JPEG and MPEG-2/4 chroma + * locations, since they share a vertical alignment */ + if (desc->log2_chroma_h == 1 && scale->in_v_chr_pos == -513) { + in_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192; + } + + if (outdesc->log2_chroma_h == 1 && scale->out_v_chr_pos == -513) { + out_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192; } + + av_opt_set_int(s, "src_h_chr_pos", scale->in_h_chr_pos, 0); + av_opt_set_int(s, "src_v_chr_pos", in_v_chr_pos, 0); + av_opt_set_int(s, "dst_h_chr_pos", scale->out_h_chr_pos, 0); + av_opt_set_int(s, "dst_v_chr_pos", out_v_chr_pos, 0); + + if ((ret = sws_init_context(s, NULL, NULL)) < 0) + return ret; + if (!scale->interlaced) + break; } if (inlink0->sample_aspect_ratio.num){ @@ -716,9 +695,10 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) AVFrame *out; const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format); char buf[32]; - int ret; - int in_range; + int in_full, out_full, brightness, contrast, saturation; + const int *inv_table, *table; int frame_changed; + int ret; *frame_out = NULL; if (in->colorspace == AVCOL_SPC_YCGCO) @@ -730,13 +710,6 @@ static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out) in->sample_aspect_ratio.den != link->sample_aspect_ratio.den || in->sample_aspect_ratio.num != link->sample_aspect_ratio.num; - if (in->color_range != AVCOL_RANGE_UNSPECIFIED && - scale->in_range == AVCOL_RANGE_UNSPECIFIED && - in->color_range != scale->in_frame_range) { - scale->in_frame_range = in->color_range; - frame_changed = 1; - } - if (scale->eval_mode == EVAL_MODE_FRAME || frame_changed) { unsigned vars_w[VARS_NB] = { 0 }, vars_h[VARS_NB] = { 0 }; @@ -804,7 +777,30 @@ FF_ENABLE_DEPRECATION_WARNINGS } scale: - if (!scale->sws) { + sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full, + (int **)&table, &out_full, + &brightness, &contrast, &saturation); + + if (scale->in_color_matrix) + inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace); + if (scale->out_color_matrix) + table = parse_yuv_type(scale->out_color_matrix, AVCOL_SPC_UNSPECIFIED); + else if (scale->in_color_matrix) + table = inv_table; + + if (scale->in_range != AVCOL_RANGE_UNSPECIFIED) + in_full = scale->in_range == AVCOL_RANGE_JPEG; + else if (in->color_range != AVCOL_RANGE_UNSPECIFIED) + in_full = in->color_range == AVCOL_RANGE_JPEG; + if (scale->out_range != AVCOL_RANGE_UNSPECIFIED) + out_full = scale->out_range == AVCOL_RANGE_JPEG; + + if (in->width == outlink->w && + in->height == outlink->h && + in->format == outlink->format && + !memcmp(inv_table, table, sizeof(int) * 4) && + in_full == out_full) { + /* no conversion needed */ *frame_out = in; return 0; } @@ -822,6 +818,7 @@ scale: av_frame_copy_props(out, in); out->width = outlink->w; out->height = outlink->h; + out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; // Sanity checks: // 1. If the output is RGB, set the matrix coefficients to RGB. @@ -838,48 +835,17 @@ scale: if (scale->output_is_pal) avpriv_set_systematic_pal2((uint32_t*)out->data[1], outlink->format == AV_PIX_FMT_PAL8 ? AV_PIX_FMT_BGR8 : outlink->format); - in_range = in->color_range; - - if ( scale->in_color_matrix - || scale->out_color_matrix - || scale-> in_range != AVCOL_RANGE_UNSPECIFIED - || in_range != AVCOL_RANGE_UNSPECIFIED - || scale->out_range != AVCOL_RANGE_UNSPECIFIED) { - int in_full, out_full, brightness, contrast, saturation; - const int *inv_table, *table; - - sws_getColorspaceDetails(scale->sws, (int **)&inv_table, &in_full, - (int **)&table, &out_full, - &brightness, &contrast, &saturation); - - if (scale->in_color_matrix) - inv_table = parse_yuv_type(scale->in_color_matrix, in->colorspace); - if (scale->out_color_matrix) - table = parse_yuv_type(scale->out_color_matrix, AVCOL_SPC_UNSPECIFIED); - else if (scale->in_color_matrix) - table = inv_table; - - if (scale-> in_range != AVCOL_RANGE_UNSPECIFIED) - in_full = (scale-> in_range == AVCOL_RANGE_JPEG); - else if (in_range != AVCOL_RANGE_UNSPECIFIED) - in_full = (in_range == AVCOL_RANGE_JPEG); - if (scale->out_range != AVCOL_RANGE_UNSPECIFIED) - out_full = (scale->out_range == AVCOL_RANGE_JPEG); - - sws_setColorspaceDetails(scale->sws, inv_table, in_full, + sws_setColorspaceDetails(scale->sws, inv_table, in_full, + table, out_full, + brightness, contrast, saturation); + if (scale->isws[0]) + sws_setColorspaceDetails(scale->isws[0], inv_table, in_full, + table, out_full, + brightness, contrast, saturation); + if (scale->isws[1]) + sws_setColorspaceDetails(scale->isws[1], inv_table, in_full, table, out_full, brightness, contrast, saturation); - if (scale->isws[0]) - sws_setColorspaceDetails(scale->isws[0], inv_table, in_full, - table, out_full, - brightness, contrast, saturation); - if (scale->isws[1]) - sws_setColorspaceDetails(scale->isws[1], inv_table, in_full, - table, out_full, - brightness, contrast, saturation); - - out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG; - } av_reduce(&out->sample_aspect_ratio.num, &out->sample_aspect_ratio.den, (int64_t)in->sample_aspect_ratio.num * outlink->h * link->w,