Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Niklas Haas <ffmpeg@haasn.xyz>
To: ffmpeg-devel@ffmpeg.org
Cc: Niklas Haas <git@haasn.dev>
Subject: [FFmpeg-devel] [PATCH v2 12/18] fftools/ffmpeg_enc: don't ignore user selected chroma location
Date: Wed, 23 Jul 2025 15:47:15 +0200
Message-ID: <20250723135626.1390296-13-ffmpeg@haasn.xyz> (raw)
In-Reply-To: <20250723135626.1390296-1-ffmpeg@haasn.xyz>

From: Niklas Haas <git@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

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

  parent reply	other threads:[~2025-07-23 13:58 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-23 13:47 [FFmpeg-devel] (no subject) Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 01/18] avutil/frame: add AVFrame.alpha_mode Niklas Haas
2025-07-23 16:00   ` Kacper Michajlow
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 02/18] fftools/ffprobe: add support for AVFrame.alpha_mode Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 03/18] avfilter/vf_showinfo: print alpha mode when relevant Niklas Haas
2025-07-23 16:11   ` Kacper Michajlow
2025-07-24 11:19     ` Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 04/18] avcodec/avcodec: add AVCodecContext.alpha_mode Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 05/18] avcodec/encode: enforce alpha mode compatibility at encode time Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 06/18] avcodec/png: set correct alpha mode Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 07/18] avcodec/exr: " Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 08/18] avcodec/libjxl: " Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 09/18] avcodec/libjxlenc: also attach extra channel info Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 10/18] avcodec/jpegxl: parse and signal correct alpha mode Niklas Haas
2025-07-23 16:19   ` Kacper Michajlow
2025-07-24 11:13     ` Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 11/18] avfilter/vf_scale: don't ignore incoming chroma location Niklas Haas
2025-07-24 11:16   ` Niklas Haas
2025-07-23 13:47 ` Niklas Haas [this message]
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 13/18] fftools/ffmpeg_enc: forward frame alpha mode to encoder Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 14/18] avfilter/vf_premultiply: tag correct alpha mode on result Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 15/18] avfilter/vf_alphamerge: tag correct alpha mode Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 16/18] avfilter/vf_overlay: respect alpha mode tagging by default Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 17/18] avfilter/vf_setparams: add alpha_mode parameter Niklas Haas
2025-07-23 13:47 ` [FFmpeg-devel] [PATCH v2 18/18] avfilter/vf_libplacebo: add an alpha_mode setting Niklas Haas
2025-07-23 14:11 ` [FFmpeg-devel] Again pre-multiplied alpha Nicolas George
2025-07-23 15:18   ` Niklas Haas
2025-07-23 17:02     ` Nicolas George
2025-07-24 11:11       ` Niklas Haas
2025-07-24 14:59         ` Nicolas George
2025-07-24 20:58           ` Niklas Haas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250723135626.1390296-13-ffmpeg@haasn.xyz \
    --to=ffmpeg@haasn.xyz \
    --cc=ffmpeg-devel@ffmpeg.org \
    --cc=git@haasn.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git