From: "Clément Bœsch" <u@pkh.me> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> Subject: Re: [FFmpeg-devel] [PATCH] Revert "avfilter/vf_palette(gen|use): support palettes with alpha" Date: Mon, 31 Oct 2022 11:57:16 +0100 Message-ID: <Y1+qDCehfrC+0Fzy@ssq0.pkh.me> (raw) In-Reply-To: <DM8P223MB03650C1AC2A4FA38C2FF561FBA379@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM> On Mon, Oct 31, 2022 at 01:43:11AM +0000, Soft Works wrote: [...] > > > > The patch I had submitted doesn't change the previous behavior > > > > without the use_alpha parameter. > > > > Yes I noticed, but unfortunately I'm reworking the color distance to > > work > > in perceptual color space, and the way that alpha is mixed up in the > > equation just doesn't make any sense at all and prevents me from > > doing > > these changes. > > If you want to implement a new color distance algorithm, it should > be either a new filter or a new (switchable) mode for the existing > filter. Why? > Photoshop has these different modes as well and it would > surely be useful, but I don't think it should be replacing the > existing behavior. > There is no point in keeping a ton of complexity exposed as user options for something implementation specific. We offer no guarantee over how the quantization is expected to run. > When it turns out that the use_alpha implementation doesn't fit > with your new color distance calculation and you add it as > an additional mode, then it would be fine IMO when the filter > errors in case it would be attempted to use that mode in > combination with use_alpha. IMO The use_alpha option shouldn't exist in the first place, it should be the default behaviour because honoring the alpha is the correct thing to do. That's not what the option is currently doing though. > > > Do you think it might make sense to put more weight on the > > > alpha value by tripling it? So it would be weighted equally to the > > > RGB value? > > > > You cannot mix alpha with colors at all, they are separate domains > > and you > > need to treat them as such. > > What's interesting is that I've followed the same (simplified) > way for adding a use_alpha option to vf_elbg and it provides excellent > results without treating alpha separately. I don't know how the filter works and what it's supposed to do, but if it's indeed using the same approach as the palette ones, it cannot work. > > From paletteuse perspective what you need to do is first choose the > > colors > > in the palette that match exactly the alpha (or at least the closest > > if > > and only there is no exact match). Then within that set, and only > > within > > that one, you'd pick the closest color. > > > > From palettegen perspective, you need to split the colors in > > different > > transparency domain (a first dimensional quantization), then quantize > > the > > colors in each quantized alpha dimension. And when you have all your > > quantized palettes for each level of alpha, you find an algorithm to > > reduce the number of transparency dimensions or the number of colors > > per > > dimension to make it fit inside a single palette. But you can't just > > do > > the alpha and the colors at the same time, it cannot work, whatever > > weights you choose. > > I would be curious to see how well that would work, especially > in cases when the target palettes have just a few number of colors. > You have to make a call between whether you want to preserve the transparency or the color while constructing the palette, but when choosing a color you must absolutely not choose a color with a different transparency, you must pick amongst the closest alpha, with a particular attention to extreme alphas: an opaque colors must stay opaque, and fully transparent one as well: - rounding a color with 43% alpha into 50% alpha is acceptable - rounding a color with 100% alpha into a 99% alpha is not acceptable in any way because you're starting to make transparent areas that weren't - rounding a color with 0% alpha into a 1% alpha is not acceptable because some areas of the images are not starting to blend into an area that was supposedly non-existent > But to return to the proposal of removal: If everything from ffmpeg > would be removed which is not perfect, then it would be lacking > quite a number of features I suppose :-) We're not talking about perfection, we're talking about files with artifacts. It's almost as bad as a corrupted file, because if used in a pipeline where transparency matters, you're going to get a completely broken output. > In the same way, one could say that palettegen/-use should be removed > because its results are wrong and colors are randomly mixed and > misplaced while the vf_elbg filter does it right. > When you look at the result under the heading > > "Paletteuse/gen Regular (to 8-bit non-alpha palette; only single > transparent color)" > https://gist.github.com/softworkz/deef5c2a43d3d629c3e17f9e21544a8f?permalink_comment_id=3905155#gistcomment-3905155 > > Even without the alpha, many color pixels appear to be wrong and > random like for example the light purple pixels on the darker purple > at the bottom of the "O". That's not much different from irregularities > in the alpha channel you've shown (https://imgur.com/a/50YyRGV). > So, I agree to that it's not perfect, but the whole filter is > not perfect (vf_elbg is close-to). Do we remove the filter because > it's not perfect? The colors are indeed wrong because it has some *incorrect* computations (working in sRGB space, and the MSE is wrong at least) and these should be corrected (working on it). I'm also adding improvements on the quality by working in the perceptual space. > As mentioned above, if you want to add an additional mode for > calculating the color distance, it's fine when it doesn't > work with use_alpha IMO. That's not how it's going to work, sorry; I'm not going to increase complexity and maintenance effort for no gain. Implementing a correct support for the alpha will likely involve a revert of that commit anyway. Note that if I was active at the time this patch was submitted I would certainly have rejected it in this state. So it's my fault, but I'm working on fixing it. -- Clément B. _______________________________________________ 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".
next prev parent reply other threads:[~2022-10-31 10:57 UTC|newest] Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-10-30 17:58 Clément Bœsch 2022-10-30 21:19 ` Soft Works 2022-10-30 21:30 ` Clément Bœsch 2022-10-30 21:37 ` Clément Bœsch 2022-10-30 21:41 ` Soft Works 2022-10-30 22:55 ` Soft Works 2022-10-31 0:29 ` Clément Bœsch 2022-10-31 1:43 ` Soft Works 2022-10-31 10:57 ` Clément Bœsch [this message] 2022-10-31 11:58 ` Paul B Mahol 2022-10-31 12:41 ` Clément Bœsch 2022-10-31 15:11 ` Soft Works 2022-10-31 18:51 ` Clément Bœsch 2022-10-31 20:41 ` Soft Works 2022-10-31 21:58 ` Michael Niedermayer 2022-10-31 23:34 ` Soft Works 2022-11-01 10:18 ` Clément Bœsch 2022-10-31 2:09 ` Soft Works
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=Y1+qDCehfrC+0Fzy@ssq0.pkh.me \ --to=u@pkh.me \ --cc=ffmpeg-devel@ffmpeg.org \ /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