fdintino / pillow-avif-plugin

A pillow plugin that adds avif support via libavif
BSD 2-Clause "Simplified" License
90 stars 13 forks source link

Encoding with `Image.save(..., "AVIF")` fails #23

Closed Sir-Photch closed 7 months ago

Sir-Photch commented 1 year ago

Hi,

on Ubuntu 20.04.1 LTS, I am getting the following error when trying to save an image with avif format:

>>> image.save("abc.avif")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/christoph/src/repos/encoder-trainer-py/.venv/lib/python3.8/site-packages/PIL/Image.py", line 2432, in save
    save_handler(self, fp, filename)
  File "/home/christoph/src/repos/encoder-trainer-py/.venv/lib/python3.8/site-packages/pillow_avif/AvifImagePlugin.py", line 235, in _save
    enc.add(
RuntimeError: Failed to encode image: Encoding of color planes failed

Since 20.04 does not provide libavif / libaom, I compiled them from source, versions being:

Are these the right versions / do I need other libraries as well?

fdintino commented 1 year ago

Are you able to encode the image using the avifenc command line utility?

Sir-Photch commented 1 year ago

Actually I wasn't, since libavif does not specify you have to enable additional apps like avifenc during manual build.

I configured with options -DAVIF_BUILD_APPS=ON -DAVIF_CODEC_AOM=ON.

Anyway, I can now, but I still get the same error.

una-dinosauria commented 8 months ago

I ran into the same issue on CentOS 8 and 9. I am very confused by the error. It is being thrown by this check in libavif, which ensures that avifEncoderAddImage is only called once which -- and I've checked! -- the C wrapper in this repo does.

That particular encoder->data->singleImage variable should be initialized to 0 = AVIF_FALSE on creation, and has no business being set to anything else elsewhere in the code, so I am wondering whether there is some nasty memory overwrite going on.

Like @Sir-Photch, I was able to compile libavif from scratch and I'm using the standalone avifenc without issues.

Unfortunately, github only provides ubuntu runners for linux so I cannot create a workflow that will reproduce the issue, but I'll try to reproduce this on a minimal docker image.

fdintino commented 8 months ago

What platform, architecture, and python version are you running? If there isn't a binary wheel available I can add one to CI.

una-dinosauria commented 8 months ago

CentOS 8 and 9. AMD x86_64, python 3.10. The wheel already exists for this combo. I tried to reproduce the issue on a docker image with CentOS but wasn't quite successful, sorry. I'll keep trying and let you know if I do achieve a successful repro.

yit-b commented 8 months ago

Hi @fdintino. I'm also having the same issue, but was able to mitigate it with a small hack to _avif.c. Here's the patch:

diff --git a/src/pillow_avif/_avif.c b/src/pillow_avif/_avif.c
index 3b2970d..f1d6f8c 100644
--- a/src/pillow_avif/_avif.c
+++ b/src/pillow_avif/_avif.c
@@ -448,7 +448,7 @@ AvifEncoderNew(PyObject *self_, PyObject *args) {
             init_max_threads();
         }
-        encoder->maxThreads = max_threads;
+        encoder->maxThreads = 1;
 #if AVIF_VERSION >= 1000000
         if (enc_options.qmin != -1 && enc_options.qmax != -1) {
             encoder->minQuantizer = enc_options.qmin;

This was a wild guess, and I don't know why this fixes it, but I'm assuming it's some race condition or some memory is getting corrupted. I know this isn't really a proper fix, but maybe it will point you or somebody with more intimate knowledge of the library in the right direction?

System specs

Centos 9

The following packages are installed via Conda:

python=3.8
libavif=0.11.1
aom=3.5.0

I also tried this with libavif=1.0.3 and aom=3.6.0

The bug is present when installing pillow-avif-plugin either from the remote whl hosted on PyPi or installing locally from a git clone with pip install .

Below follows my path of reasoning:

I Googled "Encoding of color planes failed". Found this in libavif source: https://github.com/AOMediaCodec/libavif/blob/main/src/avif.c#L84

Searched all occurrences of AVIF_RESULT_ENCODE_COLOR_FAILED in the libavif source. There are only two spots, but this is the most interesting: https://github.com/AOMediaCodec/libavif/blob/6cc154be9d0d0b50b12cfab0d65049de1ef8407e/src/write.c#L1407

Note the comment:

if (encoder->data->singleImage) {
    // The previous call to avifEncoderAddImage() set AVIF_ADD_IMAGE_FLAG_SINGLE.
    // avifEncoderAddImage() cannot be called again for this encode.
    return AVIF_RESULT_ENCODE_COLOR_FAILED;
}

This leads me to believe that the avifEncoderAddImage() method is getting erroneously called by the pillow plugin more than once.

Let's check in on the pillow plugin side. Right near the top we see a method called init_max_threads... uh oh https://github.com/fdintino/pillow-avif-plugin/blob/main/src/pillow_avif/_avif.c#L52

Looks like the encoder takes a maxThreads param: https://github.com/fdintino/pillow-avif-plugin/blob/main/src/pillow_avif/_avif.c#L451

Explanation of the param here: https://github.com/AOMediaCodec/libavif/blob/6cc154be9d0d0b50b12cfab0d65049de1ef8407e/include/avif/avif.h#L808

Let's just hard code this and see what happens: encoder->maxThreads = 1;

First let's repro:

repro.py:

from PIL import Image
import pillow_avif
import numpy as np
from pathlib import Path

def main():
    arr = np.ones((128, 128, 3), dtype=np.uint8)
    img_orig = Image.fromarray(arr)
    out_path = Path("/tmp") / f"foobar.avif"
    print(f"Saving to {out_path}")
    img_orig.save(out_path)

if __name__ == "__main__":
    main()

git clone https://github.com/fdintino/pillow-avif-plugin.git cd ~/pillow-avif-plugin pip install --no-index . python repro.py

Saving to /tmp/foobar.avif
Traceback (most recent call last):
  File "/home/yitb/foo.py", line 14, in <module>
    main()
  File "/home/yitb/foo.py", line 11, in main
    img_orig.save(out_path)
  File "[...]/lib/python3.8/site-packages/PIL/Image.py", line 2431, in save
    save_handler(self, fp, filename)
  File "[...]/lib/python3.8/site-packages/pillow_avif/AvifImagePlugin.py", line 244, in _save
    enc.add(
RuntimeError: Failed to encode image: Encoding of color planes failed

Also most of the tests fail: pytest tests

tests/test_file_avif.py ...F......F.FF..FFFFF.FFF..FFF..FFF.F.F..FF......s.sssss...........FF..FFFF.. [100%]

Let's try the patch:

git apply my_patch.patch pip install --no-index . python repro.py

Saving to /tmp/foobar.avif

No error and the output image is well-formed.

After applying the patch, the test suite also looks good (save for one test failing because of some deprecated pytest feature)

pytest tests

tests/test_file_avif.py ..........F..................................s...s.........s................. [100%]
yit-b commented 8 months ago

Ok! I found the cause. If you try to set encoder->numThreads to a value greater than 64, encoding fails. By default, pillow-avif-plugin sets numThreads to the number of cores on your machine. In my case, I have 96, so it will always fail.

Propose setting a limit of 64 returned from init_max_threads: https://github.com/fdintino/pillow-avif-plugin/blob/main/src/pillow_avif/_avif.c#L52

yit-b commented 8 months ago

See this check in libavif source: https://github.com/AOMediaCodec/libavif/blob/3ead1f3e99112871af0c52d4dc57a144d662b8cd/src/codec_aom.c#L750

una-dinosauria commented 8 months ago

Great sleuthing @yit-b! Do you plan to make a PR on _avif.c?

yit-b commented 8 months ago

@fdintino Please see PR #41