Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] AVIF support through proper content negotiation #1314

Open
fbourigault opened this issue Dec 14, 2020 · 8 comments
Open

[RFC] AVIF support through proper content negotiation #1314

fbourigault opened this issue Dec 14, 2020 · 8 comments
Labels
Level: New Feature 🆕 This item involves the introduction of new functionality.

Comments

@fbourigault
Copy link
Contributor

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 #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
Copy link

pbowyer commented Dec 23, 2020

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:

  • Works on all servers with just the PHP libraries installed
  • More control, e.g. tweak AVIF quality per filter, rather than global settings

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
Copy link
Contributor Author

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
Copy link

pbowyer commented Dec 24, 2020

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

Not all drivers supported AVIF format. Need to take into account it in the implementation of a new format in this bundle.
php-imagine/Imagine#742

@dbu dbu added the Level: New Feature 🆕 This item involves the introduction of new functionality. label Oct 6, 2021
@hockdudu hockdudu mentioned this issue Jun 5, 2024
@dbu dbu added this to the 3.0.0 milestone Jun 6, 2024
@dbu
Copy link
Member

dbu commented Jun 6, 2024

to summarize:

  • we need to improve the configuration we introduced to optionally support webp
  • we want to explore if we can leverage the nginx/apache/... web server to avoid having to hit the /resolve symfony entpoint for each image request (where the controller would be able to negotiate the correct format for this client) (some brainstorming also in avif support #1585)
  • we want to check how cache warmup for this scenario should work

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

@dbu dbu modified the milestones: 3.0.0, 3.0 Refactoring Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Level: New Feature 🆕 This item involves the introduction of new functionality.
Projects
None yet
Development

No branches or pull requests

4 participants