Removed alpha_premultiplied

[radarhere]

Suggestion for python-pillow#5201

To my simple way of thinking, alpha_premultiplied would cause the saved image to no longer be accurate to the Pillow image being saved. So I suggest removing it.

[fdintino]

There is a predictable amount of drift with premultiplied alpha, but in certain circumstances it can result in more efficient compression. How is that different than other types of lossy compression?

[fdintino]

I don't think this is right. The PIL mode "RGBa" should actually affect alphaPremultiplied on avifRGBImage, not avifImage. For avifRGBImage, setting rgb.alphaPremultiplied = AVIF_TRUE signals that the source RGB image has premultiplied alpha. Whereas for avifImage, setting frame->alphaPremultiplied = AVIF_TRUE causes the YUV alpha to be premultiplied. Depending on their relative values, avifRGBImageToYUV will either do nothing if they are both false or both true, or else premultiply or un-premultiply the alpha depending on which is true. And the decoder will return un-premultiplied RGBA (unless we were to set rgb.alphaPremultiplied to AVIF_TRUE in _decoder_get_frame).

Part of the confusion might be due to the test, which uses such a low alpha value that all precision is lost. If you were to use a higher alpha, you could see that the behavior as it has been changed here is incorrect.

The folllowing test ought to succeed, but fails with the change in this pull request:

    def test_source_rgb_alpha_premultiplied(self, tmp_path: Path) -> None:
        temp_file = str(tmp_path / "temp.avif")
        color = (85, 170, 254, 128)
        im = Image.new("RGBA", (1, 1), color)
        im.convert("RGBa").save(temp_file)

        with Image.open(temp_file) as reloaded:
            assert_image_similar(im, reloaded, 5)

Though if you were to change the lines here

    } else {
        rgb.format = AVIF_RGB_FORMAT_RGBA;
#if AVIF_VERSION >= 90000
        if (strcmp(mode, "RGBa") == 0) {
            frame->alphaPremultiplied = AVIF_TRUE;
        }
#endif

to instead be

    } else {
        rgb.format = AVIF_RGB_FORMAT_RGBA;
#if AVIF_VERSION >= 90000
        if (strcmp(mode, "RGBa") == 0) {
            rgb.alphaPremultiplied = AVIF_TRUE;
        }
#endif

it would succeed. Though I'm not sure it's worth the added complexity compared to first converting to RGBA with pillow, since source RGBa images are probably fairly rare.

A similar test for the alpha_premultiplied save parameter would look something like:

    @pytest.mark.parametrize("alpha_premultiplied", [False, True])
    def test_alpha_premultiplied_roundtrip(self, tmp_path: Path, alpha_premultiplied: bool) -> None:
        temp_file = str(tmp_path / "temp.avif")
        color = (85, 170, 254, 128)
        im = Image.new("RGBA", (1, 1), color)
        im.save(temp_file, alpha_premultiplied=alpha_premultiplied)

        with Image.open(temp_file) as reloaded:
            assert_image_similar(im, reloaded, 5)

Though that doesn't actually test that libavif is doing any premultiplying. To verify that, your test with the reloaded getpixel for (200, 200, 200, 1) makes sense.

[radarhere]

Yeah, what I was trying to demonstrate in code was that Pillow has an RGBa mode, and that if users want to save an image with premultiplied alpha, then it would simpler for them to save an RGBa image, rather than using a format specific flag like alpha_premultiplied.

So taking a step back for a moment, do you agree with that idea in theory?

[fdintino]

I think if the underlying pixel data in the AVIF image was RGBa then it would make sense, because you would then presumably get RGBa back from the decoder. But because it is converting to YUV, and because alphaPremultiply is specified separately for the RGB and YUV image, with the former only existing so that libavif can do alpha multiplying and unmultiplying if necessary, I'm not so sure. To the extent that the RGBa mode is used in pillow, I imagine it is mostly for image compositing operations. But for an AVIF image it really only serves to get more efficient compression on images with detailed alpha planes, in a way that doesn't compromise perceptual quality. I think it wouldn't be obvious that the intended way to enable (what amounts to) a lossy compression flag would be to convert to a different mode, particularly when that mode is different from what is actually stored in the image.

[radarhere]

Ok. It's not the default behaviour of the plugin, so I'm not going to fight too hard for this. I've closed this, and left a comment on the main PR.