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 #12740 (new)

Opened 5 years ago

Last modified 3 years ago

Bug: Miscellaneous OMETiffReader defects

Reported by: rleigh Owned by:
Priority: minor Milestone: OME-Files
Component: Bio-Formats Version: 5.1.0-m1
Keywords: OMETiffReader Cc: jburel
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: n.a.
Sprint: n.a.

Description

While doing the C++ conversion, I noticed a few small problems, mostly confined to initFile.

  • "i" and "s" are both used to refer to the current series in a loop. s is assigned from i in the loop but both are identical. Suggestion: Remove s, rename i to s. Even better, rename i to series so it's obvious what it is.
  • TiffData? processing has O(n!) complexity. Reduce to O(2n) by moving the plane clearing loop out of the TiffData? loop so it's two single passes only. In the C++ code this was done by changing exists from a boolean to an enum to differentiate between the states so we only clear unknown planes.
  • In "verify that all planes are available" loop, if the reader is null and the planes are reinitialised, there's no explicit break to exit the loop and prevent the process happening again. Shouldn't in practice but it would make the intent clearer.
  • At the end of initFile null series are removed. This breaks indexing when restoring dates, etc. This should be the last operation in initFile to avoid indexing errors.
  • [zct]OneIndexed? only cope with indexing starting at zero or one. The code can be trivially generalised to support arbitrary index starts, done in the C++ code, which will make it a bit more robust.
  • Several checks use data from IFD 0, even when they are doing a check for something in a completely different series, which could likely be using completely different values, so several things could break if using multi-series data where the series are not the same. To fix, it needs to use an IFD from the series in question.

Change History (6)

comment:1 Changed 5 years ago by mlinkert

  • Milestone changed from Unscheduled to 5.1.1

Moving to 5.1.1 for triage.

comment:2 Changed 5 years ago by jburel

  • Cc jburel added
  • Milestone changed from 5.1.1 to 5.1.3

Moving to 5.1.3 as part of the export as OME-Tiff effort.

comment:3 Changed 4 years ago by mlinkert

  • Milestone changed from 5.1.3 to 5.2.0

Moving to 5.2.0, since none of this appears to be urgent. If there are test cases that highlight any of these problems, that would certainly be helpful.

comment:4 Changed 4 years ago by jamoore

  • Milestone changed from 5.2.0 to B-F-5.2.0

Splitting due to milestone decoupling.

comment:5 Changed 4 years ago by mlinkert

  • Keywords OMETiffReader added
  • Owner mlinkert deleted

No ongoing work here. As noted above, if test cases are available (or can be created) that highlight where the C++ is reader is correct but the Java one is incorrect, that would be a good first step.

The only point I don't completely agree with is this one:

  • [zct]OneIndexed? only cope with indexing starting at zero or one. The code can be trivially generalised to support arbitrary index starts, done in the C++ code, which will make it a

bit more robust.

since the spec only allows zero-based indexing. Support for one-based indexing is a special case since that's a common mistake for other software writing OME-TIFFs, but unless the spec is being changed support for arbitrary indexing might be a little too lenient (but definitely tricky to draw the line for data that's technically invalid).

comment:6 Changed 3 years ago by sbesson

  • Milestone changed from B-F-5.2.0 to OME-Files
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.138362 sec.)

We're Hiring!