Skip to content

BUG: updated JPEG IO#2975

Merged
dzenanz merged 8 commits intoInsightSoftwareConsortium:masterfrom
issakomi:jpegio
Dec 21, 2021
Merged

BUG: updated JPEG IO#2975
dzenanz merged 8 commits intoInsightSoftwareConsortium:masterfrom
issakomi:jpegio

Conversation

@issakomi
Copy link
Copy Markdown
Member

@issakomi issakomi commented Dec 17, 2021

Commits are intentionally not squashed.

Follow-up of #2933

@github-actions github-actions Bot added the area:IO Issues affecting the IO module label Dec 17, 2021
Comment thread Modules/IO/JPEG/src/itkJPEGImageIO.cxx
Comment thread Modules/IO/JPEG/src/itkJPEGImageIO.cxx
Comment thread Modules/IO/JPEG/src/itkJPEGImageIO.cxx
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Comment thread Modules/IO/JPEG/src/itkJPEGImageIO.cxx Outdated
Comment thread Modules/IO/JPEG/src/itkJPEGImageIO.cxx Outdated
case 4:
// TODO
m_PixelType = IOPixelEnum::RGBA;
itkWarningMacro("JPEG image may be opened incorrectly");
Copy link
Copy Markdown
Member Author

@issakomi issakomi Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

@issakomi issakomi Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is how Gimp handles this

  switch (cinfo.output_components)
    {
    case 1:
      image_type = GIMP_GRAY;
      layer_type = GIMP_GRAY_IMAGE;
      break;

    case 3:
      image_type = GIMP_RGB;
      layer_type = GIMP_RGB_IMAGE;
      break;

    case 4:
      if (cinfo.out_color_space == JCS_CMYK)
        {
          image_type = GIMP_RGB;
          layer_type = GIMP_RGB_IMAGE;
          break;
        }
      /*fallthrough*/

    default:
      g_message ("Don't know how to load JPEG images "
                 "with %d color channels, using colorspace %d (%d).",
                 cinfo.output_components, cinfo.out_color_space,
                 cinfo.jpeg_color_space);
      return -1;
      break;
    }

CMYK data is additionally processed later.

Copy link
Copy Markdown
Member Author

@issakomi issakomi Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMYK is already done, will be added in separate PR. The files are not unusual, Photoshop has the option to export CMYK jpegs. They have (checked it):

cinfo.out_color_space == JCS_CMYK
cinfo.jpeg_color_space == JCS_YCCK

Current "RGBA" part is bug.

Important specially for external library. Not supported.
UINT input doesn't work, output is broken image
JPEG defines JPEG_MAX_DIMENSION in jmorecfg.h (65500L)
System library may have modified jmorecfg.h too.
Ensure there is no overflow in ReadImageInformation
Only componets number 1 to 4 was handled, may be more.
Give warnings about unsupported cases.
Set 'row_pointers' volatile and delete on jump
Don't flush twice
Warn about upsupported components
}
jpeg_write_scanlines(&cinfo, row_pointers, height);

if (fflush(fp) == EOF)
Copy link
Copy Markdown
Member Author

@issakomi issakomi Dec 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked, fflush is called here in term_destination (j_compress_ptr cinfo) called by jpeg_finish_compress(&cinfo);

@issakomi issakomi changed the title WIP: updated JPEG IO BUG: updated JPEG IO Dec 18, 2021
@issakomi issakomi marked this pull request as ready for review December 18, 2021 09:49
@issakomi issakomi requested a review from dzenanz December 18, 2021 09:49
Comment thread Modules/IO/JPEG/src/itkJPEGImageIO.cxx Outdated
@github-actions github-actions Bot added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Dec 20, 2021
Several minor changes as requested.
Improved check for overflow to usually avoid 2 multiplications.
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@issakomi excellent, thank you!

@dzenanz dzenanz merged commit ddcaa84 into InsightSoftwareConsortium:master Dec 21, 2021
@issakomi issakomi deleted the jpegio branch December 21, 2021 17:49
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:IO Issues affecting the IO module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants