Dear list, Back in April I submitted a patch that allowed imread() to correctly read PNGs that have odd bit-depths, ie not 8 or 16 (I actually submitted that to the Users list as I was unsure of protocol). There were a couple of things I left unfinished that I've finally got round to looking at again. The main remaining issue for me is that PNG specifies that all bit depths should be scaled to have the same maximum brightness, so that a value of 8191 in an 13-bit image is displayed the same as 65535 in a 16-bit image. Unfortunately, the LabView drivers for the 12-bit CCD in our lab do not follow this convention. A higher bit-depth from this setup means the image was brighter in an absolute sense and no scaling takes place. So this is not an error with Matplotlib as such, but more about having a decent way to handle iffy PNGs. It is worth noting that Matlab does not handle these PNGs well either (We have to query the image file using iminfo and then correct it) and PIL ignores anything above 8-bits as far as I can tell. A simple method, in my mind, and originally suggested by Andrew Straw is to add a keyword argument to imread() that indicates whether a user wants floats scaled between 0 and 1, or the raw byte values which they can then scale as required. This then gets passed to read_png(), which does the scaling if necessary and if not returns an array of UINT16s. I wrote a patch that does this, changing both image.py and _png.cpp. I'm very much open to other suggestions, as I didn't particularly want to fiddle with a core function like imread() and I'm fairly new to Python. In particular I have not changed anything to do with PIL - although it would not be much work to update pil_to_array() to follow the same behaviour as read_png(). I have tested this with the pngsuite.py*, and if desired I can submit an extended version of this that tests the extended bit-depth images from the PNG suite. Thanks in advance, Toby Wood * My patch also includes a minor change to pngsuite.py which was throwing a deprecation warning about using get_frame() istead of patch
I don't ever work with data-in-PNGs, so I won't comment on the use cases or API here -- I'll leave that to others. However, for the patch, I think the reinterpret_cast<unsigned short *> would be safer as reinterpret_cast<png_uint_16>, since unsigned ints are not guaranteed to be 16-bits, and png.h provides a nice convenient typedef for us. Also, why does the code not create an 8-bit numpy array for "raw" images that are only 8-bits? Also a style note: I find assignments inside of ternary operators (... ? ... : ...) confusing. I'd rather see that as a proper "if" block. Cheers, Mike Tobias Wood wrote: > Dear list, > Back in April I submitted a patch that allowed imread() to correctly > read PNGs that have odd bit-depths, ie not 8 or 16 (I actually > submitted that to the Users list as I was unsure of protocol). There > were a couple of things I left unfinished that I've finally got round > to looking at again. > > The main remaining issue for me is that PNG specifies that all bit > depths should be scaled to have the same maximum brightness, so that a > value of 8191 in an 13-bit image is displayed the same as 65535 in a > 16-bit image. Unfortunately, the LabView drivers for the 12-bit CCD in > our lab do not follow this convention. A higher bit-depth from this > setup means the image was brighter in an absolute sense and no scaling > takes place. So this is not an error with Matplotlib as such, but more > about having a decent way to handle iffy PNGs. It is worth noting that > Matlab does not handle these PNGs well either (We have to query the > image file using iminfo and then correct it) and PIL ignores anything > above 8-bits as far as I can tell. > > A simple method, in my mind, and originally suggested by Andrew Straw > is to add a keyword argument to imread() that indicates whether a user > wants floats scaled between 0 and 1, or the raw byte values which they > can then scale as required. This then gets passed to read_png(), which > does the scaling if necessary and if not returns an array of UINT16s. > I wrote a patch that does this, changing both image.py and _png.cpp. > I'm very much open to other suggestions, as I didn't particularly want > to fiddle with a core function like imread() and I'm fairly new to > Python. In particular I have not changed anything to do with PIL - > although it would not be much work to update pil_to_array() to follow > the same behaviour as read_png(). I have tested this with the > pngsuite.py*, and if desired I can submit an extended version of this > that tests the extended bit-depth images from the PNG suite. > > Thanks in advance, > Toby Wood > > * My patch also includes a minor change to pngsuite.py which was > throwing a deprecation warning about using get_frame() istead of patch > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------------ > Are you an open source citizen? Join us for the Open Source Bridge conference! > Portland, OR, June 17-19. Two days of sessions, one day of unconference: 250ドル. > Need another reason to go? 24-hour hacker lounge. Register today! > http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org > ------------------------------------------------------------------------ > > _______________________________________________ > Matplotlib-devel mailing list > Mat...@li... > https://lists.sourceforge.net/lists/listinfo/matplotlib-devel -- Michael Droettboom Science Software Branch Operations and Engineering Division Space Telescope Science Institute Operated by AURA for NASA
Michael, Thanks for the comments, much appreciated. I've attached an updated patch including your suggestions and some more whitespace for readability. There was no reason other than simplicity for not returning a 8-bit numpy array. I actually meant to ask about the ternary blocks - I think I picked up the style from the original code and had continued in the same vein for compactness. While I was testing this I came across another issue - which variety of FLOAT should the code return? My understanding was that Python floats are C doubles. However the code was previously returning a PyArray_FLOAT, which seems to be a FLOAT32 rather than a FLOAT64. Hence I removed any trace of doubles from my code and have left it all at float precision. Thanks again, Toby On 2009年6月22日 15:58:58 +0100, Michael Droettboom <md...@st...> wrote: > I don't ever work with data-in-PNGs, so I won't comment on the use cases > or API here -- I'll leave that to others. > > However, for the patch, I think the reinterpret_cast<unsigned short *> > would be safer as reinterpret_cast<png_uint_16>, since unsigned ints are > not guaranteed to be 16-bits, and png.h provides a nice convenient > typedef for us. Also, why does the code not create an 8-bit numpy array > for "raw" images that are only 8-bits? > > Also a style note: I find assignments inside of ternary operators (... ? > ... : ...) confusing. I'd rather see that as a proper "if" block. > > Cheers, > Mike > > Tobias Wood wrote: >> Dear list, >> Back in April I submitted a patch that allowed imread() to correctly >> read PNGs that have odd bit-depths, ie not 8 or 16 (I actually >> submitted that to the Users list as I was unsure of protocol). There >> were a couple of things I left unfinished that I've finally got round >> to looking at again. >> >> The main remaining issue for me is that PNG specifies that all bit >> depths should be scaled to have the same maximum brightness, so that a >> value of 8191 in an 13-bit image is displayed the same as 65535 in a >> 16-bit image. Unfortunately, the LabView drivers for the 12-bit CCD in >> our lab do not follow this convention. A higher bit-depth from this >> setup means the image was brighter in an absolute sense and no scaling >> takes place. So this is not an error with Matplotlib as such, but more >> about having a decent way to handle iffy PNGs. It is worth noting that >> Matlab does not handle these PNGs well either (We have to query the >> image file using iminfo and then correct it) and PIL ignores anything >> above 8-bits as far as I can tell. >> >> A simple method, in my mind, and originally suggested by Andrew Straw >> is to add a keyword argument to imread() that indicates whether a user >> wants floats scaled between 0 and 1, or the raw byte values which they >> can then scale as required. This then gets passed to read_png(), which >> does the scaling if necessary and if not returns an array of UINT16s. I >> wrote a patch that does this, changing both image.py and _png.cpp. I'm >> very much open to other suggestions, as I didn't particularly want to >> fiddle with a core function like imread() and I'm fairly new to Python. >> In particular I have not changed anything to do with PIL - although it >> would not be much work to update pil_to_array() to follow the same >> behaviour as read_png(). I have tested this with the pngsuite.py*, and >> if desired I can submit an extended version of this that tests the >> extended bit-depth images from the PNG suite. >> >> Thanks in advance, >> Toby Wood >> >> * My patch also includes a minor change to pngsuite.py which was >> throwing a deprecation warning about using get_frame() istead of patch >> ------------------------------------------------------------------------ >> >> ------------------------------------------------------------------------------ >> Are you an open source citizen? Join us for the Open Source Bridge >> conference! >> Portland, OR, June 17-19. Two days of sessions, one day of >> unconference: 250ドル. >> Need another reason to go? 24-hour hacker lounge. Register today! >> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org >> ------------------------------------------------------------------------ >> >> _______________________________________________ >> Matplotlib-devel mailing list >> Mat...@li... >> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel >
Tobias Wood wrote: > Michael, > Thanks for the comments, much appreciated. I've attached an updated > patch including your suggestions and some more whitespace for > readability. There was no reason other than simplicity for not > returning a 8-bit numpy array. I actually meant to ask about the > ternary blocks - I think I picked up the style from the original code > and had continued in the same vein for compactness. Thanks! > > While I was testing this I came across another issue - which variety > of FLOAT should the code return? My understanding was that Python > floats are C doubles. However the code was previously returning a > PyArray_FLOAT, which seems to be a FLOAT32 rather than a FLOAT64. > Hence I removed any trace of doubles from my code and have left it all > at float precision. Yes, that's all correct. I suspect read_png creates arrays of floats rather than doubles just for the sake of memory savings -- and the fact that doubles would be overkill for even 16-bit integral data. Thanks for the new patch. I'll wait a bit to see if there's any comments on the functionality itself or the API before committing it. Cheers, Mike > > Thanks again, > Toby > > On 2009年6月22日 15:58:58 +0100, Michael Droettboom > <md...@st...> wrote: > >> I don't ever work with data-in-PNGs, so I won't comment on the use >> cases or API here -- I'll leave that to others. >> >> However, for the patch, I think the reinterpret_cast<unsigned short >> *> would be safer as reinterpret_cast<png_uint_16>, since unsigned >> ints are not guaranteed to be 16-bits, and png.h provides a nice >> convenient typedef for us. Also, why does the code not create an >> 8-bit numpy array for "raw" images that are only 8-bits? >> >> Also a style note: I find assignments inside of ternary operators >> (... ? ... : ...) confusing. I'd rather see that as a proper "if" >> block. >> >> Cheers, >> Mike >> >> Tobias Wood wrote: >>> Dear list, >>> Back in April I submitted a patch that allowed imread() to correctly >>> read PNGs that have odd bit-depths, ie not 8 or 16 (I actually >>> submitted that to the Users list as I was unsure of protocol). There >>> were a couple of things I left unfinished that I've finally got >>> round to looking at again. >>> >>> The main remaining issue for me is that PNG specifies that all bit >>> depths should be scaled to have the same maximum brightness, so that >>> a value of 8191 in an 13-bit image is displayed the same as 65535 in >>> a 16-bit image. Unfortunately, the LabView drivers for the 12-bit >>> CCD in our lab do not follow this convention. A higher bit-depth >>> from this setup means the image was brighter in an absolute sense >>> and no scaling takes place. So this is not an error with Matplotlib >>> as such, but more about having a decent way to handle iffy PNGs. It >>> is worth noting that Matlab does not handle these PNGs well either >>> (We have to query the image file using iminfo and then correct it) >>> and PIL ignores anything above 8-bits as far as I can tell. >>> >>> A simple method, in my mind, and originally suggested by Andrew >>> Straw is to add a keyword argument to imread() that indicates >>> whether a user wants floats scaled between 0 and 1, or the raw byte >>> values which they can then scale as required. This then gets passed >>> to read_png(), which does the scaling if necessary and if not >>> returns an array of UINT16s. I wrote a patch that does this, >>> changing both image.py and _png.cpp. I'm very much open to other >>> suggestions, as I didn't particularly want to fiddle with a core >>> function like imread() and I'm fairly new to Python. In particular I >>> have not changed anything to do with PIL - although it would not be >>> much work to update pil_to_array() to follow the same behaviour as >>> read_png(). I have tested this with the pngsuite.py*, and if desired >>> I can submit an extended version of this that tests the extended >>> bit-depth images from the PNG suite. >>> >>> Thanks in advance, >>> Toby Wood >>> >>> * My patch also includes a minor change to pngsuite.py which was >>> throwing a deprecation warning about using get_frame() istead of patch >>> ------------------------------------------------------------------------ >>> >>> >>> ------------------------------------------------------------------------------ >>> >>> Are you an open source citizen? Join us for the Open Source Bridge >>> conference! >>> Portland, OR, June 17-19. Two days of sessions, one day of >>> unconference: 250ドル. >>> Need another reason to go? 24-hour hacker lounge. Register today! >>> http://ad.doubleclick.net/clk;215844324;13503038;v?http://opensourcebridge.org >>> >>> ------------------------------------------------------------------------ >>> >>> >>> _______________________________________________ >>> Matplotlib-devel mailing list >>> Mat...@li... >>> https://lists.sourceforge.net/lists/listinfo/matplotlib-devel >> > -- Michael Droettboom Science Software Branch Operations and Engineering Division Space Telescope Science Institute Operated by AURA for NASA