liip / LiipImagineBundle

Symfony Bundle to assist in image manipulation using the imagine library
http://liip.ch
MIT License
1.66k stars 378 forks source link

[RFC] AVIF support through proper content negotiation #1314

Open fbourigault opened 3 years ago

fbourigault commented 3 years ago

As both Google Chrome and Imagick support AVIF, it would be nice to have this format supported.

If we look at how WebP support was added in this bundle (see https://github.com/liip/LiipImagineBundle/pull/1307), we can't add another if ($avifGenerate) { ... } through the codebase.

My proposal would be to replace the format scalar under the filter_sets configuration with an array which would configure each output format we want to offer through content negotiation. A special format original could be used to keep the source file format.

This would leverage https://github.com/willdurand/Negotiation to handle all the subtleties of https://tools.ietf.org/html/rfc7231!

This could also ease the use of browser side image format selection through the use of the <picture> HTML element.

Also, this would make this bundle almost ready for any future image format!

pbowyer commented 3 years ago

I would prefer to leave content negotiation to the server, rather than dealing with it in PHP. This is something I've been thinking about for a while as my current personal project is image-heavy. Here's what I would like to see:

Criteria

  1. LiipImagineBundle handles generating all formats asynchronously. It can take long enough to generate one sizeable JPEG; I don't want the first person loading an image to wait while multiple copies of the image are generated
    • With expensive transformations, an improvement for even the 1st image would be to deliver something cheap & quick for the first request(s) with a short cache expiry, and the processed image when ready
  2. Ability to have the final URL output from the start, skipping the /resolve/ step. When caching pages, it's a pain to have this and then later have to clear the cache just to get the direct link to the image
  3. Ability to handle content negotiation on the server. Here's examples for handling the negotiation for WebP, which assumes a naming convention of <<image.jpg>>.webp

Some may already be possible and I've missed it in the documentation/it's only visible in the LiipImagineBundle source code.

Others may be too different from the bundle's way of doing things - I personally would like an option to embed all settings in the URL Cloudinary-style (this makes responsive image sets and adding dynamic text to an image easier), as well as filters. But this appears a fundamental difference in approach to this bundle.

Image format support in the bundle or server-side (nginx/apache)?

With the naming convention used in (3) you could have a server-side script that runs regularly and generates WebP/AVIF from your existing images. The advantage of not doing this and moving it into LiipImagineBundle would be:

Keeping true to the bundle

The bundle fills an interesting spot in the market. It's not a stand-alone image transformation server handling delivery, caching and more; neither is it a "make a thumbnail" only simple system. It provides more than the latter without the complexity of the former, which is super-convenient and a sweet spot for many projects. I haven't turned up a "Why we designed the bundle this way" document but I'm sure it was thought about, given the bundle's creators. And so generating multiple sizes for responsive image embeds, and content negotiation and delivery may be added to be added because they are useful to many, but may also be better as a sibling bundle, which handles their special cases.

fbourigault commented 3 years ago

I would prefer to leave content negotiation to the server, rather than dealing with it in PHP.

Aren’t the server and PHP the same thing?

Ability to have the final URL output from the start, skipping the /resolve/ step. When caching pages, it's a pain to have this and then later have to clear the cache just to get the direct link to the image

I thought the same when first using this bundle. This would not work when using s3 resolver as the final URL will be served by something else but the server. Also, I didn’t test it, but is there anything blocking you caching generated images with a /resolve/ part in the url?

pbowyer commented 3 years ago

Aren’t the server and PHP the same thing?

They aren't for most people. The server would be nginx or Apache for example, designed and optimised for serving static assets. PHP would be the processing done behind the server and to which they forward specific requests.

I thought the same when first using this bundle. This would not work when using s3 resolver as the final URL will be served by something else but the server.

Very true.

Also, I didn’t test it, but is there anything blocking you caching generated images with a /resolve/ part in the url?

When I hit a /resolve/ URL it 301's to the image, so I think I would end up caching the redirect and then the image. Each request is then two requests to the server. AFAICT there's no way round that with the current system.

fbourigault commented 3 years ago

Ok, so the HTTP server :) Your comment looks clearer to me now :)

When I hit a /resolve/ URL it 301's to the image, so I think I would end up caching the redirect and then the image. Each request is then two requests to the server. AFAICT there's no way round that with the current system.

There is probably room for improvement to get this working.

peter-gribanov commented 3 years ago

If we look at how WebP support was added in this bundle (see #1307), we can't add another if ($avifGenerate) { ... } through the codebase.

Why do you think so? We can easily add another format and make AVIF more priority than WebP. This will certainly be a clutter and a <picture> tag is preferred.

peter-gribanov commented 3 years ago

When I hit a /resolve/ URL it 301's to the image, so I think I would end up caching the redirect and then the image. Each request is then two requests to the server. AFAICT there's no way round that with the current system.

We solved the problem by creating our own controller. This made it possible to bypass the call to the server for each request. WebP links are resolved in Nginx.

peter-gribanov commented 3 years ago

Not all drivers supported AVIF format. Need to take into account it in the implementation of a new format in this bundle. https://github.com/avalanche123/Imagine/issues/742

dbu commented 5 months ago

to summarize:

i added this to the 3.0 milestone to remind us we should challenge this topic