Skip to content

Conversation

@lablancas
Copy link

Filestack engineering team recommended that we put the output transformation last in the URL.

This does seem to provide a drastic reduction in image delivery time.

Please review and confirm that changing this order does not have some other negative impacts.

const expected = {
img: {
src: 'https://cdn.filestackcontent.com/output=format:webp/quality=value:5/sepia=tone:70/seW1thvcR1aQBfOCF8bX',
src: 'https://cdn.filestackcontent.com/quality=value:5/sepia=tone:70/output=format:webp/seW1thvcR1aQBfOCF8bX',
Copy link

@gr00by gr00by Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately it will have a negative impact in this case scenario. If the input file was an image that doesn't support the quality setting, let's say a png image, the quality task wouldn't be applied, as the image would still be a png at this point. In the old example the image would be converted to webp before running the quality task, making it possible to apply the quality setting. Notice the difference in image quality in the below examples:
https://cdn.filestackcontent.com/quality=v:1/output=f:webp/LJNf24eDS1O7GsbpaEMs
https://cdn.filestackcontent.com/output=f:webp/quality=v:1/LJNf24eDS1O7GsbpaEMs

I'm not familiar with the adaptive package, but it looks to me that it might require a greater refactoring in order to make it possible to apply the output task as the last task in chain (which makes perfect sense) with being backwards compatible at the same time.

@pcholuj @sebastian-wec @promanski

Copy link
Author

@lablancas lablancas Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gr00by87 I could update the sort function so that quality is sorted after output, but I am not sure if sepia (or other transformations) should also be sorted after the output transform?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants