[FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context

[FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

More commonly, this fixes the case of sws_setColorspaceDetails after
sws_getContext, since the latter implies sws_init_context.

The problem here is that sws_init_context sets up the range conversion
and fast path tables based on the values of srcRange/dstRange at init
time. This may result in locking in a "wrong" path (either using
unscaled fast path when range conversion later required, or using
scaled slow path when range conversion becomes no longer required).

There are two way outs:

1. Always initialize range conversion and unscaled converters, even if
   they will be unused, and extend the runtime check.
2. Re-do initialization if the values change after
   sws_setColorspaceDetails.

I opted for approach 1 because it was simpler and easier to reason
about.
---
 libswscale/swscale.c | 2 +-
 libswscale/utils.c   | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/libswscale/swscale.c b/libswscale/swscale.c
index 90e5b299ab..46ba68fe6a 100644
--- a/libswscale/swscale.c
+++ b/libswscale/swscale.c
@@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
     reset_ptr(src2, c->srcFormat);
     reset_ptr((void*)dst2, c->dstFormat);
 
-    if (c->convert_unscaled) {
+    if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
         int offset  = srcSliceY_internal;
         int slice_h = srcSliceH;
 
diff --git a/libswscale/utils.c b/libswscale/utils.c
index e1ad685972..455955e907 100644
--- a/libswscale/utils.c
+++ b/libswscale/utils.c
@@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
     }
 
     /* unscaled special cases */
-    if (unscaled && !usesHFilter && !usesVFilter &&
-        (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
-         isFloat(srcFormat) || isFloat(dstFormat))){
+    if (unscaled && !usesHFilter && !usesVFilter) {
         ff_get_unscaled_swscale(c);
 
         if (c->convert_unscaled) {
@@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
                 av_log(c, AV_LOG_INFO,
                        "using unscaled %s -> %s special converter\n",
                        av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
-            return 0;
         }
     }
 
-- 
2.42.0

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Alpha planes are explicitly full range, even for limited range YUVA
formats. Mark them as such.
---
 libavfilter/vf_extractplanes.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavfilter/vf_extractplanes.c b/libavfilter/vf_extractplanes.c
index 7b7149ab24..ca406ff323 100644
--- a/libavfilter/vf_extractplanes.c
+++ b/libavfilter/vf_extractplanes.c
@@ -312,6 +312,8 @@ static int extract_plane(AVFilterLink *outlink, AVFrame *frame)
     if (!out)
         return AVERROR(ENOMEM);
     av_frame_copy_props(out, frame);
+    if (idx == 3 /* alpha */)
+        out->color_range = AVCOL_RANGE_JPEG;
 
     if (s->is_packed) {
         extract_from_packed(out->data[0], out->linesize[0],
-- 
2.42.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/8] avfilter/vf_extractplanes: tag alpha plane as full range

[Vittorio Giovara]

On Fri, Oct 27, 2023 at 10:05 AM Niklas Haas <ffmpeg@haasn.xyz> wrote:

> From: Niklas Haas <git@haasn.dev>
>
> Alpha planes are explicitly full range, even for limited range YUVA
> formats. Mark them as such.
> ---
>  libavfilter/vf_extractplanes.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavfilter/vf_extractplanes.c
> b/libavfilter/vf_extractplanes.c
> index 7b7149ab24..ca406ff323 100644
> --- a/libavfilter/vf_extractplanes.c
> +++ b/libavfilter/vf_extractplanes.c
> @@ -312,6 +312,8 @@ static int extract_plane(AVFilterLink *outlink,
> AVFrame *frame)
>      if (!out)
>          return AVERROR(ENOMEM);
>      av_frame_copy_props(out, frame);
> +    if (idx == 3 /* alpha */)
> +        out->color_range = AVCOL_RANGE_JPEG;
>

this might be overkill, but do you think the check here should be made more
generic, in case the alpha plane is not in position #3?
-- 
Vittorio
_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not full range

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Alpha planes must always be full range, so complain loudly if fed
limited range grayscale input.
---
 libavfilter/vf_alphamerge.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavfilter/vf_alphamerge.c b/libavfilter/vf_alphamerge.c
index 4bbc06da36..b814aa64f0 100644
--- a/libavfilter/vf_alphamerge.c
+++ b/libavfilter/vf_alphamerge.c
@@ -60,6 +60,12 @@ static int do_alphamerge(FFFrameSync *fs)
     if (!alpha_buf)
         return ff_filter_frame(ctx->outputs[0], main_buf);
 
+    if (alpha_buf->color_range == AVCOL_RANGE_MPEG) {
+        av_log(ctx, AV_LOG_WARNING, "alpha plane color range tagged as %s, "
+               "output will be wrong!",
+               av_color_range_name(alpha_buf->color_range));
+    }
+
     if (s->is_packed_rgb) {
         int x, y;
         uint8_t *pin, *pout;
-- 
2.42.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/8] avfilter/vf_alphamerge: warn if input not full range

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 540 bytes --]

On Fri, Oct 27, 2023 at 07:04:41PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> Alpha planes must always be full range, so complain loudly if fed
> limited range grayscale input.
> ---
>  libavfilter/vf_alphamerge.c | 6 ++++++
>  1 file changed, 6 insertions(+)

should be ok

[...]

thx
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

[FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

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..19c91cb86e 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 &&
+        inv_table == table &&
+        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,
-- 
2.42.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1658 bytes --]

On Fri, Oct 27, 2023 at 07:04:42PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> 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(-)

This breaks
tickets/2939/lena.jpg

(color looks slightly wrong compared to reference lena image)



[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry

[Niklas Haas]

On Fri, 27 Oct 2023 23:01:25 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Oct 27, 2023 at 07:04:42PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > 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(-)
> 
> This breaks
> tickets/2939/lena.jpg
> 
> (color looks slightly wrong compared to reference lena image)

How do I reproduce this? I tested with:

  ffmpeg -i lena.jpg lena.png

But the resulting checksum was identical before and after this commit.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 4/8] avfilter/vf_scale: properly respect to input colorimetry

[Niklas Haas]

On Sat, 28 Oct 2023 12:50:14 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> On Fri, 27 Oct 2023 23:01:25 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Oct 27, 2023 at 07:04:42PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > > 
> > > 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(-)
> > 
> > This breaks
> > tickets/2939/lena.jpg
> > 
> > (color looks slightly wrong compared to reference lena image)
> 
> How do I reproduce this? I tested with:
> 
>   ffmpeg -i lena.jpg lena.png
> 
> But the resulting checksum was identical before and after this commit.

Nvm, I see the issue. Actually, the bug is in patch 1 - the logic is
insufficient to cover all cases, since ff_sws_init_range_convert is
never re-called after changing range.

I will rewrite that commit, which should fix this one also.
_______________________________________________
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".

[FFmpeg-devel] [PATCH 5/8] avfilter/vf_scale: preserve YUV range by default

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

YUV->YUV conversions should preserve input range, if the output range is
unspecified. Ensures full-range YUV input comes out as full-range YUV
output by default, even through YUV->YUV pixel format conversions.
---
 libavfilter/vf_scale.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 19c91cb86e..f9b0746072 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -687,6 +687,28 @@ static int scale_field(ScaleContext *scale, AVFrame *dst, AVFrame *src,
     return 0;
 }
 
+static int is_regular_yuv(enum AVPixelFormat fmt)
+{
+    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(fmt);
+    if (desc->flags & (AV_PIX_FMT_FLAG_RGB | AV_PIX_FMT_FLAG_PAL))
+        return 0;
+    if (desc->name && av_strstart(desc->name, "xyz", NULL))
+        return 0;
+    if (desc->nb_components < 3)
+        return 0; /* grayscale is forced full range inside libswscale */
+    switch (fmt) {
+    case AV_PIX_FMT_YUVJ420P:
+    case AV_PIX_FMT_YUVJ422P:
+    case AV_PIX_FMT_YUVJ444P:
+    case AV_PIX_FMT_YUVJ440P:
+    case AV_PIX_FMT_YUVJ411P:
+        return 0;
+    default:
+        return 1;
+    }
+}
+
+
 static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
 {
     AVFilterContext *ctx = link->dst;
@@ -794,6 +816,8 @@ scale:
         in_full = in->color_range == AVCOL_RANGE_JPEG;
     if (scale->out_range != AVCOL_RANGE_UNSPECIFIED)
         out_full = scale->out_range == AVCOL_RANGE_JPEG;
+    else if (is_regular_yuv(in->format) && is_regular_yuv(outlink->format))
+        out_full = in_full; /* preserve pixel range by default */
 
     if (in->width == outlink->w &&
         in->height == outlink->h &&
-- 
2.42.0

_______________________________________________
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".

[FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

No need to write a custom string parser when we can just use an integer
option with preset values. The various bits of fallback logic are wholly
redundant with equivalent logic already inside sws_getCoefficients.
---
 libavfilter/vf_scale.c | 60 +++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 41 deletions(-)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f9b0746072..4a2f0bd1f1 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -139,8 +139,8 @@ typedef struct ScaleContext {
 
     char *flags_str;
 
-    char *in_color_matrix;
-    char *out_color_matrix;
+    int in_color_matrix;
+    int out_color_matrix;
 
     int in_range;
     int out_range;
@@ -407,30 +407,6 @@ static int query_formats(AVFilterContext *ctx)
     return 0;
 }
 
-static const int *parse_yuv_type(const char *s, enum AVColorSpace colorspace)
-{
-    if (!s)
-        s = "bt601";
-
-    if (s && strstr(s, "bt709")) {
-        colorspace = AVCOL_SPC_BT709;
-    } else if (s && strstr(s, "fcc")) {
-        colorspace = AVCOL_SPC_FCC;
-    } else if (s && strstr(s, "smpte240m")) {
-        colorspace = AVCOL_SPC_SMPTE240M;
-    } else if (s && (strstr(s, "bt601") || strstr(s, "bt470") || strstr(s, "smpte170m"))) {
-        colorspace = AVCOL_SPC_BT470BG;
-    } else if (s && strstr(s, "bt2020")) {
-        colorspace = AVCOL_SPC_BT2020_NCL;
-    }
-
-    if (colorspace < 1 || colorspace > 10 || colorspace == 8) {
-        colorspace = AVCOL_SPC_BT470BG;
-    }
-
-    return sws_getCoefficients(colorspace);
-}
-
 static int scale_eval_dimensions(AVFilterContext *ctx)
 {
     ScaleContext *scale = ctx->priv;
@@ -803,11 +779,13 @@ scale:
                              (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)
+    if (scale->in_color_matrix == -1 /* auto */)
+        inv_table = sws_getCoefficients(in->colorspace);
+    else if (scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED)
+        inv_table = sws_getCoefficients(scale->in_color_matrix);
+    if (scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
+        table = sws_getCoefficients(scale->out_color_matrix);
+    else if (scale->in_color_matrix != AVCOL_SPC_UNSPECIFIED)
         table = inv_table;
 
     if (scale->in_range != AVCOL_RANGE_UNSPECIFIED)
@@ -993,16 +971,16 @@ static const AVOption scale_options[] = {
     { "interl", "set interlacing", OFFSET(interlaced), AV_OPT_TYPE_BOOL, {.i64 = 0 }, -1, 1, FLAGS },
     { "size",   "set video size",          OFFSET(size_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, FLAGS },
     { "s",      "set video size",          OFFSET(size_str), AV_OPT_TYPE_STRING, {.str = NULL}, 0, FLAGS },
-    {  "in_color_matrix", "set input YCbCr type",   OFFSET(in_color_matrix),  AV_OPT_TYPE_STRING, { .str = "auto" }, .flags = FLAGS, "color" },
-    { "out_color_matrix", "set output YCbCr type",  OFFSET(out_color_matrix), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = FLAGS,  "color"},
-        { "auto",        NULL, 0, AV_OPT_TYPE_CONST, { .str = "auto" },      0, 0, FLAGS, "color" },
-        { "bt601",       NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt601" },     0, 0, FLAGS, "color" },
-        { "bt470",       NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt470" },     0, 0, FLAGS, "color" },
-        { "smpte170m",   NULL, 0, AV_OPT_TYPE_CONST, { .str = "smpte170m" }, 0, 0, FLAGS, "color" },
-        { "bt709",       NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt709" },     0, 0, FLAGS, "color" },
-        { "fcc",         NULL, 0, AV_OPT_TYPE_CONST, { .str = "fcc" },       0, 0, FLAGS, "color" },
-        { "smpte240m",   NULL, 0, AV_OPT_TYPE_CONST, { .str = "smpte240m" }, 0, 0, FLAGS, "color" },
-        { "bt2020",      NULL, 0, AV_OPT_TYPE_CONST, { .str = "bt2020" },    0, 0, FLAGS, "color" },
+    {  "in_color_matrix", "set input YCbCr type",   OFFSET(in_color_matrix),  AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color" },
+    { "out_color_matrix", "set output YCbCr type",  OFFSET(out_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color"},
+        { "auto",        NULL, 0, AV_OPT_TYPE_CONST, { .i64 = -1 },                     0, 0, FLAGS, "color" },
+        { "bt601",       NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },      0, 0, FLAGS, "color" },
+        { "bt470",       NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },      0, 0, FLAGS, "color" },
+        { "smpte170m",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT470BG },      0, 0, FLAGS, "color" },
+        { "bt709",       NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT709 },        0, 0, FLAGS, "color" },
+        { "fcc",         NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_FCC },          0, 0, FLAGS, "color" },
+        { "smpte240m",   NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_SMPTE240M },    0, 0, FLAGS, "color" },
+        { "bt2020",      NULL, 0, AV_OPT_TYPE_CONST, { .i64 = AVCOL_SPC_BT2020_NCL },   0, 0, FLAGS, "color" },
     {  "in_range", "set input color range",  OFFSET( in_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
     { "out_range", "set output color range", OFFSET(out_range), AV_OPT_TYPE_INT, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 2, FLAGS, "range" },
     { "auto",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_UNSPECIFIED }, 0, 0, FLAGS, "range" },
-- 
2.42.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 6/8] avfilter/vf_scale: simplify color matrix parsing logic

[Niklas Haas]

On Fri, 27 Oct 2023 19:04:44 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> -    {  "in_color_matrix", "set input YCbCr type",   OFFSET(in_color_matrix),  AV_OPT_TYPE_STRING, { .str = "auto" }, .flags = FLAGS, "color" },
> -    { "out_color_matrix", "set output YCbCr type",  OFFSET(out_color_matrix), AV_OPT_TYPE_STRING, { .str = NULL }, .flags = FLAGS,  "color"},
> +    {  "in_color_matrix", "set input YCbCr type",   OFFSET(in_color_matrix),  AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color" },
> +    { "out_color_matrix", "set output YCbCr type",  OFFSET(out_color_matrix), AV_OPT_TYPE_INT, { .i64 = AVCOL_SPC_UNSPECIFIED }, 0, AVCOL_SPC_NB-1, .flags = FLAGS, "color"},

Bug: Should be set to -1 by default, to preserve old behavior. Will fix
in v2.
_______________________________________________
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".

[FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

When using vf_scale to force a specific output color space, also tag
this on the AVFrame. (Mirroring existing logic for output range)
---
 libavfilter/vf_scale.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 4a2f0bd1f1..d79d67c413 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -821,6 +821,9 @@ scale:
     out->width  = outlink->w;
     out->height = outlink->h;
     out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
+    if (scale->out_color_matrix >= 0 &&
+        scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
+        out->colorspace = scale->out_color_matrix;
 
     // Sanity checks:
     //   1. If the output is RGB, set the matrix coefficients to RGB.
-- 
2.42.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 7/8] avfilter/vf_scale: tag output color space

[Vittorio Giovara]

On Fri, Oct 27, 2023 at 10:06 AM Niklas Haas <ffmpeg@haasn.xyz> wrote:

> From: Niklas Haas <git@haasn.dev>
>
> When using vf_scale to force a specific output color space, also tag
> this on the AVFrame. (Mirroring existing logic for output range)
> ---
>  libavfilter/vf_scale.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 4a2f0bd1f1..d79d67c413 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -821,6 +821,9 @@ scale:
>      out->width  = outlink->w;
>      out->height = outlink->h;
>      out->color_range = out_full ? AVCOL_RANGE_JPEG : AVCOL_RANGE_MPEG;
> +    if (scale->out_color_matrix >= 0 &&
> +        scale->out_color_matrix != AVCOL_SPC_UNSPECIFIED)
> +        out->colorspace = scale->out_color_matrix;
>

what about the other color properties?
-- 
Vittorio
_______________________________________________
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".

[FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

PGMYUV seems to be always limited range. This was a format originally
invented by FFmpeg at a time when YUVJ distinguished limited from full
range YUV, and this codec never appeared to output YUVJ in any
circumstance, so hard-coding limited range preserves the status quo.

The other formats are explicitly documented to be full range RGB/gray
formats. That said, don't tag them yet, due to outstanding bugs w.r.t
grayscale formats and color range handling.

This change in behavior updates a bunch of FATE tests in trivial ways
(added tagging being the only difference).
---
 libavcodec/pnm.c              |  5 ++++
 tests/ref/lavf/mkv            |  4 ++--
 tests/ref/lavf/mkv_attachment |  4 ++--
 tests/ref/lavf/mxf            |  6 ++---
 tests/ref/lavf/y4m            |  4 ++--
 tests/ref/seek/lavf-mkv       | 44 +++++++++++++++++------------------
 tests/ref/seek/lavf-y4m       | 22 +++++++++---------
 7 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/libavcodec/pnm.c b/libavcodec/pnm.c
index 77d24eeaf7..bc51f94b1c 100644
--- a/libavcodec/pnm.c
+++ b/libavcodec/pnm.c
@@ -240,5 +240,10 @@ int ff_pnm_decode_header(AVCodecContext *avctx, PNMContext * const s)
         h /= 3;
         avctx->height = h;
     }
+
+    /* PGMYUV is always limited range */
+    if (avctx->codec_id == AV_CODEC_ID_PGMYUV)
+        avctx->color_range = AVCOL_RANGE_MPEG;
+
     return 0;
 }
diff --git a/tests/ref/lavf/mkv b/tests/ref/lavf/mkv
index a8c3fd13e8..5a3c3b931e 100644
--- a/tests/ref/lavf/mkv
+++ b/tests/ref/lavf/mkv
@@ -1,3 +1,3 @@
-6224bc0893bd0bb8a789e74bbd38c9c7 *tests/data/lavf/lavf.mkv
-320440 tests/data/lavf/lavf.mkv
+dd709c2b5e173eaca39cdd4a10aac3ec *tests/data/lavf/lavf.mkv
+320447 tests/data/lavf/lavf.mkv
 tests/data/lavf/lavf.mkv CRC=0xec6c3c68
diff --git a/tests/ref/lavf/mkv_attachment b/tests/ref/lavf/mkv_attachment
index 4c958af162..1a086a4f24 100644
--- a/tests/ref/lavf/mkv_attachment
+++ b/tests/ref/lavf/mkv_attachment
@@ -1,3 +1,3 @@
-05132b99d16128e552c1a6f1619be8b7 *tests/data/lavf/lavf.mkv_attachment
-472590 tests/data/lavf/lavf.mkv_attachment
+7cd7b06892b74d66da217c8dda90bfac *tests/data/lavf/lavf.mkv_attachment
+472597 tests/data/lavf/lavf.mkv_attachment
 tests/data/lavf/lavf.mkv_attachment CRC=0xec6c3c68
diff --git a/tests/ref/lavf/mxf b/tests/ref/lavf/mxf
index fdd1ef5c9c..4cf3388afa 100644
--- a/tests/ref/lavf/mxf
+++ b/tests/ref/lavf/mxf
@@ -1,9 +1,9 @@
-9ec1ad83b3400de11ca2f70b3b2d4c4d *tests/data/lavf/lavf.mxf
+fac1fb467168a374e96ea12755558869 *tests/data/lavf/lavf.mxf
 526393 tests/data/lavf/lavf.mxf
 tests/data/lavf/lavf.mxf CRC=0x8dddfaab
-3edfabe839a29f5902969c15ebac6d8d *tests/data/lavf/lavf.mxf
+d711481c4f81f6466fd92bdc7ed6c968 *tests/data/lavf/lavf.mxf
 551481 tests/data/lavf/lavf.mxf
 tests/data/lavf/lavf.mxf CRC=0xf091e687
-5bd0ce691510e6fae969886c32ad7a14 *tests/data/lavf/lavf.mxf
+7f4f8048c4f2d714e45947d4f39b8ea3 *tests/data/lavf/lavf.mxf
 526393 tests/data/lavf/lavf.mxf
 tests/data/lavf/lavf.mxf CRC=0x8dddfaab
diff --git a/tests/ref/lavf/y4m b/tests/ref/lavf/y4m
index 82c7087673..3c3fa830ad 100644
--- a/tests/ref/lavf/y4m
+++ b/tests/ref/lavf/y4m
@@ -1,3 +1,3 @@
-ec8178cb152f9cdbfd9cb724d977db2e *tests/data/lavf/lavf.y4m
-3801808 tests/data/lavf/lavf.y4m
+54f4ebcffedc886835444bb9d6aba3fb *tests/data/lavf/lavf.y4m
+3801828 tests/data/lavf/lavf.y4m
 tests/data/lavf/lavf.y4m CRC=0x0a941f26
diff --git a/tests/ref/seek/lavf-mkv b/tests/ref/seek/lavf-mkv
index b8028dd075..e327959058 100644
--- a/tests/ref/seek/lavf-mkv
+++ b/tests/ref/seek/lavf-mkv
@@ -1,48 +1,48 @@
-ret: 0         st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos:    682 size:   208
+ret: 0         st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos:    689 size:   208
 ret: 0         st:-1 flags:0  ts:-1.000000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
 ret: 0         st:-1 flags:1  ts: 1.894167
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
 ret: 0         st: 0 flags:0  ts: 0.788000
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
 ret: 0         st: 0 flags:1  ts:-0.317000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
 ret:-1         st: 1 flags:0  ts: 2.577000
 ret: 0         st: 1 flags:1  ts: 1.471000
-ret: 0         st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320158 size:   209
+ret: 0         st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320165 size:   209
 ret: 0         st:-1 flags:0  ts: 0.365002
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146866 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146873 size: 27925
 ret: 0         st:-1 flags:1  ts:-0.740831
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
 ret:-1         st: 0 flags:0  ts: 2.153000
 ret: 0         st: 0 flags:1  ts: 1.048000
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
 ret: 0         st: 1 flags:0  ts:-0.058000
-ret: 0         st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos:    682 size:   208
+ret: 0         st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos:    689 size:   208
 ret: 0         st: 1 flags:1  ts: 2.836000
-ret: 0         st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320158 size:   209
+ret: 0         st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320165 size:   209
 ret:-1         st:-1 flags:0  ts: 1.730004
 ret: 0         st:-1 flags:1  ts: 0.624171
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146866 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146873 size: 27925
 ret: 0         st: 0 flags:0  ts:-0.482000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
 ret: 0         st: 0 flags:1  ts: 2.413000
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
 ret:-1         st: 1 flags:0  ts: 1.307000
 ret: 0         st: 1 flags:1  ts: 0.201000
-ret: 0         st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos:    682 size:   208
+ret: 0         st: 1 flags:1 dts:-0.011000 pts:-0.011000 pos:    689 size:   208
 ret: 0         st:-1 flags:0  ts:-0.904994
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
 ret: 0         st:-1 flags:1  ts: 1.989173
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
 ret: 0         st: 0 flags:0  ts: 0.883000
-ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292314 size: 27834
+ret: 0         st: 0 flags:1 dts: 0.960000 pts: 0.960000 pos: 292321 size: 27834
 ret: 0         st: 0 flags:1  ts:-0.222000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
 ret:-1         st: 1 flags:0  ts: 2.672000
 ret: 0         st: 1 flags:1  ts: 1.566000
-ret: 0         st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320158 size:   209
+ret: 0         st: 1 flags:1 dts: 0.982000 pts: 0.982000 pos: 320165 size:   209
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146866 size: 27925
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos: 146873 size: 27925
 ret: 0         st:-1 flags:1  ts:-0.645825
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    898 size: 27837
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:    905 size: 27837
diff --git a/tests/ref/seek/lavf-y4m b/tests/ref/seek/lavf-y4m
index c416b4657b..67793ea40c 100644
--- a/tests/ref/seek/lavf-y4m
+++ b/tests/ref/seek/lavf-y4m
@@ -1,19 +1,19 @@
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     84 size:152064
 ret:-1         st:-1 flags:0  ts:-1.000000
 ret: 0         st:-1 flags:1  ts: 1.894167
 ret:-EOF
 ret: 0         st: 0 flags:0  ts: 0.800000
-ret: 0         st: 0 flags:1 dts: 0.800000 pts: 0.800000 pos:3041464 size:152064
+ret: 0         st: 0 flags:1 dts: 0.800000 pts: 0.800000 pos:3041484 size:152064
 ret: 0         st: 0 flags:1  ts:-0.320000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     84 size:152064
 ret: 0         st:-1 flags:0  ts: 2.576668
 ret:-EOF
 ret: 0         st:-1 flags:1  ts: 1.470835
 ret:-EOF
 ret: 0         st: 0 flags:0  ts: 0.360000
-ret: 0         st: 0 flags:1 dts: 0.360000 pts: 0.360000 pos:1368694 size:152064
+ret: 0         st: 0 flags:1 dts: 0.360000 pts: 0.360000 pos:1368714 size:152064
 ret: 0         st: 0 flags:1  ts:-0.760000
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     84 size:152064
 ret: 0         st:-1 flags:0  ts: 2.153336
 ret:-EOF
 ret: 0         st:-1 flags:1  ts: 1.047503
@@ -24,26 +24,26 @@ ret:-EOF
 ret: 0         st:-1 flags:0  ts: 1.730004
 ret:-EOF
 ret: 0         st:-1 flags:1  ts: 0.624171
-ret: 0         st: 0 flags:1 dts: 0.600000 pts: 0.600000 pos:2281114 size:152064
+ret: 0         st: 0 flags:1 dts: 0.600000 pts: 0.600000 pos:2281134 size:152064
 ret:-1         st: 0 flags:0  ts:-0.480000
 ret: 0         st: 0 flags:1  ts: 2.400000
 ret:-EOF
 ret: 0         st:-1 flags:0  ts: 1.306672
 ret:-EOF
 ret: 0         st:-1 flags:1  ts: 0.200839
-ret: 0         st: 0 flags:1 dts: 0.160000 pts: 0.160000 pos: 608344 size:152064
+ret: 0         st: 0 flags:1 dts: 0.160000 pts: 0.160000 pos: 608364 size:152064
 ret:-1         st: 0 flags:0  ts:-0.920000
 ret: 0         st: 0 flags:1  ts: 2.000000
 ret:-EOF
 ret: 0         st:-1 flags:0  ts: 0.883340
-ret: 0         st: 0 flags:1 dts: 0.880000 pts: 0.880000 pos:3345604 size:152064
+ret: 0         st: 0 flags:1 dts: 0.880000 pts: 0.880000 pos:3345624 size:152064
 ret: 0         st:-1 flags:1  ts:-0.222493
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     84 size:152064
 ret: 0         st: 0 flags:0  ts: 2.680000
 ret:-EOF
 ret: 0         st: 0 flags:1  ts: 1.560000
 ret:-EOF
 ret: 0         st:-1 flags:0  ts: 0.460008
-ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos:1824904 size:152064
+ret: 0         st: 0 flags:1 dts: 0.480000 pts: 0.480000 pos:1824924 size:152064
 ret: 0         st:-1 flags:1  ts:-0.645825
-ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     64 size:152064
+ret: 0         st: 0 flags:1 dts: 0.000000 pts: 0.000000 pos:     84 size:152064
-- 
2.42.0

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 8/8] avcodec/pnm: explicitly tag color range

[Vittorio Giovara]

On Fri, Oct 27, 2023 at 10:06 AM Niklas Haas <ffmpeg@haasn.xyz> wrote:

> From: Niklas Haas <git@haasn.dev>
>
> PGMYUV seems to be always limited range. This was a format originally
> invented by FFmpeg at a time when YUVJ distinguished limited from full
> range YUV, and this codec never appeared to output YUVJ in any
> circumstance, so hard-coding limited range preserves the status quo.
>
> The other formats are explicitly documented to be full range RGB/gray
> formats. That said, don't tag them yet, due to outstanding bugs w.r.t
> grayscale formats and color range handling.
>
> This change in behavior updates a bunch of FATE tests in trivial ways
> (added tagging being the only difference).
> ---
>  libavcodec/pnm.c              |  5 ++++
>  tests/ref/lavf/mkv            |  4 ++--
>  tests/ref/lavf/mkv_attachment |  4 ++--
>  tests/ref/lavf/mxf            |  6 ++---
>  tests/ref/lavf/y4m            |  4 ++--
>  tests/ref/seek/lavf-mkv       | 44 +++++++++++++++++------------------
>  tests/ref/seek/lavf-y4m       | 22 +++++++++---------
>  7 files changed, 47 insertions(+), 42 deletions(-)
>
> diff --git a/libavcodec/pnm.c b/libavcodec/pnm.c
> index 77d24eeaf7..bc51f94b1c 100644
> --- a/libavcodec/pnm.c
> +++ b/libavcodec/pnm.c
> @@ -240,5 +240,10 @@ int ff_pnm_decode_header(AVCodecContext *avctx,
> PNMContext * const s)
>          h /= 3;
>          avctx->height = h;
>      }
> +
> +    /* PGMYUV is always limited range */
> +    if (avctx->codec_id == AV_CODEC_ID_PGMYUV)
> +        avctx->color_range = AVCOL_RANGE_MPEG;
>

Why does this check for the codec_id?
-- 
Vittorio
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3006 bytes --]

On Fri, Oct 27, 2023 at 07:04:39PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> More commonly, this fixes the case of sws_setColorspaceDetails after
> sws_getContext, since the latter implies sws_init_context.
> 
> The problem here is that sws_init_context sets up the range conversion
> and fast path tables based on the values of srcRange/dstRange at init
> time. This may result in locking in a "wrong" path (either using
> unscaled fast path when range conversion later required, or using
> scaled slow path when range conversion becomes no longer required).
> 
> There are two way outs:
> 
> 1. Always initialize range conversion and unscaled converters, even if
>    they will be unused, and extend the runtime check.
> 2. Re-do initialization if the values change after
>    sws_setColorspaceDetails.
> 
> I opted for approach 1 because it was simpler and easier to reason
> about.
> ---
>  libswscale/swscale.c | 2 +-
>  libswscale/utils.c   | 5 +----
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> index 90e5b299ab..46ba68fe6a 100644
> --- a/libswscale/swscale.c
> +++ b/libswscale/swscale.c
> @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
>      reset_ptr(src2, c->srcFormat);
>      reset_ptr((void*)dst2, c->dstFormat);
>  
> -    if (c->convert_unscaled) {
> +    if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
>          int offset  = srcSliceY_internal;
>          int slice_h = srcSliceH;
>  
> diff --git a/libswscale/utils.c b/libswscale/utils.c
> index e1ad685972..455955e907 100644
> --- a/libswscale/utils.c
> +++ b/libswscale/utils.c
> @@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
>      }
>  
>      /* unscaled special cases */
> -    if (unscaled && !usesHFilter && !usesVFilter &&
> -        (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
> -         isFloat(srcFormat) || isFloat(dstFormat))){
> +    if (unscaled && !usesHFilter && !usesVFilter) {
>          ff_get_unscaled_swscale(c);
>  
>          if (c->convert_unscaled) {
> @@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
>                  av_log(c, AV_LOG_INFO,
>                         "using unscaled %s -> %s special converter\n",
>                         av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> -            return 0;
>          }
>      }

the av_log() message will be wrong if this path is unused

also this ties convert_unscaled to never do range conversion, if thats
intended, i guess thats ok. Otherwise that maybe is a restriction
we dont want to add.

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context

[Niklas Haas]

On Fri, 27 Oct 2023 23:42:41 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Fri, Oct 27, 2023 at 07:04:39PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > More commonly, this fixes the case of sws_setColorspaceDetails after
> > sws_getContext, since the latter implies sws_init_context.
> > 
> > The problem here is that sws_init_context sets up the range conversion
> > and fast path tables based on the values of srcRange/dstRange at init
> > time. This may result in locking in a "wrong" path (either using
> > unscaled fast path when range conversion later required, or using
> > scaled slow path when range conversion becomes no longer required).
> > 
> > There are two way outs:
> > 
> > 1. Always initialize range conversion and unscaled converters, even if
> >    they will be unused, and extend the runtime check.
> > 2. Re-do initialization if the values change after
> >    sws_setColorspaceDetails.
> > 
> > I opted for approach 1 because it was simpler and easier to reason
> > about.
> > ---
> >  libswscale/swscale.c | 2 +-
> >  libswscale/utils.c   | 5 +----
> >  2 files changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > index 90e5b299ab..46ba68fe6a 100644
> > --- a/libswscale/swscale.c
> > +++ b/libswscale/swscale.c
> > @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
> >      reset_ptr(src2, c->srcFormat);
> >      reset_ptr((void*)dst2, c->dstFormat);
> >  
> > -    if (c->convert_unscaled) {
> > +    if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
> >          int offset  = srcSliceY_internal;
> >          int slice_h = srcSliceH;
> >  
> > diff --git a/libswscale/utils.c b/libswscale/utils.c
> > index e1ad685972..455955e907 100644
> > --- a/libswscale/utils.c
> > +++ b/libswscale/utils.c
> > @@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> >      }
> >  
> >      /* unscaled special cases */
> > -    if (unscaled && !usesHFilter && !usesVFilter &&
> > -        (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
> > -         isFloat(srcFormat) || isFloat(dstFormat))){
> > +    if (unscaled && !usesHFilter && !usesVFilter) {
> >          ff_get_unscaled_swscale(c);
> >  
> >          if (c->convert_unscaled) {
> > @@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> >                  av_log(c, AV_LOG_INFO,
> >                         "using unscaled %s -> %s special converter\n",
> >                         av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> > -            return 0;
> >          }
> >      }
> 
> the av_log() message will be wrong if this path is unused

Indeed. Perhaps the only way to fix that would be to instead try
approach 2 and go through full reinit if any params change after
setColorspaceDetails.

> also this ties convert_unscaled to never do range conversion, if thats
> intended, i guess thats ok. Otherwise that maybe is a restriction
> we dont want to add.

In the original code, c->convert_unscaled is set behind a `c->srcRange
== c->dstRange` check, so how is that a change in behavior?
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 4298 bytes --]

On Sat, Oct 28, 2023 at 12:46:38PM +0200, Niklas Haas wrote:
> On Fri, 27 Oct 2023 23:42:41 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Fri, Oct 27, 2023 at 07:04:39PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > > 
> > > More commonly, this fixes the case of sws_setColorspaceDetails after
> > > sws_getContext, since the latter implies sws_init_context.
> > > 
> > > The problem here is that sws_init_context sets up the range conversion
> > > and fast path tables based on the values of srcRange/dstRange at init
> > > time. This may result in locking in a "wrong" path (either using
> > > unscaled fast path when range conversion later required, or using
> > > scaled slow path when range conversion becomes no longer required).
> > > 
> > > There are two way outs:
> > > 
> > > 1. Always initialize range conversion and unscaled converters, even if
> > >    they will be unused, and extend the runtime check.
> > > 2. Re-do initialization if the values change after
> > >    sws_setColorspaceDetails.
> > > 
> > > I opted for approach 1 because it was simpler and easier to reason
> > > about.
> > > ---
> > >  libswscale/swscale.c | 2 +-
> > >  libswscale/utils.c   | 5 +----
> > >  2 files changed, 2 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/libswscale/swscale.c b/libswscale/swscale.c
> > > index 90e5b299ab..46ba68fe6a 100644
> > > --- a/libswscale/swscale.c
> > > +++ b/libswscale/swscale.c
> > > @@ -1016,7 +1016,7 @@ static int scale_internal(SwsContext *c,
> > >      reset_ptr(src2, c->srcFormat);
> > >      reset_ptr((void*)dst2, c->dstFormat);
> > >  
> > > -    if (c->convert_unscaled) {
> > > +    if (c->convert_unscaled && !c->lumConvertRange && !c->chrConvertRange) {
> > >          int offset  = srcSliceY_internal;
> > >          int slice_h = srcSliceH;
> > >  
> > > diff --git a/libswscale/utils.c b/libswscale/utils.c
> > > index e1ad685972..455955e907 100644
> > > --- a/libswscale/utils.c
> > > +++ b/libswscale/utils.c
> > > @@ -1728,9 +1728,7 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> > >      }
> > >  
> > >      /* unscaled special cases */
> > > -    if (unscaled && !usesHFilter && !usesVFilter &&
> > > -        (c->srcRange == c->dstRange || isAnyRGB(dstFormat) ||
> > > -         isFloat(srcFormat) || isFloat(dstFormat))){
> > > +    if (unscaled && !usesHFilter && !usesVFilter) {
> > >          ff_get_unscaled_swscale(c);
> > >  
> > >          if (c->convert_unscaled) {
> > > @@ -1738,7 +1736,6 @@ static av_cold int sws_init_single_context(SwsContext *c, SwsFilter *srcFilter,
> > >                  av_log(c, AV_LOG_INFO,
> > >                         "using unscaled %s -> %s special converter\n",
> > >                         av_get_pix_fmt_name(srcFormat), av_get_pix_fmt_name(dstFormat));
> > > -            return 0;
> > >          }
> > >      }
> > 
> > the av_log() message will be wrong if this path is unused
> 
> Indeed. Perhaps the only way to fix that would be to instead try
> approach 2 and go through full reinit if any params change after
> setColorspaceDetails.
> 

> > also this ties convert_unscaled to never do range conversion, if thats
> > intended, i guess thats ok. Otherwise that maybe is a restriction
> > we dont want to add.
> 
> In the original code, c->convert_unscaled is set behind a `c->srcRange
> == c->dstRange` check, so how is that a change in behavior?

not "change in behavior" but it may be more complex if a
yuv->yuv unscaled convert is added that supports range convert
because before convert_unscaled is set only when it can be used
afterwards convert_unscaled is always set but sometimes it then
can be used with differing yuv ranges sometimes not.

what i meant is that the conditions related to convert_unscaled
could become more dispersed and duplciated _IF_ specific future
code is added
also its not just c->srcRange == c->dstRange, the condition is
more complex

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/8] swscale: fix sws_setColorspaceDetails after sws_init_context

[Niklas Haas]

On Fri, 27 Oct 2023 23:42:41 +0200 Michael Niedermayer <michael@niedermayer.cc> wrote:
> the av_log() message will be wrong if this path is unused

Well, this is a problem even if we move the log printing to context
reinit after sws_setColorspaceDetails, because it will still get printed
on the initial init (before range is known), unless we move it to
sws_scale_frame() itself. But then we would need to keep track also of
whether or not we already printed this.

Since this is just a developer-facing debug message, I'll probably just
reword it.
_______________________________________________
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".