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 via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Cc: Niklas Haas <ffmpeg@haasn.xyz>
Subject: Re: [FFmpeg-devel] [PATCH] lavfi: protection against premultiplied alpha (was: The patch series about premultiplied alpha)
Date: Wed, 20 Aug 2025 23:43:18 +0200
Message-ID: <20250820234318.GB486061@haasn.xyz> (raw)
In-Reply-To: <aJ3r7KXGa46jJNqx@phare.normalesup.org>

[-- Attachment #1: Type: text/plain, Size: 4453 bytes --]

On Thu, 14 Aug 2025 16:00:12 +0200 Nicolas George <george@nsup.org> wrote:
> Niklas Haas (HE12025-08-13):
> > Updated https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20031 with full negotiation
> > for the alpha mode. I also went ahead and fixed the drawutils filters to support
> > premultiplied alpha, since it was low-hanging fruit.
>
> Nice. I must confess, I was not paying enough attention when the new
> fields were added to the negotiation. I will need a little time to
> re-familiarize myself with the code.
>
> The new patch series is not arrived on the mailing-list, apparently the
> link is random. I tried reviewing on the web thing, it is way too
> inconvenient, especially for jumping between related parts of the code.
> Please send it to the mailing-list.

You can get a raw list of patches here:

https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/20031.patch

I've done you the courtesy of attaching it to this e-mail.

>
> > Now all the filters you were concerned about should be protected from receiving
> > premultiplied alpha, at filter graph configuration time.
>
> Uh… no, absolutely not. Filters using drawutils were only an obvious
> example, with drawbox a very easy test case, but the list I gave was
> never supposed to be exhaustive, it was only meant to explain the issue
> to you. There are a few other obvious examples and certainly several
> less obvious cases.
>
> Furthermore, it would not be reasonable to demand that the authors of
> future filters implement it or even think about it.

This argument works both ways.

filter authors to remember to enable support
for premultiplied alpha, even if they don't even touch the alpha plane?

>
> Which is why the default, i.e. the alpha format list set by the famework
> if the filter did not set it, must not include premultiplied.
>
> Note that it is not a sudden random requirement from me. We did it the
> same way when we added support for unknown channel layouts while keeping
> API compatibility with the forks: filters that had not been explicitly
> vetted for support were assumed to not support them. Now like before, it
> is the only reasonable way to do things.

This is not at all what I see in the commit log, which clearly states that
whether to use a whitelist or a blacklist was a matter of disagreement between
FFmpeg and libav, and upon merging the two, the FFmpeg behavior (accept all
by default) was chosen as the new default. (See commit 7ceb9e6b11)

Unlike the alpha mode, which merely affects the colorimetric interpretation
of pixel values, the channel layout actually affects the layout in memory, so
it would even be *reasonable* to be conservative when it comes to them, and
still, the default was chosen to be permissive rather than restrictive.

So if anything, the precedent, and the existing code, supports my proposal.

Lastly, I also think that turning this option into a whitelist is
unnecessarily restrictive, not to mention more invasive/bloated, because
practically all filters are compatible with both alpha types - a blacklist is
much shorter and more practical than a whitelist.

So I respectfully disagree with your preference, unless you can provide a more
sound technical argument for why this property is exceptional enough to
deserve special handling. I personally don't find it more exceptional than,
say, chroma location or color range.

---

Anyways, to avoid dragging out this discussion longer (and in light of the
TC's lack of interest), and given that we already missed the 8.0 target for
this change, I suggest we just merge this series for now; then, you can
submit a follow-up series wherein you make your proposed change of changing
the default. (Or, assuming the TC responds with a clear preference, I would
be happy to change the default behavior at that point in time)

As it stands, there is no actual regression or new bug introduced by this
series, so it is already an improvement over the status quo - delaying it
further is just letting perfect be the enemy of good, and is clearly getting
us nowhere.

With that in mind, I will push this series at the end of the week; assuming
I get another LGTM for the latest changes.

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

[-- Attachment #2: 20031.patch --]
[-- Type: application/mbox, Size: 374314 bytes --]

[-- Attachment #3: 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".

  reply	other threads:[~2025-08-20 21:43 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-02 15:18 [FFmpeg-devel] The patch series about premultiplied alpha Nicolas George
2025-08-02 18:03 ` Nicolas George
2025-08-03 10:42   ` Niklas Haas
2025-08-03 10:50     ` Niklas Haas
2025-08-03 14:35     ` Nicolas George
2025-08-03 15:49       ` Nicolas George
2025-08-03 18:15         ` [FFmpeg-devel] [PATCH] lavfi: protection against premultiplied alpha (was: The patch series about premultiplied alpha) Nicolas George
2025-08-03 20:04           ` Michael Niedermayer
2025-08-03 20:50             ` Kacper Michajlow
2025-08-05  8:51           ` Niklas Haas
2025-08-05  8:58             ` Robert Nagy
2025-08-05  9:05             ` Nicolas George
2025-08-05  9:16               ` Nicolas George
2025-08-05  9:25                 ` Robert Nagy
2025-08-05  9:31                   ` Nicolas George
2025-08-05  9:31                     ` Robert Nagy
2025-08-11  9:18           ` Niklas Haas
2025-08-11  9:19             ` Niklas Haas
2025-08-11  9:32             ` Nicolas George
2025-08-13 14:25               ` Niklas Haas
2025-08-14 14:00                 ` Nicolas George
2025-08-20 21:43                   ` Niklas Haas via ffmpeg-devel [this message]
2025-08-20 21:44                     ` Niklas Haas via ffmpeg-devel

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=20250820234318.GB486061@haasn.xyz \
    --to=ffmpeg-devel@ffmpeg.org \
    --cc=ffmpeg@haasn.xyz \
    /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