[FFmpeg-devel] [PATCH v2 12/18] fftools/ffmpeg_enc: don't ignore user selected chroma location

[Niklas Haas]

From: Niklas Haas <git at haasn.dev>

This code always ignored the user-provided enc_ctx->chroma_sample_location
in favor of the location tagged on the frame. This leads to a very (IMHO)
unexpected outcome where -chroma_sample_location works differently from the
related options like -colorspace and -color_range, the latter of which
override the frame properties as a consequence of being configured on the
filter graph output.

The discrepancy comes from the fact that the chroma sample location does not
itself participate in filter graph negotiation.

Solve the situation by only overriding the enc_ctx option if it was left
unspecified by the user, and otherwise printing a warning if the requested
parameter does not match the frame parameter.
---
 fftools/ffmpeg_enc.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index a46af4dce1..4568c15073 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -266,11 +266,26 @@ int enc_open(void *opaque, const AVFrame *frame)
             enc_ctx->bits_per_raw_sample = FFMIN(fd->bits_per_raw_sample,
                                                  av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth);
 
+        /**
+         * The video color properties should always be in sync with the user-
+         * requested values, since we forward them to the filter graph.
+         */
         enc_ctx->color_range            = frame->color_range;
         enc_ctx->color_primaries        = frame->color_primaries;
         enc_ctx->color_trc              = frame->color_trc;
         enc_ctx->colorspace             = frame->colorspace;
-        enc_ctx->chroma_sample_location = frame->chroma_location;
+
+        /* Video properties which are not part of filter graph negotiation */
+        if (enc_ctx->chroma_sample_location == AVCHROMA_LOC_UNSPECIFIED) {
+            enc_ctx->chroma_sample_location = frame->chroma_location;
+        } else if (enc_ctx->chroma_sample_location != frame->chroma_location &&
+                   frame->chroma_location != AVCHROMA_LOC_UNSPECIFIED) {
+            av_log(e, AV_LOG_WARNING,
+                   "Requested chroma sample location '%s' does not match the "
+                   "frame tagged sample location '%s'; result may be incorrect.\n",
+                   av_chroma_location_name(enc_ctx->chroma_sample_location),
+                   av_chroma_location_name(frame->chroma_location));
+        }
 
         if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) ||
             (frame->flags & AV_FRAME_FLAG_INTERLACED)
-- 
2.50.1