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

image/jpeg: unsupported downsampling error messages swapped for luma and chroma #5569

Closed
gopherbot opened this issue May 27, 2013 · 10 comments
Closed

Comments

@gopherbot
Copy link

by Shifter1:

http://golang.org/src/pkg/image/jpeg/reader.go#L177


The error messages "luma downsample ratio"  and "chroma downsample
ratio"  appear to be swapped.
@minux
Copy link
Member

minux commented May 27, 2013

Comment 1:

i == 0 means Y component (luma), so the order is actually correct.

Status changed to WorkingAsIntended.

@gopherbot
Copy link
Author

Comment 2 by Shifter1:

I created a number of sample images with no luma subsampling (only chroma subsampling)
and made a test program to exhibit the problem.  Here is the output:
$ go run jpeg.go *.jpg
2013/05/27 14:51:42 1x1.jpg: sucess
2013/05/27 14:51:42 1x2.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 1x3.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 1x4.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 2x1.jpg: sucess
2013/05/27 14:51:42 2x2.jpg: sucess
2013/05/27 14:51:42 2x3.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 2x4.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 3x1.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 3x2.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 4x1.jpg: unsupported JPEG feature: luma downsample ratio
2013/05/27 14:51:42 4x2.jpg: unsupported JPEG feature: luma downsample ratio
The error message should be the that the chroma sub sampling is wrong, not the luma.

Attachments:

  1. images.zip (51800 bytes)
  2. jpeg.go (364 bytes)

@davecheney
Copy link
Contributor

Comment 3:

Status changed to New.

@minux
Copy link
Member

minux commented May 27, 2013

Comment 4:

$ for i in *.jpg; do echo $i; identify -verbose "$i" | grep "jpeg:sampling-factor"; done
1x1.jpg
    jpeg:sampling-factor: 1x1,1x1,1x1
1x2.jpg
    jpeg:sampling-factor: 1x2,1x1,1x1
1x3.jpg
    jpeg:sampling-factor: 1x3,1x1,1x1
1x4.jpg
    jpeg:sampling-factor: 1x4,1x1,1x1
2x1.jpg
    jpeg:sampling-factor: 2x1,1x1,1x1
2x2.jpg
    jpeg:sampling-factor: 2x2,1x1,1x1
2x3.jpg
    jpeg:sampling-factor: 2x3,1x1,1x1
2x4.jpg
    jpeg:sampling-factor: 2x4,1x1,1x1
3x1.jpg
    jpeg:sampling-factor: 3x1,1x1,1x1
3x2.jpg
    jpeg:sampling-factor: 3x2,1x1,1x1
4x1.jpg
    jpeg:sampling-factor: 4x1,1x1,1x1
4x2.jpg
    jpeg:sampling-factor: 4x2,1x1,1x1

@minux
Copy link
Member

minux commented May 27, 2013

Comment 5:

technically, all your samples vary only by luma sampling factor, so the error message is
actually correct.
(the fact that it's not intuitive is because the sampling factor is intertwined)
the correct fix perhaps is to drop "luma" or "chroma" entirely in the messages.

@gopherbot
Copy link
Author

Comment 6 by Shifter1:

The images were generated with the "convert" program with the "-sampling-factor"  flags,
which (maybe incorrectly) only affect chroma sampling: 
http://www.imagemagick.org/script/command-line-options.php#sampling-factor
Secondly, using an alternate tool ( http://regex.info/exif.cgi ) on some of the images
lists them as only doing chroma subsampling as well.  For example, 2x2.jpg is listed as
4:2:0 which is only chroma subsampling.
I still believe this is a bug.

@minux
Copy link
Member

minux commented May 28, 2013

Comment 7:

we can change the first error message to simply say "unsupported JPEG feature:
luma/chroma downsample ratio".
and leave the 2nd one as is.
the three sub-sampling ratio in SOF frames must be considered together to give more
meaningful error
message.
another viewpoint is that normally we only sub-sample the chroma signal, so in fact we
can change both
error message to say chroma downsampling.
what do you think?

@nigeltao
Copy link
Contributor

nigeltao commented Jun 4, 2013

Comment 8:

I guess it all depends on your perspective. For example, 4:2:0 chroma subsampling means
that in a 16x16 Minimum Coded Unit, there are 2x2 luma blocks (each block is 8x8), and
1x1 chroma block for each of U and V (the 8x8 block is stretched over the 16x16 MCU).
The overall scheme is called "chroma subsampling" but the interesting number is the 2x2,
which is associated with luma.
Anyway, the easy fix is to change all the messages to say "luma/chroma downsample
ratio". I'll do that.

@dsymonds
Copy link
Contributor

dsymonds commented Jun 4, 2013

Comment 9:

Owner changed to @nigeltao.

@nigeltao
Copy link
Contributor

nigeltao commented Jun 5, 2013

Comment 10:

This issue was closed by revision 427bec6.

Status changed to Fixed.

@golang golang locked and limited conversation to collaborators Jun 24, 2016
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants