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

Opened 9 years ago

Last modified 8 years ago

Bug: OMERO clients report BIT pixel type as uint8

Reported by: rleigh Owned by:
Priority: minor Milestone: GatherReqs
Component: Bio-Formats Version: 5.1.0-m1
Keywords: metadata, model Cc:
Resources: n.a. Referenced By: n.a.
References: n.a. Remaining Time: n.a.
Sprint: n.a.

Description

See attached images, which as shown below have 1 bit per sample:

% tiffinfo data-layout-bit-planar-pi1-strip-32-ordered-optimal.tiff
TIFF Directory at offset 0x608 (1544)
  Image Width: 64 Image Length: 64
  Bits/Sample: 1
  Sample Format: unsigned integer
  Compression Scheme: None
  Photometric Interpretation: min-is-black
  Samples/Pixel: 3
  Rows/Strip: 32
  Planar Configuration: separate image planes

% tiffinfo data-layout-bit-planar-pi2-strip-32-ordered-optimal.tiff
TIFF Directory at offset 0x608 (1544)
  Image Width: 64 Image Length: 64
  Bits/Sample: 1
  Sample Format: unsigned integer
  Compression Scheme: None
  Photometric Interpretation: RGB color
  Samples/Pixel: 3
  Rows/Strip: 32
  Planar Configuration: separate image planes

When imported into OMERO, these images all render correctly but are shown to have an incorrect pixel type (uint8) in both Insight and the Web client which is most certainly incorrect here.

Not sure if this is definitely a client bug or a deeper problem in the server, but filing again Insight initially since I'm uncertain.

Thanks,
Roger

Attachments (3)

bit.zip (455.6 KB) - added by rleigh 9 years ago.
Sample images with BIT pixel type (1 bit/sample)
bit-insight.png (307.7 KB) - added by rleigh 9 years ago.
Insight screenshot
bit-web.png (329.3 KB) - added by rleigh 9 years ago.
Web client screenshot

Download all attachments as: .zip

Change History (18)

Changed 9 years ago by rleigh

Sample images with BIT pixel type (1 bit/sample)

Changed 9 years ago by rleigh

Insight screenshot

Changed 9 years ago by rleigh

Web client screenshot

comment:1 Changed 9 years ago by jamoore

What's the impact of this? Perhaps if the issue doesn't exist in the API, then there won't be any data "corruption" (false readings)

comment:2 Changed 9 years ago by jburel

  • Component changed from Insight to Bio-Formats
  • Owner changed from jburel to mlinkert

The problem is not in the clients but in B-F
showinf on a file from bit.zip
`
Image count = 1

RGB = true (3)
Interleaved = false
Indexed = false (false color)
Width = 64
Height = 64
SizeZ = 1
SizeT = 1
SizeC = 3 (effectively 1)
Thumbnail size = 64 x 64
Endianness = intel (little)
Dimension order = XYCZT (certain)
Pixel type = uint8
Valid bits per pixel = 1
Metadata complete = true
Thumbnail series = false
-----
Plane #0 <=> Z 0, C 0, T 0

`

comment:3 Changed 9 years ago by jburel

see also #4107

comment:4 Changed 9 years ago by rleigh

Ah, I missed that for some reason, possibly using one with a different PhotometricInterp?.

I think the problem is that we have BIT with 3-channel RGB. Is the assumption being made that RGB==uint8? Looks like it is:

tiff/IFD.java getPixelType:
        return bitFormat == 2 ? FormatTools.INT8 : FormatTools.UINT8;

as the default case if the bit depth is not 16/24/32/64; does BIT need special-casing here?

comment:5 Changed 9 years ago by rleigh

Looks like it's independent of PhotometricInterpretation?: BIT is converted to UINT8 in all cases.

comment:6 Changed 9 years ago by mlinkert

  • Milestone changed from Unscheduled to 5.1.1

Moving to 5.1.1 for triage. This is all completely expected at the moment, per #4107 - the BIT type is effectively unused.

comment:7 Changed 9 years ago by mlinkert

  • Milestone changed from 5.1.1 to 5.1.3

