Warning: Can't synchronize with repository "(default)" (/home/git/ome.git does not appear to be a Git repository.). Look in the Trac log for more information.
Notice: In order to edit this ticket you need to be either: a Product Owner, The owner or the reporter of the ticket, or, in case of a Task not yet assigned, a team_member"

Task #6728 (closed)

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

Bug : Corruption in dv file

Reported by: saloynton Owned by: saloynton
Priority: critical Milestone: OMERO-4.4
Component: General Version: n.a.
Keywords: n.a. Cc: saloynton, cxallan
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: 0.0d
Sprint: 2012-02-14 (8)

Description (last modified by saloynton)

Following the testing in #6718 the following .dv file
(test_images_good/dv/CFPNEAT01_R3D.dv) the top z section has a tile corrupted.

See screenshot attached.

Attachments (2)

corrupt dv image at z section 29.png (329.5 KB) - added by saloynton 10 years ago.
working image z 29.png (525.0 KB) - added by saloynton 10 years ago.

Download all attachments as: .zip

Change History (22)

Changed 10 years ago by saloynton

comment:1 Changed 10 years ago by saloynton

  • Description modified (diff)

comment:2 Changed 10 years ago by mlinkert

I wasn't able to reproduce this testing just now (local server, recent client build, imported via Insight). Can you confirm that this occurred with the most recent OMERO-trunk QA build (#105 right now)? Was the file imported with Insight or the importer?

The images do all look correct in Fiji, so it's probably not a problem with the Deltavision reader itself.

comment:3 Changed 10 years ago by saloynton

It was with the insight importer and was with the #105 the qa test build on friday.

comment:4 Changed 10 years ago by cxallan

The context here Melissa is a bit whacky. In order to test a wide variety of pixel types for the completion of #6718 Scott and I enabled FS lite for Deltavision. The fact that it's a single file format (at least as far as the binary data is concerned) made it an ideal candidate. This is also how I found the bug with the missing log files and the parsing of the file paths (#6726). Deltavision is not enabled as FS lite in the default installs.

With that context in mind, the bug appears to be related to the compression, writing or reading of that particular tile.

comment:5 Changed 10 years ago by mlinkert

I can reproduce this now by adding the Deltavision reader to the list of FS lite formats. However, I'm having a really difficult time finding the cause of the problem.

Turning off BigTIFF pyramid generation does not solve the problem. Turning off pyramid compression does solve the problem, which definitely points to a problem in the JPEG-2000 codec and/or service.

If I convert the offending DV file to plain TIFF and import the TIFF, then the problem does not appear with the imported TIFF. If I add OME-TIFF to the list of FS lite formats, convert the DV file to OME-TIFF, and import the OME-TIFF then the problem does appear with the imported OME-TIFF. The only real difference between the two files is that the OME-TIFF has Z=29 C=2 T=1 and the plain TIFF has Z=1 C=1 T=58.

Adding some debugging to TiffSaver?, what happens is that the correct tile is passed in to TiffSaver?, but the tile changes during the process of compression (TiffSaver?.java:342). That is, the MD5 of the original tile does not match the MD5 of decompressed tile, but only for the upper-right tile in plane Z=28 C=0.


At a bit of a loss as to what to try next. I'll revisit later this evening.

comment:6 Changed 10 years ago by mlinkert

Digging a little deeper, the tiles mismatch because the tile that is written somehow has all of the pixel values incremented by 6 (?!). I still don't know why though. It does suggest that this is not an issue of a buffer not being cleared.

comment:7 Changed 10 years ago by mlinkert

This is almost certainly a bug in JAI. For this particular file at least, changing the number of resolution levels that are saved does actually affect the pixel data that is written. It is possible to work around the problem by setting the default number of resolution levels to 4 (instead of 5):

diff --git a/components/bio-formats/src/loci/formats/services/JAIIIOServiceImpl.
index 35e0501..3d9248d 100644
--- a/components/bio-formats/src/loci/formats/services/JAIIIOServiceImpl.java
+++ b/components/bio-formats/src/loci/formats/services/JAIIIOServiceImpl.java
@@ -116,6 +116,9 @@ public class JAIIIOServiceImpl extends AbstractService
       param.setNumDecompositionLevels(
           options.numDecompositionLevels.intValue());
     }
