Feature request: Image servlet should support progressive JPEGs

[ghost]

By default, the transformed images are baseline encoded. As a consequence, if the browser painting isn't delayed until the images have been loaded, the visitor sees how the images are loaded from top to bottom. Often, this is rather ugly. Therefore, there should be an option to enable progressive encoding.

The difference is explained here: http://adavu.com/what-is-baseline-jpeg-and-progressive-jpeg/

The problem is that the named image transformation servlet is based on com.day.image.Layer which does not allow to set the progressive mode. Improving the Layer API would be possible, but then it could only be used with the latest AEM version. To avoid this, I would suggest to handle certain image formats outside of Layer#write in the NamedTransformImageServlet itself. I am aware that this is not a clean solution, but I do not think there is another option (except for simply ignoring this request).

What do you think?

By the way, the PNG encoding should also be improved. Often, PNG images become much bigger when they are transformed. This is also something which could be handled in the image servlet. Lastly, I think webp support should be added there as well.

[davidjgonzalez]

@joelrich .. So if we did this -- and forgive me, its been a long time since i looked at that code

my main concern is the ability to still use the existing transformer set which IIRC takes the Layer objs.

For the quality transformer, we had to do something quasi-similar where the transform invocation was pulled out of the flow in a rather unnatural way (remember not liking breaking from the existing paradigm). I wonder if that pattern could be followed, and this mechanism could be injected in a similar way?

EOD i think backwards compatibility is paramount, both from a supported transformers perspective, and the Transfomer interface (since ppl sometimes build custom transformers on projects and we dont want to break that code)

[justinedelson]

@joelrich @davidjgonzalez FWIW, I think this needs to be addressed in AEM proper.

[ghost]

@davidjgonzalez this behaviour could be enabled similar to how the quality can be configured. The existing transformers would not have to be touched. However, instead of just writing

layer.write(mimeType, quality, response.getOutputStream());

we would have to do something like (simplified)

        if (progressive && "imgae/jpeg".equals(mimeType)) {
            // custom jpeg write code
        } else {
            layer.write(mimeType, quality, response.getOutputStream());
        }

in NamedTransformImageServlet.

@justinedelson I agree that it would be better to improve this in AEM itself. However, then it will probably never be possible to use this feature with an older AEM version (I use 6.1 at the moment). Moreover, you would need separate code for compatibility with older AEM versions. Lastly, all the other limitations which I mentioned are unlikely to be fixed in AEM anytime soon either.

[davidjgonzalez]

@justinedelson agree, ideally this should be addressed in AEM proper, but agree with @joelrich wrt to timing -- unless you know something we don't.

[justinedelson]

@davidjgonzalez I don't know anything.

[ghost]

@justinedelson do you completely disagree with the approach above? Do you have a better idea to fix this problem (for AEM 6.1)?

[justinedelson]

@joelrich I don't "completely" disagree. I'd say I mildly disagree :) and I worry about scope creep. But I'm not standing in anyone's way.

[ghost]

@justinedelson in this case I am creating a pull request for the proposed enhancement.

[justinedelson]

@joelrich just curious - how are you handling webp? I only saw JNI-based libraries for this.

[davidjgonzalez]

it would be nice to see if we could use this opportunity to clean up some of the impl structure of the servlet. it went through a few evolutions that muddied the impl a big (quality transform, chained transforms, etc.)

It might be nice to take the opportunity to build it pluggable to allow for other image apis (ex. there have been several requests for animated gif support; TBH not sure what that would require)

[ghost]

@justinedelson I do not intend to add webp support right now. This was just an example for something which is missing in Layer and should be implemented in my opinion.

I have implemented the progressive functionality here: https://github.com/joelrich/acs-aem-commons/commit/325cacd79b94c93e1a37b8991d52d73f4ed86146

Please have a look there and let me know what you think. If this has a chance to be merged, then I will write tests, add documentation and create a pull request.

@davidjgonzalez I do not have the time right now to help with a refactoring. However, I think it would be great if we could improve this servlet further. If you have concrete plans, I might ask to get some time for this.

To support animated gifs, you would definitely also have to use encoding outside of Layer#write.

[justinedelson]

@joelrich I think https://github.com/joelrich/acs-aem-commons/commit/325cacd79b94c93e1a37b8991d52d73f4ed86146 is fine. If more of these "non-Layer" cases are added, I could see restructuring this code a bit, but since it is non-API, that doesn't need to be addressed now.

[ghost]

@justinedelson @davidjgonzalez I have created a pull request here: https://github.com/Adobe-Consulting-Services/acs-aem-commons/pull/826 If you think this is fine, would it be possible to merge #826 and #821 and release the package today/tomorrow?

[justinedelson]

PR merged

[ghost]

@justinedelson thank you! For when is the release planned?

[justinedelson]

I think we'll do the release on Monday.