Open SkyeHoefling opened 2 years ago
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
| 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 |
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
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
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
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
Ditto to Mitch's comment, anything we can do to improve performance and memory allocation we will take advantage of it!
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