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: chroma downsampling ratios are restricted #2362

Open
gopherbot opened this issue Oct 12, 2011 · 25 comments
Open

image/jpeg: chroma downsampling ratios are restricted #2362

gopherbot opened this issue Oct 12, 2011 · 25 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@gopherbot
Copy link
Contributor

by laeuter:

What steps will reproduce the problem?
1. Try to decode my JPEG photos

What is the expected output?
They should be decodable

What do you see instead?
A good number is rejected with the UnsupportedError "chroma downsample ratio".
Inspection show that the (h,v) pairs of the components are (2,2), (1,1), (2,2).

Which compiler are you using (5g, 6g, 8g, gccgo)?
6g

Which operating system are you using?
MacOS X 10.7.1

Which revision are you using?  (hg identify)
32a5db196298 (release-branch.r60) release/release.r60.2

Please provide any additional information below.
I tried to recode the images by opening them in other programs, rotating and resaving
them, but in most cases the images remain undecodable.
@rsc
Copy link
Contributor

rsc commented Oct 12, 2011

Comment 1:

Owner changed to @nigeltao.

Status changed to Accepted.

@bradfitz
Copy link
Contributor

Comment 2:

It might help to attach such an image here for testing.

@bradfitz
Copy link
Contributor

Comment 3:

Example image from Martin (OP), who couldn't attach stuff here.

Attachments:

  1. 11100602s.JPG (5741959 bytes)

@rsc
Copy link
Contributor

rsc commented Dec 9, 2011

Comment 4:

Labels changed: added priority-later, removed priority-medium.

@mpl
Copy link
Contributor

mpl commented Feb 27, 2012

Comment 6:

On a related note, a substantial number of the pics in my collection (which I would like
to handle with camlistore) have a 4:4:0 downsampling ratio (hv = 0x12).
So it's not exactly the same error, but I think it's an issue pretty close to this one.
Do you want me to open another issue for that?
Attached is an example of such a pic.

Attachments:

  1. Bleau_082007-001.jpg (795669 bytes)

@nigeltao
Copy link
Contributor

Comment 7:

No need to open another issue. It sounds like it's the same bug.
It is a bug, it just isn't going to be fixed before Go 1.

@rsc
Copy link
Contributor

rsc commented Sep 12, 2012

Comment 8:

Labels changed: added go1.1.

@rsc
Copy link
Contributor

rsc commented Dec 10, 2012

Comment 9:

Labels changed: added size-l.

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 10:

[The time for maybe has passed.]

@rsc
Copy link
Contributor

rsc commented Mar 12, 2013

Comment 11:

Labels changed: added go1.1maybe, removed go1.1.

@vdobler
Copy link
Contributor

vdobler commented Mar 14, 2013

Comment 12:

The image really has different Cb and Cr sampling:
    $ identify -format "%[jpeg:sampling-factor]" 11100602s.JPG 
    2x2,1x1,2x2
(The image seems to be made with anabuilder, an old
freeware tool from 2006 based on Java 1.4.)
Allowing this without breaking Go 1 API seems complicated.
Currently
    type YCbCr struct {
    Y, Cb, Cr      []uint8
    YStride        int
    CStride        int
    SubsampleRatio YCbCrSubsampleRatio
    Rect           Rectangle
    }
allows only for same rate sampling of Cb and Cr. As this is
a) common and b) sensible it does not seem worth changing.
But such images are legal jpegs (in my understanding of
the spec).
It should be able to decode such an 2x2,1x1,2x2 image
into 2x2,1x1,1x1 (4:2:0) YCbCr by subsampling Cr.  This 
would keep the image dimensions but discard some chroma
information contained in the image.
(To keep the chroma information one _could_ return an
image.RGBA.)
Is this a) worth fixing and b) would downsampling Cr
be the right fix?

@robpike
Copy link
Contributor

robpike commented May 18, 2013

Comment 13:

Labels changed: added go1.2maybe, removed go1.1maybe.

@rsc
Copy link
Contributor

rsc commented Jul 30, 2013

Comment 14:

Labels changed: added feature.

@rmmh
Copy link
Contributor

rmmh commented Jul 30, 2013

Comment 15:

Would adding YCbCrSubsampleRatio421 in the appropriate places break the API?
An image that formerly gave an error on decoding would instead yield a YCbCr struct with
a new SubsampleRatio value, which client code that directly examines chroma subsampling
may not be designed to handle.
Isn't this a fix to a buggy library, which is allowed?

@nigeltao
Copy link
Contributor

Comment 16:

You could avoid discarding chroma information by decoding to 4:4:4 and duplicating one
of the chroma channels.
I think that it's do-able, without breaking backwards compatability, but it does add
more complexity to the code, and I'm not sure if it's worth the cost, given how
infrequent such images are. The image/jpeg package doesn't promise to be a complete JPEG
implementation. For example, it doesn't support 12-bit JPEG images, or arithmetic-coded
JPEG images.
I note that https://en.wikipedia.org/wiki/Chroma_subsampling#4:2:1 currently says that
'4:2:1' "is not expressible in J:a:b notation. '4:2:1' is a hangover from a previous
notational scheme, and very few software or hardware codecs use it."

@robpike
Copy link
Contributor

robpike commented Aug 7, 2013

Comment 17:

Won't happen for 1.2.

Labels changed: added go1.3maybe, removed go1.2maybe.

@robpike
Copy link
Contributor

robpike commented Aug 20, 2013

Comment 18:

Labels changed: removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 19:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 20:

Labels changed: removed feature.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 21:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 22:

Labels changed: added repo-main.

@nigeltao
Copy link
Contributor

The Bleau_082007-001.jpg image should work fine now with Go tip's image/jpeg.

The 11100602s.JPG image still doesn't work, but comments 12 and 16 are still valid.

@rsc rsc removed accepted labels Apr 14, 2015
@wader
Copy link

wader commented Apr 24, 2021

Hello, i'm interested working on this. I run into this kind of images recently (2x2,1x1,2x2 and 2x2,1x1,1x2 subsampling) from a big swedish media site.

After messing around a bit i think i ended with half-doing what @nigeltao suggested in comment 16 and it looks like this:

bla

Does not look too far away but some implementation guidance would be nice.

Example images:

https://static-cdn.sr.se/images/3052/9f978572-ed62-4f0f-a6b4-9e7506cb6c44.jpg?preset=256x256 https://static-cdn.sr.se/images/2519/4ed3bde6-46a3-469b-af8e-5c2bde6d1749.jpg?preset=256x256

@nigeltao
Copy link
Contributor

nigeltao commented May 2, 2021

Does not look too far away but some implementation guidance would be nice.

Unfortunately, I don't have any spare time to give implementation guidance. It'd have to be up to somebody else to volunteer.

@wader
Copy link

wader commented May 2, 2021

Does not look too far away but some implementation guidance would be nice.

Unfortunately, I don't have any spare time to give implementation guidance. It'd have to be up to somebody else to volunteer.

Understand that, thanks for answering

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

10 participants