Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect Colorspace and Chroma Before Decode #81

Open
SkyeHoefling opened this issue Jan 21, 2022 · 4 comments
Open

Detect Colorspace and Chroma Before Decode #81

SkyeHoefling opened this issue Jan 21, 2022 · 4 comments

Comments

@SkyeHoefling
Copy link
Contributor

Description

When decoding and writing images to jpeg libheif performs a conversion from the native colorspace and chroma to the expected YUV (colorspace) and 420 (chroma). This operation takes about .5s - 1.2s in debug mode. If we detect this prior to attempting a decode and keep it in the same format we may be able to shave time off our image processing. This will require updating our jpeg encoder to handle a variety of colorspace/chromas and have a fallback strategy

@SkyeHoefling
Copy link
Contributor Author

I did some digging into this to see what is going on and what we can do to solve this. The code change may be easier than I originally anticipated. The root cause is in libheif, when you specify the Colorspace/Chroma. The FileOnQ.Imaging.Heif C++ encoder requires it to be colorspace YUV or YCbCr and chroma 420. In heif_context.cc the HeifContext::decode_image_user (see snippet below) function checks the requested colorspace and chroma. If they are different from what is stored in the file it converts them. Since our conversion is encoding the image after it is decoded, we should be able to skip this additional step and handle the format in the encoder

Error HeifContext::decode_image_user(heif_item_id ID,
                                     std::shared_ptr<HeifPixelImage>& img,
                                     heif_colorspace out_colorspace,
                                     heif_chroma out_chroma,
                                     const struct heif_decoding_options* options) const
{
  Error err = decode_image_planar(ID, img, out_colorspace);
  if (err) {
    return err;
  }

  // --- convert to output chroma format

  heif_colorspace target_colorspace = (out_colorspace == heif_colorspace_undefined ?
                                       img->get_colorspace() :
                                       out_colorspace);

  heif_chroma target_chroma = (out_chroma == heif_chroma_undefined ?
                               img->get_chroma_format() : out_chroma);

  bool different_chroma = (target_chroma != img->get_chroma_format());
  bool different_colorspace = (target_colorspace != img->get_colorspace());

  int bpp = (options && options->convert_hdr_to_8bit) ? 8 : 0;
// TODO: check BPP changed
  if (different_chroma || different_colorspace) {
    img = convert_colorspace(img, target_colorspace, target_chroma, nullptr, bpp);
    if (!img) {
      return Error(heif_error_Unsupported_feature, heif_suberror_Unsupported_color_conversion);
    }
  }

  return Error::Ok;
}

I don't have a complete working prototype, but I was able to decode one of the sample images in RGB/Chroma444 and then encode it with the same settings in our encoder. I am seeing a 0.822s performance improvement and 17MB less allocated memory. This is a significant finding, see my results below

Benchmark Results

|              Method |    Mean |    Error |   StdDev | Allocated native memory | Native memory leak | Allocated |
|---------------------|--------:|---------:|---------:|------------------------:|-------------------:|----------:|
| PrimaryImage_NewAlg | 2.148 s | 0.0082 s | 0.0072 s |                  196 MB |                  - |      2 MB |
| PrimaryImage_OldAlg | 2.970 s | 0.0157 s | 0.0147 s |                  213 MB |                  - |      2 MB |

Generated JPEG

The generated jpeg file is not 100% correct, so the benchmark results should be considered anecdotal. I do think the results are promising and we can get this tweaked to work correctly
20210224_023830752_iOS

@SkyeHoefling
Copy link
Contributor Author

I was able to fix the encoding for the JPEF image and it looks correct to me. The integration test pass with the updated encoder, which is a good sign. See image below
20210224_023830752_iOS

To keep track of this work I have pushed up a branch that has the working code. https://github.com/FileOnQ/Imaging.Heif/tree/perf. This branch is not ready to be PRed but here is the general idea of the updated implementation strategy

  • Decode image
  • Determine colorspace and chroma from decoded image
  • Use specific jpeg encoder that matches the colorspace. For example if it is YUV or RGB or RGBA the encoder will need to be slightly different. If we don't have a matching encoder, we will need to have a fall back strategy where the image is converted to a colorspace of our choice such as RGB

@mitchelsellers
Copy link
Collaborator

This is a great find and a substantial improvement.

Have you by chance found any listing of standards for image setup across common devices? That could help identify which we optimize for vs those that we fall back for

@kenny-sellers
Copy link
Collaborator

Ditto to Mitch's comment, anything we can do to improve performance and memory allocation we will take advantage of it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants