[FFmpeg-devel] [PATCH v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[FFmpeg-devel] [PATCH v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Based on my best understanding of what they do, given the source code.
---
 libswscale/swscale.h | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/libswscale/swscale.h b/libswscale/swscale.h
index 9d4612aaf3..e22931cab4 100644
--- a/libswscale/swscale.h
+++ b/libswscale/swscale.h
@@ -82,11 +82,35 @@ const char *swscale_license(void);
 #define SWS_PRINT_INFO              0x1000
 
 //the following 3 flags are not completely implemented
-//internal chrominance subsampling info
+
+/**
+ * Perform full chroma upsampling when converting to RGB as part of scaling.
+ *
+ * For example, when converting 50x50 yuv420p to 100x100 rgba, setting this flag
+ * will scale the chroma plane from 25x25 to 100x100 (4:4:4), and then convert
+ * the 100x100 yuv444p image to rgba in the final output step.
+ *
+ * Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
+ * with a single chroma sample being re-used for both horizontally adjacent RGBA
+ * output pixels.
+ */
 #define SWS_FULL_CHR_H_INT    0x2000
-//input subsampling info
+
+/**
+ * Perform full chroma interpolation when downscaling RGB sources.
+ *
+ * For example, when converting a 100x100 rgba source to 50x50 yuv444p, setting
+ * this flag will generate a 100x100 (4:4:4) chroma plane, which is then
+ * downscaled to the required 50x50.
+ *
+ * Without this flag, the chroma plane is instead generated at 50x100 (dropping
+ * every other pixel), before then being downscaled to the required 50x50
+ * resolution.
+ */
 #define SWS_FULL_CHR_H_INP    0x4000
+
 #define SWS_DIRECT_BGR        0x8000
+
 #define SWS_ACCURATE_RND      0x40000
 #define SWS_BITEXACT          0x80000
 #define SWS_ERROR_DIFFUSION  0x800000
-- 
2.45.2

_______________________________________________
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 v2 2/8] avfilter/vf_setparams: allow setting chroma location

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Shockingly, there isn't currently _any_ filter for overriding this.
---
 doc/filters.texi           | 17 +++++++++++++++++
 libavfilter/vf_setparams.c | 19 ++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index c9c4f7cf6b..ca8f6e461a 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -22073,6 +22073,23 @@ Keep the same colorspace property (default).
 @item chroma-derived-c
 @item ictcp
 @end table
+
+@item chroma_location
+Set the chroma sample location.
+Available values are:
+
+@table @samp
+@item auto
+Keep the same chroma location (default).
+
+@item unspecified, unknown
+@item left
+@item center
+@item topleft
+@item top
+@item bottomleft
+@item bottom
+@end table
 @end table
 
 @section sharpen_npp
diff --git a/libavfilter/vf_setparams.c b/libavfilter/vf_setparams.c
index b7da8eb54b..ba32a6699c 100644
--- a/libavfilter/vf_setparams.c
+++ b/libavfilter/vf_setparams.c
@@ -41,6 +41,7 @@ typedef struct SetParamsContext {
     int color_primaries;
     int color_trc;
     int colorspace;
+    int chroma_location;
 } SetParamsContext;
 
 #define OFFSET(x) offsetof(SetParamsContext, x)
@@ -119,6 +120,17 @@ static const AVOption setparams_options[] = {
     {"chroma-derived-c",           NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_SPC_CHROMA_DERIVED_CL}, INT_MIN, INT_MAX, FLAGS, .unit = "colorspace"},
     {"ictcp",                      NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_SPC_ICTCP},             INT_MIN, INT_MAX, FLAGS, .unit = "colorspace"},
     {"ipt-c2",                     NULL,  0, AV_OPT_TYPE_CONST, {.i64=AVCOL_SPC_IPT_C2},            INT_MIN, INT_MAX, FLAGS, .unit = "colorspace"},
+
+    {"chroma_location", "select chroma sample location", OFFSET(chroma_location), AV_OPT_TYPE_INT, {.i64=-1}, -1, AVCHROMA_LOC_NB-1, FLAGS, .unit = "chroma_location"},
+    {"auto", "keep the same chroma location",  0, AV_OPT_TYPE_CONST, {.i64=-1},                       INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"unspecified",                      NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_UNSPECIFIED}, INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"unknown",                          NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_UNSPECIFIED}, INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"left",                             NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_LEFT},        INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"center",                           NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_CENTER},      INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"topleft",                          NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_TOPLEFT},     INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"top",                              NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_TOP},         INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"bottomleft",                       NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_BOTTOMLEFT},  INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
+    {"bottom",                           NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_BOTTOM},      INT_MIN, INT_MAX, FLAGS, .unit = "chroma_location"},
     {NULL}
 };
 
@@ -174,17 +186,18 @@ FF_ENABLE_DEPRECATION_WARNINGS
             frame->flags &= ~AV_FRAME_FLAG_TOP_FIELD_FIRST;
     }
 
-    /* set range */
+    /* set straightforward parameters */
     if (s->color_range >= 0)
         frame->color_range = s->color_range;
-
-    /* set color prim, trc, space */
     if (s->color_primaries >= 0)
         frame->color_primaries = s->color_primaries;
     if (s->color_trc >= 0)
         frame->color_trc = s->color_trc;
     if (s->colorspace >= 0)
         frame->colorspace = s->colorspace;
+    if (s->chroma_location >= 0)
+        frame->chroma_location = s->chroma_location;
+
     return ff_filter_frame(ctx->outputs[0], frame);
 }
 
-- 
2.45.2

_______________________________________________
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 v2 3/8] swscale/options: relax src/dst_h/v_chr_pos value range

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

When dealing with 4x subsampling ratios (log2 == 2), such as can arise
with 4:1:1 or 4:1:0, a value range of 512 is not enough to cover the
range of possible scenarios.

For example, bottom-sited chroma in 4:1:0 would require an offset of 768
(three luma rows). Simply double the limit to 1024. I don't see any
place in initFilter() that would experience overflow as a result of this
change, especially since get_local_pos() right-shifts it by the
subsampling ratio again.
---
 libswscale/options.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libswscale/options.c b/libswscale/options.c
index b36f13c872..53c4f0651b 100644
--- a/libswscale/options.c
+++ b/libswscale/options.c
@@ -62,10 +62,10 @@ static const AVOption swscale_options[] = {
     { "param0",          "scaler param 0",                OFFSET(param[0]),  AV_OPT_TYPE_DOUBLE, { .dbl = SWS_PARAM_DEFAULT  }, INT_MIN, INT_MAX,        VE },
     { "param1",          "scaler param 1",                OFFSET(param[1]),  AV_OPT_TYPE_DOUBLE, { .dbl = SWS_PARAM_DEFAULT  }, INT_MIN, INT_MAX,        VE },
 
-    { "src_v_chr_pos",   "source vertical chroma position in luma grid/256"  ,      OFFSET(src_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      512,             VE },
-    { "src_h_chr_pos",   "source horizontal chroma position in luma grid/256",      OFFSET(src_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      512,             VE },
-    { "dst_v_chr_pos",   "destination vertical chroma position in luma grid/256"  , OFFSET(dst_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      512,             VE },
-    { "dst_h_chr_pos",   "destination horizontal chroma position in luma grid/256", OFFSET(dst_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      512,             VE },
+    { "src_v_chr_pos",   "source vertical chroma position in luma grid/256"  ,      OFFSET(src_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      1024,             VE },
+    { "src_h_chr_pos",   "source horizontal chroma position in luma grid/256",      OFFSET(src_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      1024,             VE },
+    { "dst_v_chr_pos",   "destination vertical chroma position in luma grid/256"  , OFFSET(dst_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      1024,             VE },
+    { "dst_h_chr_pos",   "destination horizontal chroma position in luma grid/256", OFFSET(dst_h_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513 }, -513,      1024,             VE },
 
     { "sws_dither",      "set dithering algorithm",       OFFSET(dither),    AV_OPT_TYPE_INT,    { .i64  = SWS_DITHER_AUTO    }, 0,       NB_SWS_DITHER,  VE, .unit = "sws_dither" },
     { "auto",            "leave choice to sws",           0,                 AV_OPT_TYPE_CONST,  { .i64  = SWS_DITHER_AUTO    }, INT_MIN, INT_MAX,        VE, .unit = "sws_dither" },
-- 
2.45.2

_______________________________________________
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 v2 3/8] swscale/options: relax src/dst_h/v_chr_pos value range

[Michael Niedermayer]

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

On Thu, Jul 04, 2024 at 04:30:59PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> When dealing with 4x subsampling ratios (log2 == 2), such as can arise
> with 4:1:1 or 4:1:0, a value range of 512 is not enough to cover the
> range of possible scenarios.
> 
> For example, bottom-sited chroma in 4:1:0 would require an offset of 768
> (three luma rows). Simply double the limit to 1024. I don't see any
> place in initFilter() that would experience overflow as a result of this
> change, especially since get_local_pos() right-shifts it by the
> subsampling ratio again.
> ---
>  libswscale/options.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

maybe limit it to the max value you know is needed instead of simply doubling.
it can be bumped up more later if we run in cases needing more

either way
LGTM

thx

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

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- 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 v2 4/8] avfilter/swscale: always fix interlaced chroma location

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

The current logic only fixes it when the user does not explicitly
specify the chroma location. However, this does not make a lot of sense.
Since there is no way to specify this property per-field, it effectively
*prevents* the user from being able to correctly scale interlaced frames
with top-aligned chroma.

It makes more sense to consider the user setting in the progressive case
only, and automatically adapt it to the correct interlaced field
positions, following the details of the MPEG specification.
---
 libavfilter/vf_scale.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 841075193e..0b6701673f 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -709,12 +709,18 @@ static int config_props(AVFilterLink *outlink)
              * 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 (desc->log2_chroma_h == 1) {
+                if (in_v_chr_pos == -513)
+                    in_v_chr_pos = 128; /* explicitly default missing info */
+                in_v_chr_pos += 256 * (i == 2); /* offset by one luma row for odd rows */
+                in_v_chr_pos >>= i > 0; /* double luma row distance */
             }
 
-            if (outdesc->log2_chroma_h == 1 && scale->out_v_chr_pos == -513) {
-                out_v_chr_pos = (i == 0) ? 128 : (i == 1) ? 64 : 192;
+            if (outdesc->log2_chroma_h == 1) {
+                if (out_v_chr_pos == -513)
+                    out_v_chr_pos = 128;
+                out_v_chr_pos += 256 * (i == 2);
+                out_v_chr_pos >>= i > 0;
             }
 
             av_opt_set_int(s, "src_h_chr_pos", scale->in_h_chr_pos, 0);
-- 
2.45.2

_______________________________________________
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 v2 5/8] avfilter/vf_scale: add in/out_chroma_loc

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Currently, this just functions as a more principled and user-friendly
replacement for the (undocumented and hard to use) *_chr_pos fields.

However, the goal is to automatically infer these values from the input
frames' chroma location, and deprecate the manual use of *_chr_pos
altogether. (Indeed, my plans for an swscale replacement will most
likely also end up limiting the set of legal chroma locations to those
permissible by AVFrame properties)
---
 doc/filters.texi       | 15 ++++++++
 libavfilter/vf_scale.c | 86 ++++++++++++++++++++++++++++++------------
 2 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index ca8f6e461a..3cff4eec1c 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -21137,6 +21137,21 @@ Set full range (0-255 in case of 8-bit luma).
 Set "MPEG" range (16-235 in case of 8-bit luma).
 @end table
 
+@item in_chroma_loc
+@item out_chroma_loc
+Set in/output chroma sample location. If not specified, center-sited chroma
+is used by default. Possible values:
+
+@table @samp
+@item auto, unknown
+@item left
+@item center
+@item topleft
+@item top
+@item bottomleft
+@item bottom
+@end table
+
 @item force_original_aspect_ratio
 Enable decreasing or increasing output video width or height if necessary to
 keep the original aspect ratio. Possible values:
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index 0b6701673f..b2c9d0b187 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -168,6 +168,8 @@ typedef struct ScaleContext {
     int in_range;
     int out_range;
 
+    int in_chroma_loc;
+    int out_chroma_loc;
     int out_h_chr_pos;
     int out_v_chr_pos;
     int in_h_chr_pos;
@@ -617,6 +619,40 @@ fail:
     return ret;
 }
 
+static void calc_chroma_pos(int *h_pos_out, int *v_pos_out, int chroma_loc,
+                            int h_pos_override, int v_pos_override,
+                            int h_sub, int v_sub, int index)
+{
+    int h_pos, v_pos;
+
+    /* Explicitly default to center siting for compatibility with swscale */
+    if (chroma_loc == AVCHROMA_LOC_UNSPECIFIED)
+        chroma_loc = AVCHROMA_LOC_CENTER;
+
+    /* av_chroma_location_enum_to_pos() always gives us values in the range from
+     * 0 to 256, but we need to adjust this to the true value range of the
+     * subsampling grid, which may be larger for h/v_sub > 1 */
+    av_chroma_location_enum_to_pos(&h_pos, &v_pos, chroma_loc);
+    h_pos *= (1 << h_sub) - 1;
+    v_pos *= (1 << v_sub) - 1;
+
+    if (h_pos_override != -513)
+        h_pos = h_pos_override;
+    if (v_pos_override != -513)
+        v_pos = v_pos_override;
+
+    /* Fix vertical chroma position for interlaced frames */
+    if (v_sub == 1 && index > 0) {
+        v_pos += 256 * (index == 2); /* offset by one luma row for odd rows */
+        v_pos >>= 1; /* double luma row distance */
+    }
+
+    /* Explicitly strip chroma offsets when not subsampling, because it
+     * interferes with the operation of flags like SWS_FULL_CHR_H_INP */
+    *h_pos_out = h_sub ? h_pos : -513;
+    *v_pos_out = v_sub ? v_pos : -513;
+}
+
 static int config_props(AVFilterLink *outlink)
 {
     AVFilterContext *ctx = outlink->src;
@@ -673,15 +709,16 @@ static int config_props(AVFilterLink *outlink)
         inlink0->h == outlink->h &&
         in_range == outlink->color_range &&
         in_colorspace == outlink->colorspace &&
-        inlink0->format == outlink->format)
+        inlink0->format == outlink->format &&
+        scale->in_chroma_loc == scale->out_chroma_loc)
         ;
     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;
             int in_full, out_full, brightness, contrast, saturation;
+            int h_chr_pos, v_chr_pos;
             const int *inv_table, *table;
             struct SwsContext *const s = sws_alloc_context();
             if (!s)
@@ -705,28 +742,17 @@ static int config_props(AVFilterLink *outlink)
                 av_opt_set_int(s, "dst_range",
                                outlink->color_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) {
-                if (in_v_chr_pos == -513)
-                    in_v_chr_pos = 128; /* explicitly default missing info */
-                in_v_chr_pos += 256 * (i == 2); /* offset by one luma row for odd rows */
-                in_v_chr_pos >>= i > 0; /* double luma row distance */
-            }
-
-            if (outdesc->log2_chroma_h == 1) {
-                if (out_v_chr_pos == -513)
-                    out_v_chr_pos = 128;
-                out_v_chr_pos += 256 * (i == 2);
-                out_v_chr_pos >>= i > 0;
-            }
-
-            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);
+            calc_chroma_pos(&h_chr_pos, &v_chr_pos, scale->in_chroma_loc,
+                            scale->in_h_chr_pos, scale->in_v_chr_pos,
+                            desc->log2_chroma_w, desc->log2_chroma_h, i);
+            av_opt_set_int(s, "src_h_chr_pos", h_chr_pos, 0);
+            av_opt_set_int(s, "src_v_chr_pos", v_chr_pos, 0);
+
+            calc_chroma_pos(&h_chr_pos, &v_chr_pos, scale->out_chroma_loc,
+                            scale->out_h_chr_pos, scale->out_v_chr_pos,
+                            outdesc->log2_chroma_w, outdesc->log2_chroma_h, i);
+            av_opt_set_int(s, "dst_h_chr_pos", h_chr_pos, 0);
+            av_opt_set_int(s, "dst_v_chr_pos", v_chr_pos, 0);
 
             if ((ret = sws_init_context(s, NULL, NULL)) < 0)
                 return ret;
@@ -987,6 +1013,8 @@ scale:
     out->height = outlink->h;
     out->color_range = outlink->color_range;
     out->colorspace = outlink->colorspace;
+    if (scale->out_chroma_loc != AVCHROMA_LOC_UNSPECIFIED)
+        out->chroma_location = scale->out_chroma_loc;
 
     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);
@@ -1217,6 +1245,16 @@ static const AVOption scale_options[] = {
     { "mpeg",   NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_MPEG}, 0, 0, FLAGS, .unit = "range" },
     { "tv",     NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_MPEG}, 0, 0, FLAGS, .unit = "range" },
     { "pc",     NULL, 0, AV_OPT_TYPE_CONST, {.i64 = AVCOL_RANGE_JPEG}, 0, 0, FLAGS, .unit = "range" },
+    { "in_chroma_loc",  "set input chroma sample location",  OFFSET(in_chroma_loc),  AV_OPT_TYPE_INT, { .i64 = AVCHROMA_LOC_UNSPECIFIED }, 0, AVCHROMA_LOC_NB-1, .flags = FLAGS, .unit = "chroma_loc" },
+    { "out_chroma_loc", "set output chroma sample location", OFFSET(out_chroma_loc), AV_OPT_TYPE_INT, { .i64 = AVCHROMA_LOC_UNSPECIFIED }, 0, AVCHROMA_LOC_NB-1, .flags = FLAGS, .unit = "chroma_loc" },
+        {"auto",          NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_UNSPECIFIED}, 0, 0, FLAGS, .unit = "chroma_loc"},
+        {"unknown",       NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_UNSPECIFIED}, 0, 0, FLAGS, .unit = "chroma_loc"},
+        {"left",          NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_LEFT},        0, 0, FLAGS, .unit = "chroma_loc"},
+        {"center",        NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_CENTER},      0, 0, FLAGS, .unit = "chroma_loc"},
+        {"topleft",       NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_TOPLEFT},     0, 0, FLAGS, .unit = "chroma_loc"},
+        {"top",           NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_TOP},         0, 0, FLAGS, .unit = "chroma_loc"},
+        {"bottomleft",    NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_BOTTOMLEFT},  0, 0, FLAGS, .unit = "chroma_loc"},
+        {"bottom",        NULL, 0, AV_OPT_TYPE_CONST, {.i64=AVCHROMA_LOC_BOTTOM},      0, 0, FLAGS, .unit = "chroma_loc"},
     { "in_v_chr_pos",   "input vertical chroma position in luma grid/256"  ,   OFFSET(in_v_chr_pos),  AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
     { "in_h_chr_pos",   "input horizontal chroma position in luma grid/256",   OFFSET(in_h_chr_pos),  AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
     { "out_v_chr_pos",   "output vertical chroma position in luma grid/256"  , OFFSET(out_v_chr_pos), AV_OPT_TYPE_INT, { .i64 = -513}, -513, 512, FLAGS },
-- 
2.45.2

_______________________________________________
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 v2 6/8] avfilter/vf_scale: fix 4:1:0 interlaced chroma pos

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

The current logic hard-coded a check for v_sub == 1. We can extend this
logic slightly to cover the case of interlaced 4:1:0 (which has v_sub ==
2).

Here is a diagram explaining this scenario (with center-siting):

a   a   a   a   a   a   a   a

b   b   b   b   b   b   b   b
      X               X
a   a   a   a   a   a   a   a

b   b   b   b   b   b   b   b

a   a   a   a   a   a   a   a

b   b   b   b   b   b   b   b
      Y               Y
a   a   a   a   a   a   a   a

b   b   b   b   b   b   b   b

a = even luma rows
b = odd luma rows
X = even chroma sample
Y = odd chroma sample

In progressive mode, the chroma samples sit at (384, 384) respectively.

Relative to the 8x4 grid of even luma samples (a), the X sample sits at:
  h_chr_pos = 384
  v_chr_pos = 192

Relative to the 8x4 grid of odd luma samples (b), the Y sample sits at:
  h_chr_pos = 384
  v_chr_pos = 576

The new code calculates the correct values in all circumstances.
---
 libavfilter/vf_scale.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index b2c9d0b187..2d82141b70 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -642,9 +642,19 @@ static void calc_chroma_pos(int *h_pos_out, int *v_pos_out, int chroma_loc,
         v_pos = v_pos_override;
 
     /* Fix vertical chroma position for interlaced frames */
-    if (v_sub == 1 && index > 0) {
-        v_pos += 256 * (index == 2); /* offset by one luma row for odd rows */
-        v_pos >>= 1; /* double luma row distance */
+    if (v_sub && index > 0) {
+        /* When vertically subsampling, chroma samples are effectively only
+         * placed next to even rows. To access them from the odd field, we need
+         * to account for this shift by offsetting the distance of one luma row.
+         *
+         * For 4x vertical subsampling (v_sub == 2), they are only placed
+         * next to every *other* even row, so we need to shift by three luma
+         * rows to get to the chroma sample. */
+        if (index == 2)
+            v_pos += (256 << v_sub) - 256;
+
+        /* Luma row distance is doubled for fields, so halve offsets */
+        v_pos >>= 1;
     }
 
     /* Explicitly strip chroma offsets when not subsampling, because it
-- 
2.45.2

_______________________________________________
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 v2 7/8] avfilter/vf_zscale: remove unused fields

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

---
 libavfilter/vf_zscale.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 45f1bd25ce..dd0d1b7f46 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -108,11 +108,6 @@ typedef struct ZScaleContext {
     char *w_expr;               ///< width  expression string
     char *h_expr;               ///< height expression string
 
-    int out_h_chr_pos;
-    int out_v_chr_pos;
-    int in_h_chr_pos;
-    int in_v_chr_pos;
-
     int first_time;
     int force_original_aspect_ratio;
 
-- 
2.45.2

_______________________________________________
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 v2 8/8] fate/scalechroma: switch to standard chroma location

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Replace the manually specified chroma location by one using standard
notation, arbitrarily "bottomleft" as it is a less common path.

Required if we want to phase out the use of manual chroma locations.
---
 tests/fate/filter-video.mak       |  2 +-
 tests/ref/fate/filter-scalechroma | 50 +++++++++++++++----------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/tests/fate/filter-video.mak b/tests/fate/filter-video.mak
index 7f8dc3aa27..cffd67b1f8 100644
--- a/tests/fate/filter-video.mak
+++ b/tests/fate/filter-video.mak
@@ -433,7 +433,7 @@ fate-filter-scale2ref_keep_aspect: CMD = framemd5 -frames:v 5 -/filter_complex $
 
 FATE_FILTER_VSYNTH-$(call FILTERDEMDEC, SCALE, RAWVIDEO, RAWVIDEO) += fate-filter-scalechroma
 fate-filter-scalechroma: tests/data/vsynth1.yuv
-fate-filter-scalechroma: CMD = framecrc -flags bitexact -s 352x288 -pix_fmt yuv444p -i $(TARGET_PATH)/tests/data/vsynth1.yuv -pix_fmt yuv420p -sws_flags +bitexact -vf scale=out_v_chr_pos=33:out_h_chr_pos=151
+fate-filter-scalechroma: CMD = framecrc -flags bitexact -s 352x288 -pix_fmt yuv444p -i $(TARGET_PATH)/tests/data/vsynth1.yuv -pix_fmt yuv420p -sws_flags +bitexact -vf scale=out_chroma_loc=bottomleft
 
 FATE_FILTER_VSYNTH_VIDEO_FILTER-$(CONFIG_VFLIP_FILTER) += fate-filter-vflip
 fate-filter-vflip: CMD = video_filter "vflip"
diff --git a/tests/ref/fate/filter-scalechroma b/tests/ref/fate/filter-scalechroma
index 842769c2d5..dbe08600ab 100644
--- a/tests/ref/fate/filter-scalechroma
+++ b/tests/ref/fate/filter-scalechroma
@@ -3,28 +3,28 @@
 #codec_id 0: rawvideo
 #dimensions 0: 352x288
 #sar 0: 0/1
-0,          0,          0,        1,   152064, 0xdcab783a
-0,          1,          1,        1,   152064, 0x79c7f1f6
-0,          2,          2,        1,   152064, 0x3b810afb
-0,          3,          3,        1,   152064, 0x892aca1d
-0,          4,          4,        1,   152064, 0x52fdd093
-0,          5,          5,        1,   152064, 0xaa643426
-0,          6,          6,        1,   152064, 0x9ad020ed
-0,          7,          7,        1,   152064, 0x5c179057
-0,          8,          8,        1,   152064, 0xa56bf155
-0,          9,          9,        1,   152064, 0x61dcffca
-0,         10,         10,        1,   152064, 0x0d51a1d3
-0,         11,         11,        1,   152064, 0x652f9e8d
-0,         12,         12,        1,   152064, 0xdc0bb4d8
-0,         13,         13,        1,   152064, 0x561437cf
-0,         14,         14,        1,   152064, 0x69ef8e4f
-0,         15,         15,        1,   152064, 0xe7244350
-0,         16,         16,        1,   152064, 0xe65651cf
-0,         17,         17,        1,   152064, 0xfc9ff646
-0,         18,         18,        1,   152064, 0x6ae10bc4
-0,         19,         19,        1,   152064, 0xd3d1898a
-0,         20,         20,        1,   152064, 0xf3f8b139
-0,         21,         21,        1,   152064, 0x68c129be
-0,         22,         22,        1,   152064, 0xc3922593
-0,         23,         23,        1,   152064, 0x2b14d96e
-0,         24,         24,        1,   152064, 0xab119489
+0,          0,          0,        1,   152064, 0x77bb80f8
+0,          1,          1,        1,   152064, 0x3a21f6e8
+0,          2,          2,        1,   152064, 0xcc0907b0
+0,          3,          3,        1,   152064, 0xaa5cd87b
+0,          4,          4,        1,   152064, 0x410bd74d
+0,          5,          5,        1,   152064, 0x7a763b14
+0,          6,          6,        1,   152064, 0x3e4020d4
+0,          7,          7,        1,   152064, 0x46be8b5d
+0,          8,          8,        1,   152064, 0x8021f16c
+0,          9,          9,        1,   152064, 0x82ca033d
+0,         10,         10,        1,   152064, 0xa76ca6ca
+0,         11,         11,        1,   152064, 0x49019bb7
+0,         12,         12,        1,   152064, 0x3590adf5
+0,         13,         13,        1,   152064, 0xf21235dc
+0,         14,         14,        1,   152064, 0x6b5f93a9
+0,         15,         15,        1,   152064, 0x4b8c5137
+0,         16,         16,        1,   152064, 0x698b67cc
+0,         17,         17,        1,   152064, 0x3cebef62
+0,         18,         18,        1,   152064, 0x77770da1
+0,         19,         19,        1,   152064, 0xecdd84d4
+0,         20,         20,        1,   152064, 0x873fbc32
+0,         21,         21,        1,   152064, 0x55043c82
+0,         22,         22,        1,   152064, 0xee2b3669
+0,         23,         23,        1,   152064, 0xa64ae990
+0,         24,         24,        1,   152064, 0xb3499e12
-- 
2.45.2

_______________________________________________
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 v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Niklas Haas]

Changes since v1:

- Added commit relaxing the chr_v_pos value range in swscale to account
  for the existence of 4:1:1 / 4:1:0 content.
- Fix the chroma location calculation for 4:1:1 / 4:1:0, including in
  interlaced mode.
- Made the old (more granular) fields override the new one, rather than
  vice versa.
- Added more comments explaining everything that's going on
_______________________________________________
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 v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Andrew Sayers]

On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> Based on my best understanding of what they do, given the source code.
> ---
>  libswscale/swscale.h | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> index 9d4612aaf3..e22931cab4 100644
> --- a/libswscale/swscale.h
> +++ b/libswscale/swscale.h
> @@ -82,11 +82,35 @@ const char *swscale_license(void);
>  #define SWS_PRINT_INFO              0x1000
>  
>  //the following 3 flags are not completely implemented
> -//internal chrominance subsampling info
> +
> +/**
> + * Perform full chroma upsampling when converting to RGB as part of scaling.

Nitpick: "as part of scaling" seems redundant - can it be removed?

> + *
> + * For example, when converting 50x50 yuv420p to 100x100 rgba, setting this flag
> + * will scale the chroma plane from 25x25 to 100x100 (4:4:4), and then convert
> + * the 100x100 yuv444p image to rgba in the final output step.
> + *
> + * Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
> + * with a single chroma sample being re-used for both horizontally adjacent RGBA
> + * output pixels.

Nitpick: this would be more readable as "for both of the...".

Consider the following sentence:

    Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
    with a single chroma sample being re-used for both horizontally and vertically
    adjacent RGBA output pixels.

Using "both of the" would make it clear what "both" refers to before the reader
starts doing branch-prediction in their head.

Otherwise, LGTM (by which I mean it's clear, not that I know whether it's
correct).
_______________________________________________
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 v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Niklas Haas]

On Thu, 04 Jul 2024 16:24:24 +0100 Andrew Sayers <ffmpeg-devel@pileofstuff.org> wrote:
> On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote:
> > From: Niklas Haas <git@haasn.dev>
> > 
> > Based on my best understanding of what they do, given the source code.
> > ---
> >  libswscale/swscale.h | 28 ++++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> > index 9d4612aaf3..e22931cab4 100644
> > --- a/libswscale/swscale.h
> > +++ b/libswscale/swscale.h
> > @@ -82,11 +82,35 @@ const char *swscale_license(void);
> >  #define SWS_PRINT_INFO              0x1000
> >  
> >  //the following 3 flags are not completely implemented
> > -//internal chrominance subsampling info
> > +
> > +/**
> > + * Perform full chroma upsampling when converting to RGB as part of scaling.
> 
> Nitpick: "as part of scaling" seems redundant - can it be removed?

I wrote it this way because, afaict, this flag does not affect unscaled
special converters (yuv->rgba). But I can remove it if you still think
it's unnecessary.

> 
> > + *
> > + * For example, when converting 50x50 yuv420p to 100x100 rgba, setting this flag
> > + * will scale the chroma plane from 25x25 to 100x100 (4:4:4), and then convert
> > + * the 100x100 yuv444p image to rgba in the final output step.
> > + *
> > + * Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
> > + * with a single chroma sample being re-used for both horizontally adjacent RGBA
> > + * output pixels.
> 
> Nitpick: this would be more readable as "for both of the...".
> 
> Consider the following sentence:
> 
>     Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
>     with a single chroma sample being re-used for both horizontally and vertically
>     adjacent RGBA output pixels.
> 
> Using "both of the" would make it clear what "both" refers to before the reader
> starts doing branch-prediction in their head.

Fixed.

> 
> Otherwise, LGTM (by which I mean it's clear, not that I know whether it's
> correct).
> _______________________________________________
> 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 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 v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Niklas Haas]

On Thu, 04 Jul 2024 19:56:56 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> On Thu, 04 Jul 2024 16:24:24 +0100 Andrew Sayers <ffmpeg-devel@pileofstuff.org> wrote:
> > On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > > 
> > > Based on my best understanding of what they do, given the source code.
> > > ---
> > >  libswscale/swscale.h | 28 ++++++++++++++++++++++++++--
> > >  1 file changed, 26 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> > > index 9d4612aaf3..e22931cab4 100644
> > > --- a/libswscale/swscale.h
> > > +++ b/libswscale/swscale.h
> > > @@ -82,11 +82,35 @@ const char *swscale_license(void);
> > >  #define SWS_PRINT_INFO              0x1000
> > >  
> > >  //the following 3 flags are not completely implemented
> > > -//internal chrominance subsampling info
> > > +
> > > +/**
> > > + * Perform full chroma upsampling when converting to RGB as part of scaling.
> > 
> > Nitpick: "as part of scaling" seems redundant - can it be removed?
> 
> I wrote it this way because, afaict, this flag does not affect unscaled
> special converters (yuv->rgba). But I can remove it if you still think
> it's unnecessary.

How about: "Perform full chroma upsampling when upscaling to RGB"?

> 
> > 
> > > + *
> > > + * For example, when converting 50x50 yuv420p to 100x100 rgba, setting this flag
> > > + * will scale the chroma plane from 25x25 to 100x100 (4:4:4), and then convert
> > > + * the 100x100 yuv444p image to rgba in the final output step.
> > > + *
> > > + * Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
> > > + * with a single chroma sample being re-used for both horizontally adjacent RGBA
> > > + * output pixels.
> > 
> > Nitpick: this would be more readable as "for both of the...".
> > 
> > Consider the following sentence:
> > 
> >     Without this flag, the chroma plane is instead scaled to 50x100 (4:2:2),
> >     with a single chroma sample being re-used for both horizontally and vertically
> >     adjacent RGBA output pixels.
> > 
> > Using "both of the" would make it clear what "both" refers to before the reader
> > starts doing branch-prediction in their head.
> 
> Fixed.
> 
> > 
> > Otherwise, LGTM (by which I mean it's clear, not that I know whether it's
> > correct).
> > _______________________________________________
> > 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 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 v2 1/8] swscale: document SWS_FULL_CHR_H_* flags

[Andrew Sayers]

On Thu, Jul 04, 2024 at 07:59:10PM +0200, Niklas Haas wrote:
> On Thu, 04 Jul 2024 19:56:56 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> > On Thu, 04 Jul 2024 16:24:24 +0100 Andrew Sayers <ffmpeg-devel@pileofstuff.org> wrote:
> > > On Thu, Jul 04, 2024 at 04:30:57PM +0200, Niklas Haas wrote:
> > > > From: Niklas Haas <git@haasn.dev>
> > > > 
> > > > Based on my best understanding of what they do, given the source code.
> > > > ---
> > > >  libswscale/swscale.h | 28 ++++++++++++++++++++++++++--
> > > >  1 file changed, 26 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/libswscale/swscale.h b/libswscale/swscale.h
> > > > index 9d4612aaf3..e22931cab4 100644
> > > > --- a/libswscale/swscale.h
> > > > +++ b/libswscale/swscale.h
> > > > @@ -82,11 +82,35 @@ const char *swscale_license(void);
> > > >  #define SWS_PRINT_INFO              0x1000
> > > >  
> > > >  //the following 3 flags are not completely implemented
> > > > -//internal chrominance subsampling info
> > > > +
> > > > +/**
> > > > + * Perform full chroma upsampling when converting to RGB as part of scaling.
> > > 
> > > Nitpick: "as part of scaling" seems redundant - can it be removed?
> > 
> > I wrote it this way because, afaict, this flag does not affect unscaled
> > special converters (yuv->rgba). But I can remove it if you still think
> > it's unnecessary.
> 
> How about: "Perform full chroma upsampling when upscaling to RGB"?

Ah, I hadn't understood that distinction at all.  I'd recommend...

1. keep the original if this applies to both up- and down-scaling
2. use the second if it's just for upscaling
3. either way, add a line like this at the end of the section:

    Note: this flag is ignored by unscaled special converters.

I realise this patch is just documenting current behaviour, and I'm not saying
that behaviour is correct or incorrect, but it seems important and certainly
wasn't intuitive to me.  So it's worth mentioning a bit louder :) 
_______________________________________________
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".