BUG: BMP can't be read without proper extension

[Nekto89]

This change allows reading BMP files that have wrong extension.

PR Checklist

• 🚫 Makes breaking changes
• 🚫 Makes design changes
• 🚫 Adds the License notice to new files.
• 🚫 Adds Python wrapping.
• 🚫 Adds tests and baseline comparison (quantitative).
• 🚫 Adds test data.
• 🚫 Adds Examples to ITKExamples
• 🚫 Adds Documentation.

[thewtex]

@Nekto89 well done - thanks!

[YannLePoul]

First question is is it a feature or a bug. For bmp, it has been implemented on purpose this way.

But I guess the real problem here is that the behavior is not consistent across all imageIO.
extensionFound comes from the HasSupportedReadExtension function. This function seems to be called in the CanReadFile method of BMPImageIO, JPEGImageIO, JPEG2000ImageIO, MINCImageIO, MRCImageIO, NrrdImageIO, StimulateImageIO, but not for other IO, like TiffImageIo...

[YannLePoul]

Merged too fast ?

I would be in favor of a consistent behavior for all ImageIO to test whether they can read the file or not not based on the file extension.

[YannLePoul]

The same behavior is found for BMPImageIO, JPEGImageIO, JPEG2000ImageIO, MINCImageIO, MRCImageIO, NrrdImageIO, StimulateImageIO, but not for other IO, like TiffImageIo...

[thewtex]

Some file formats or the reader implementations only can check whether they can read a given file based on the file extension.

Other readers can perform more advanced checks, e.g. looking at the first "magic" bytes that identify a file format in its header. These readers can correctly identify a file without a filename, with the wrong filename, or without a filename extension. In DICOM, for example, it is common to not have a filename extension.

So, we should not limit all readers to require an extension. But, if we can improve the BMP reader to go beyond extension checking, patches for that would be welcome :-).

[dzenanz]

BMP reader already looks for magic BM, it is just that it was also looking for .bmp extension before this PR.

[blowekamp]

We could cautiously allow ImageIOs to open the file to check if it can read the file. The way the IO factory works is that for every ImageIO it is constructed and then "CanRead" is called to determine if it's the correct one to use. If every ImageIO open the file, checks, and closes the file it will slow down this process ( consider network or remote files ).

There have been recent improvements for the ImageIOs to report the extensions it can read. To pursue this feature further I'd recommend making the IO Factory construction two passes:

• Check the extension of the file is supported for each IO, if found then use that ImageIO.
• Then check the CanRead method which may open the file.

[dzenanz]

Sounds logical, but is it a non-trivial amount of work. Can you create an issue to track the implementation? Are you willing to undertake it?

[dzenanz]

@Nekto89 I guess this patch introduced a new warning on one of the nightly build machines:
https://open.cdash.org/viewBuildError.php?type=1&buildid=6201669

[Nekto89]

@Nekto89 I guess this patch introduced a new warning on one of the nightly build machines:
https://open.cdash.org/viewBuildError.php?type=1&buildid=6201669

This is probably caused by older Xcode that warns on perfectly normal C++ code (same with std::array). Do you want me to add additional braces here?

[dzenanz]

It would be good.