Fix 411 and 410 Subsampling ratios in ycc.go

nfnt / resize

Pure golang image resizing

ISC License

3.02k stars 321 forks source link

Fix 411 and 410 Subsampling ratios in ycc.go #38

Closed carl-mastrangelo closed 8 years ago

carl-mastrangelo commented 9 years ago

Resolves #37

The subsampling ratios were added in Go 1.5. Previously, trying to open an image with one of these ratios would fail before they could be resized. Since adding them, it is now possible to attempt to resize them, resulting in a runtime panic.

I see that the the travis tests are failing, due to the new ratios not being in the tested Go versions. I see two viable options:

Hard code the sampling ratio numbers into the switch, and just put a comment saying what they stand for.
Update Travis to use go 1.5

charlievieth commented 9 years ago

@carl-mastrangelo Damn, you're fast! I wish I saw this before submitting #39. Also, thanks - I somehow missed that image was updated in the go1.5 release.

carl-mastrangelo commented 9 years ago

@charlievieth You probably know the library better than me; I'm not too keen on who submits, just as long as it is fixed.

The trouble is that the default behavior will break when new subsampling ratios are added. Wikipedia mentions a few others, but also that they are rarely used. It probably won't be that much of a problem in the future. Still, it might be better to future proof by having an explicit case for 444 and have the default fallback to resampling if it isn't a known subsample ratio. That way an error won't have to be returned.

Thoughts?

nfnt commented 8 years ago

Thanks for the PRs of both of you! My primary concerns right now are backwards compatibility and avoiding unnecessary code duplications. I'll stick with @charlievieth 's PR, because it's using build constraints to provide backwards compatibility. Therefore I'll close this PR and concentrate on the other one.