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

Scaling WebP images fail in synchronously mode. #53

Closed
jbluemink opened this issue May 6, 2019 · 6 comments · Fixed by #59
Closed

Scaling WebP images fail in synchronously mode. #53

jbluemink opened this issue May 6, 2019 · 6 comments · Fixed by #59

Comments

@jbluemink
Copy link

jbluemink commented May 6, 2019

With the latest source and Dianoga.Strategy.GetMediaStreamSync.config
scaling WebP images fail In synchronously mode.
If the first request to a media with scaling parameters with a WebP Accept header you get the original uploaded image back, not scaled.

In the log file you see somethings like this:
8764 13:51:08 INFO Dianoga: optimized /Images/Doner.jpeg [original size] (final size: 12534 bytes) - saved 18461 bytes / 59.56 %. Optimized in 157ms.
8764 13:51:08 ERROR Could not run the 'getMediaStream' pipeline for '/sitecore/media library/Images/Doner'. Original media data will be used.
Exception: System.ArgumentException
Message: Parameter is not valid.
Source: System.Drawing
at System.Drawing.Bitmap..ctor(Stream stream)
at Sitecore.Resources.Media.SaveColorProfileProcessor.Process(GetMediaStreamPipelineArgs args)
at (Object , Object )
at Sitecore.Pipelines.CorePipeline.Run(PipelineArgs args)
at Sitecore.Pipelines.DefaultCorePipelineManager.Run(String pipelineName, PipelineArgs args, String pipelineDomain, Boolean failIfNotExists)
at Sitecore.Pipelines.DefaultCorePipelineManager.Run(String pipelineName, PipelineArgs args, String pipelineDomain)
at Sitecore.Resources.Media.Media.GetStreamFromPipeline(MediaOptions options, Boolean& canBeCached)

Reproduction.

  • Install Dianoga from latest source,
  • enable WebP.config
  • enable GetMediaStreamSync mode
  • clear media cache
  • do a valid request with accept WebP header to a jpg media in Sitecore with scaling parameters for example /-/media/Images/Doner.ashx?h=50&w=70&hash=7C0CD542B2833FC370CE13DD90C54FAE
    Result error in log, original uploaded media instead of scaled WebP Image.

When you first hit to the image (with the scaling parameters) is a non WebP request, then it can works okay.

@alex-nanai
Copy link

alex-nanai commented Sep 16, 2019

Looks like optimization pipeline is executed 2 times:

  1. Get original file
  2. Get scaled image based on 1st

Second step gets webp instead of png (or jpeg) so it can't scale.
Somehow if full size png is already in the MediaCache then 2nd step will work fine.
It seems like correct behavior would be to not apply webp conversion during first part.

Unfortunately I could not translate hypothesis into code and make a proper PR so far, so I've applied a workaround to not apply webp optimization on scaling requests - extend ParseAcceptHeaders and bind new class through Dianoga.WebP.config.

public class ParseMediaRequest : ParseAcceptHeaders
{
	public override bool BrowserSupportWebP
	{
		get
		{
			bool imageScalingRequest =
				HttpContext.Current != null &&
				(HttpContext.Current.Request.QueryString.Get("w") != null ||
				 HttpContext.Current.Request.QueryString.Get("h") != null);

			// do not apply webp for scaling requests until this one is fixed 
			// https://github.com/kamsar/Dianoga/issues/53
			return !imageScalingRequest && base.BrowserSupportWebP;
		}
		set { base.BrowserSupportWebP = value; }
	}
}  

@markgibbons25
Copy link
Collaborator

I can also reproduce this issue. The link above from HageMaster makes no difference as that code is in sitecore 9.0+ now from what I can see (even 8.2 is the same for this discussion).

@HageMaster3108
Copy link

I can't confirm that @markgibbons25.
My vanilla 9.1.1 instance still aborts in SaveColorProfileProcessor because of the color-profile related exception.

@markgibbons25
Copy link
Collaborator

Yes, I can reproduce it with or without the SaveColorProfileProcessor pipeline code from here I'm just saying that code seems to be in Sitecore.Kernel since at least 8.2.5, I would guess from the date that answer was posted they were on Sitecore 8.1 or an early version of 8.2

markgibbons25 pushed a commit to markgibbons25/Dianoga that referenced this issue Feb 16, 2020
…ibwebp to 1.1.0. Change to use cwebp defaults. Add better support for webp files in media library.
markgibbons25 pushed a commit to markgibbons25/Dianoga that referenced this issue Feb 16, 2020
…ibwebp to 1.1.0. Change to use cwebp defaults. Add better support for webp files in media library.
@markgibbons25
Copy link
Collaborator

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 a pull request may close this issue.

4 participants