akash-akya / vix

Elixir extension for libvips
MIT License
171 stars 20 forks source link

Streaming large-ish images to AWS results in "Failed to write to target" #151

Closed kipcole9 closed 2 months ago

kipcole9 commented 5 months ago

Symptoms

Streaming images to AWS using ex_aws results in "Failed to write to target" exception which the size of the image reaches approx 100Mb.

Smaller images stream with no issue. Streaming to Stream.into(<<>>) works on larger images without issue.

To reproduce

I have created two test cases in Image in the image_to_target branch. The tests are in the big_stream_test.exs file. One case streams to a binary without issue, the other to AWS which then errors.

Unfortunately the AWS test case relies upon having Minio (or other) running and credentials set in text.exs.

Using the test image, calling Vix.Vips.Operation.resize(image, 7.839) will cause the streaming to AWS to fail. Resizing with 7.838 will not. Interestingly, the resulting file size with 7.838 produces a file that is 98.1Mb, which is why I suspect there may be a 100Mb issue somewhere that I can't find. But that may just be coincidence.

The exception is raised very early - no calls to the stream chunker are ever called. This correlates with the code in vips_image.c which raises the error:

  if (vips_image_write_to_target(image, suffix, target, NULL)) {
    error("Failed to create image from fd. error: %s", vips_error_buffer());
    vips_error_clear();
    ret = make_error(env, "Failed to write to target");
    goto exit;
  }

I appreciate that it's unclear if this is an ex_aws issue or a vix issue. I'm starting here simply because the source of the exception seems to be libvips.

akash-akya commented 5 months ago

@kipcole9 thanks for the detailed issue. I'll check

akash-akya commented 3 months ago

Hi @kipcole9, first, I am sorry for such a long gap 😅

I just checked and the libvips error is VipsJpeg: Maximum supported image dimension is 65500 pixels. It seems JPEG format has max dimension limit. In libvips that is 65500, in case of test we are crossing because 8356 x 7.839  = 65,502.68.

Narrowed down the issue to this, without aws & streaming

  test "jpeg write failure" do
    {:ok, image} = Vimage.new_from_file("./test/images/Hong-Kong-2015-07-1998.jpg")

    assert {:ok, _} =
             image
             |> Operation.resize!(7.839)
             |> Vimage.write_to_buffer(".jpeg")
  end

Btw second test in big_stream_test.exs is succeeding because stream is never forced/evaluated, stream is only built.

akash-akya commented 2 months ago

@kipcole9 ping! in case you missed earlier comment :)

kipcole9 commented 2 months ago

Ahhh, that all makes a great deal of sense. Perhaps I will add a check in image for pixel count when saving jpegs in order to make this more obvious.

I don't suppose there is a way to propagate the underlying errors returning by libvips in a vix? Assuming there is in fact an underlying error returned!

akash-akya commented 2 months ago

We can return the error but I avoided sofar because of two reasons -- libvips errors are confusing when streaming is involved. I am not completely sure about building a string dynamically from vips_error_buffer() it might contain dynamic content and might be large.