This is going to take more work than I can do in the next 30 hours. Pushing to 5.1.3 (though incremental progress may be possible for 5.1.2).

comment:8 Changed 9 years ago by mlinkert

  • Milestone changed from 5.1.3 to 5.2.0-m1

Bumping to 5.2.0-m1. My understanding of the BIT type is that pixels should be stored as packed bytes; that's a breaking change at the moment, which I'd rather do for 5.2.0 than have in a patch release.

comment:9 Changed 9 years ago by jamoore

  • Milestone changed from 5.2.0 to B-F-5.2.0

Splitting due to milestone decoupling.

comment:10 Changed 8 years ago by mlinkert

  • Keywords metadata model added
  • Owner mlinkert deleted

Related to #11862; might sense to work on both together. I don't have immediate plans to work on either, so if anyone has strong feelings on these pixel types being supported for 5.2.0 feel free. The main reason I haven't done anything with this is that I'm not completely clear on what BIT actually means practically - should openBytes return what it does now, so effectively BIT == UINT8 + (ValidBits? == 1)? Or is the expectation that BIT implies packing 8 samples into each byte, so openBytes returns 1/8th its current size? The former is pretty easy; the latter will require substantial work across the whole stack, from common classes to readers to display in showinf/ImageJ/Matlab as well as docs.

It would be nice to at least answer that question before 5.2.0; if it gets down to the wire, the actual work would likely need to be pushed to 5.3.0.

comment:11 Changed 8 years ago by rleigh

It depends on what type of interface we want to expose really.

ImageJ uses uint8 with 0 and 255 as the valid values. The C++ interface likewise uses uint8 with 0 and 255 as the valid values--as a POD class with conversion to and from bool to make its usage as a bit type completely transparent.

I think if the Java API did that as well, that would be compatible with the existing behaviour (is Bio-Formats also using 0/255 ?). That would mean no big changes. In this case it's really specifying the semantics of what a byte means--by constraining it to min/max and not allowing grey levels for BIT and allowing the full range for UINT8. The rest of the behaviour would remain unchanged.

The main reason in C++ for not packing into bits is that the minimum size for array addressing is a byte, so it's not possible to easily index and subset an array if you also need to repack it. You then also have to handle trailing bits when not a multiple of 8; this means that you can't divide by the pixel type size to get the total number of pixels. That constraint would also apply to Java. In general, it's a bit wasteful of memory but it's vastly more flexible and simple to deal with.

Last edited 8 years ago by rleigh (previous) (diff)

comment:12 Changed 8 years ago by mlinkert

I agree that trying to pack bits requires more effort than the typically small byte savings warrants. The current return value of openBytes should result in min/max of 0 and 1 (this can be confirmed with 'showinf -minmax'), since this means that the actual pixel values are unchanged from what is stored in the file and setting the number of valid bits to 1 (as shown above) is meaningful. I don't feel particularly strongly about whether 1 or 255 should be the max once the pixel type is switched, so if there is a majority in favor of switching to 255 then go for it (but please do document in as many places as possible).

comment:13 Changed 8 years ago by rleigh

The C++ behaviour was pretty much to match ImageJ's behaviour plus also allow direct masking. Since it's an overloaded type, we treat any value > 0 as set when casting to bool, but always set to 255 on setting. So the 0/255 setting is opaque to the end user unless they inspect the raw bits, whether it's a view on original data or set by us; if they use the pixel buffer they never see this detail. The main benefit is that you can directly do a logical AND on uint8 and bit pixels.

I think that if we expose BIT as a pixel type in Java then it would be useful for whatever behaviour it chooses to be consistent across all readers, rather than copying what's stored in the file, so that it's abstracting the implementation details of the individual readers.

comment:14 Changed 8 years ago by rleigh

The other advantage of min/max being 0/255 is that it's directly viewable as the mask with most renderers without having to do any contrast adjustment.

comment:15 Changed 8 years ago by sbesson

  • Milestone changed from B-F-5.2.0 to GatherReqs

Moving alongside http://trac.openmicroscopy.org/ome/ticket/11862 as we will not tackle this ticket for 5.2.0.

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.66812 sec.)

We're Hiring!