Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(202)

Issue 13779043: code review 13779043: go.image/tiff: encoder fix non LZW Predictor and Palett... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years ago by chai2010
Modified:
8 years ago
Reviewers:
nigeltao, bsiegert
CC:
bsiegert, nigeltao, gbulmer, golang-dev
Visibility:
Public.

Description

go.image/tiff: encoder fix non LZW Predictor and Paletted SamplesPerPixel and non RGB ExtraSamples. TIFF6.0 Spec: Page 31: ExtraSamples can only included in RGB mode image. Page 39, SamplesPerPixel is usually 1 for bilevel, grayscale, and palette-color images. Page 64, Currently Predictor field is used only with LZW. Fixes issue 6421.

Patch Set 1 #

Patch Set 2 : diff -r a6a97af39394 https://code.google.com/p/go.image #

Patch Set 3 : diff -r a6a97af39394 https://code.google.com/p/go.image #

Total comments: 5

Patch Set 4 : diff -r a6a97af39394 https://code.google.com/p/go.image #

Total comments: 1

Patch Set 5 : diff -r a6a97af39394 https://code.google.com/p/go.image #

Patch Set 6 : diff -r a6a97af39394 https://code.google.com/p/go.image #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -4 lines) Patch
M tiff/writer.go View 1 2 3 4 5 5 chunks +16 lines, -4 lines 3 comments Download

Messages

Total messages: 16
chai2010
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go.image
9 years ago (2013-09-19 09:16:37 UTC) #1
bsiegert
LGTM https://codereview.appspot.com/13779043/diff/5001/tiff/writer.go File tiff/writer.go (right): https://codereview.appspot.com/13779043/diff/5001/tiff/writer.go#newcode300 tiff/writer.go:300: // Currently this field is used only with ...
9 years ago (2013-09-19 13:57:33 UTC) #2
gbulmer
On 2013/09/19 09:16:37, chai2010 wrote: > Hello mailto:golang-dev@googlegroups.com, > > I'd like you to review ...
9 years ago (2013-09-19 14:15:04 UTC) #3
bsiegert
I'll try to investigate this tomorrow.
9 years ago (2013-09-19 14:54:38 UTC) #4
chai2010
On 2013/09/19 14:15:04, gbulmer wrote: > On 2013/09/19 09:16:37, chai2010 wrote: > > Hello mailto:golang-dev@googlegroups.com, ...
9 years ago (2013-09-20 00:44:35 UTC) #5
gbulmer
On 2013/09/20 00:44:35, chai2010 wrote: > On 2013/09/19 14:15:04, gbulmer wrote: > > On 2013/09/19 ...
9 years ago (2013-09-20 16:41:12 UTC) #6
chai2010
On 2013/09/20 16:41:12, gbulmer wrote: > On 2013/09/20 00:44:35, chai2010 wrote: > > On 2013/09/19 ...
9 years ago (2013-09-21 01:11:46 UTC) #7
gbulmer
On 2013/09/21 01:11:46, chai2010 wrote: > On 2013/09/20 16:41:12, gbulmer wrote: > > On 2013/09/20 ...
9 years ago (2013-09-21 20:08:18 UTC) #8
gbulmer
The defect in bw-packbits.tiff video-001-gray-16bit.tiff video-001-gray.tiff video-001-paletted.tiff which produce 1/2 x 1/2 resolution, repeated images ...
9 years ago (2013-09-21 20:41:14 UTC) #9
chai2010
Rollback to PatchSet 4. PTAL https://codereview.appspot.com/13779043/diff/5001/tiff/writer.go File tiff/writer.go (right): https://codereview.appspot.com/13779043/diff/5001/tiff/writer.go#newcode300 tiff/writer.go:300: // Currently this field ...
9 years ago (2013-09-21 23:58:38 UTC) #10
nigeltao
LGTM. I'll make the trivial fixes and submit. https://codereview.appspot.com/13779043/diff/20001/tiff/writer.go File tiff/writer.go (right): https://codereview.appspot.com/13779043/diff/20001/tiff/writer.go#newcode300 tiff/writer.go:300: // ...
9 years ago (2013-09-22 01:10:15 UTC) #11
nigeltao
*** Submitted as https://code.google.com/p/go/source/detail?r=14beb99f9689&repo=image *** go.image/tiff: encoder fix non LZW Predictor and Paletted SamplesPerPixel and ...
9 years ago (2013-09-22 01:24:42 UTC) #12
gbulmer
I'd like to add a small observation. I believe the solution causes tiff.Options.Predictor to have ...
9 years ago (2013-09-22 16:58:41 UTC) #13
nigeltao
On Mon, Sep 23, 2013 at 2:58 AM, G Bulmer <gbulmer@gmail.com> wrote: > Of course, ...
9 years ago (2013-09-22 23:37:26 UTC) #14
bsiegert
On Mon, Sep 23, 2013 at 1:37 AM, Nigel Tao <nigeltao@golang.org> wrote: > As Chaishushan ...
9 years ago (2013-09-23 08:43:10 UTC) #15
chai2010
9 years ago (2013-09-24 12:48:10 UTC) #16
On 2013/09/23 08:43:10, bsiegert wrote:
> On Mon, Sep 23, 2013 at 1:37 AM, Nigel Tao <mailto:nigeltao@golang.org> wrote:
> > As Chaishushan said in https://codereview.appspot.com/13779043
> > "encoder fix non LZW Predictor", page 64 of
> > http://partners.adobe.com/public/developer/en/tiff/TIFF6.pdf says that
> > Predictor "is used only with LZW (Compression=5) encoding".
> >
> > Benny, you wrote https://codereview.appspot.com/6567077 "support
> > writing files with differencing predictor" and
> > https://codereview.appspot.com/6866051 "support for writing compressed
> > images. Only Deflate compression is supported for now". Any thoughts?
> 
> Yes. If I interpret the spec right, the section on Deflate compression
> was added later, without updating the main text. So I think that it
> should be possible to use a differencing predictor (at least) with
> Deflate compression as well. I will send a CL.
> 
> Shushan: If you want a difficult TIFF problem to tackle, you could try
> fixing the decoding of LZW-compressed TIFF images.
> 
> --Benny.

Benny,
I don't understanding TIFF LZW clearly.
I need more time to read the spec.

ChaiShushan
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b