+    else {
+      param.setNumDecompositionLevels(4);
+    }
     writer.write(null, iioImage, param);
     ios.close();
   }

I can apply that patch, but it doesn't feel like a proper fix, and does affect anything that writes JPEG-2000 data (so should really be tested quite a bit before releasing). I personally think it would be safer to wait until after release, and then figure out why the number of resolution levels matters in this case - other opinions would be welcome though.

comment:8 Changed 10 years ago by jburel

  • Milestone changed from OMERO-Beta4.3.2 to OME-5.0

comment:9 Changed 10 years ago by mlinkert

  • Sprint set to 2012-01-17 (6)

comment:10 Changed 10 years ago by mlinkert

  • Remaining Time set to 2

comment:11 Changed 10 years ago by mlinkert

  • Status changed from new to accepted

comment:12 Changed 10 years ago by jburel

  • Sprint changed from 2012-01-17 (6) to 2012-01-31 (7)

Referencing ticket #7716 has changed sprint.

comment:13 Changed 10 years ago by mlinkert

  • Priority changed from minor to critical

Changing resolution levels as noted above is not the correct solution, as it does not work for many of the other files tested.

This is actually symptomatic of a much larger problem: lossless JPEG-2000 encoding simply is not reliably lossless, at least for 16-bit data. There is now a minimal test case that illustrates this:

https://github.com/melissalinkert/bioformats/commit/9552a658ff954230f80006ac145d88420df77d06

Basically, writing a single 16-bit pixel causes the pixel value to be changed. Writing an 8-bit pixel with the exact same value is just fine.

So, changing the bug to 'critical', because this is really really not good. I think almost of the big images that we encounter are 8-bit, so fortunately it's not likely that someone has run into this in the wild.

Any and all ideas would be welcome here - especially if you think I'm just missing something completely obvious.

comment:14 Changed 10 years ago by mlinkert

Finally think this is sorted, or at least on the right track. PR for Bio-Formats changes:

https://github.com/openmicroscopy/bioformats/pull/23

...that's a two-line fix in JJ2000 plus tests for lossless JPEG-2000 compression.

PR for OMERO (which just has updated Bio-Formats JARs from above PR):

https://github.com/openmicroscopy/openmicroscopy/pull/75

Chris and/or Scott, when you have a chance could you please have a look at importing test_images_good/dv/CFPNEAT01_R3D.dv into a server built against the branch from the OMERO PR (making sure that DeltavisionReader? is added to the list of FS lite readers)? I tested it locally and it seems to work, but given the amount of trial and error here it's completely possible that I've missed something.

comment:15 Changed 10 years ago by jmoore

  • Sprint changed from 2012-01-31 (7) to 2012-02-14 (8)

Moved from sprint 2012-01-31 (7)

comment:16 Changed 10 years ago by mlinkert

  • Owner mlinkert-x deleted

Unassigning, since there isn't really much more that I can do here at the moment. All that really needs to happen is for someone to double-check importing test_images_good/dv/CFPNEAT01_R3D.dv against a server/Insight built from the branch here : https://github.com/openmicroscopy/openmicroscopy/pull/75, with DeltavisionReader? added to the list of FS lite readers.

comment:17 Changed 10 years ago by saloynton

  • Owner set to saloynton
  • Remaining Time changed from 2 to 0.1

I am sorry Melissa, I thought I had left a comment on this ticket. The file was viewable when I imported it. I had just wanted to double check I was working was with the correct build. I will confirm this and make sure to close the ticket.

comment:18 Changed 10 years ago by saloynton

Tested on OMERO-merge-blue-66 screenshot attached with the expected view ticket closed.

Changed 10 years ago by saloynton

comment:19 Changed 10 years ago by saloynton

  • Remaining Time changed from 0.1 to 0
  • Resolution set to fixed
  • Status changed from accepted to closed

comment:20 Changed 10 years ago by Melissa Linkert <melissa@…>

(In [aa931d67acd8150aafb544894072e4ba23195ada/ome.git] on branch develop) Update Bio-Formats JARs to 0875d18/bioformats.git

See #6728.

Note: See TracTickets for help on using tickets. You may also have a look at Agilo extensions to the ticket.

1.3.13-PRO © 2008-2011 Agilo Software all rights reserved (this page was served in: 0.82937 sec.)

We're Hiring!