genicam / harvesters

Image Acquisition Library for GenICam-based Machine Vision System
Apache License 2.0
518 stars 89 forks source link

Mono10p packed pixel format decoding #222

Closed erik-mansson closed 3 years ago

erik-mansson commented 3 years ago

Describe the bug I downloaded the tagged version 1.2.8 where I saw that some packed pixel formats were supported (issues #146 and #167). 12- and 14-bit formats were mentioned in those issues but I have a camera (Mako U-130B from Allied Vision) that can give Mono10p and it seems Harvesters doesn't decode this format correctly.

In util/pfnc.py the Mono10p format gets handled by _10p.expand(array) via the subclasses _Mono_10pand Mono10p when buffer.payload.components[0].data_format == 'Mono10p', which returns an array of the wrong length and where pixel intensities are scrambled (incorrect). For instance, I acquired 1280*1024=1310720 pixels so that the Mono10p-packed raw buffer has 1638400 bytes (1.25 B/px = 10b/px), but then _10p.expand(array) gives 1228800 array elements with incorrect values. unpacked_Mono10p_wrong

After comparing to the packing formats defined in https://www.emva.org/wp-content/uploads/GenICam_PFNC_2_3.pdf (mainly pages 32 and 34) I think the _10p.expand(array) is a slightly incorrect (off by 1 bit) implementation of Mono10c3p32 and the equivalent RGB10p32, where 4 bytes give 3 pixels (ignoring 2 bits of padding). But the Mono10p as defined on page 34 should use 5 bytes to give 4 pixels.

Expected behavior (& fix) In the attached patch I have replaced the _10p.expand(array)-method with what I believe is the correct implementation for Mono10p, based on the PDF-documentation and based on the actual bytes I get from this camera (the images get the correct size and look good). (Well, I didn't actually run with the patched util/pfnc.py yet, I applied the conversion after having exported the raw buffer of bytes.) unpacked_Mono10p_correct

Since there may well be a use-case for the Mono10c3p32 and the equivalent RGB10p32 packings, my patch file also keeps a renamed version of the old _10p.expand(array)-method where I documented what it does, and modified it to avoid the off-by-1-bit error for one of the three pixels per chunk. So I believe it now matches the documented Mono10c3p32 and RGB10p32, but I didn't create or look for the suitable classes in util/pfnc.py where this method would have to be moved to be useful.

Since version 1.2.8 where any packed formats are supported is not so old, I would guess not many users rely on the old behaviour. If there are other cameras where the old behavior was good we should of course find some way to keep it possible to use, but my search through closed issues didn't show anyone having requested Mono10p or the old version's behavior.

System information:

kazunarikudo commented 3 years ago

@erik-mansson Halle Erik, thank you very much for your valuable report. I will look at the suggested patch and merge it in the master. Regards, Kazunari.

kazunarikudo commented 3 years ago

Please feel free to re-open this ticket if there still be a bug. Regards, Kazunari.