How to actually use set_exif_metadata?

[Weasy666]

Here is a code snippet in how i wanted to use set_exif_metadata.

pub fn write_to<W>(self, writer: W) -> anyhow::Result<()>
where
    W: std::io::Write + std::io::Seek,
{
    let mut out = Cursor::new(Vec::new());
    let mut encoder = JpegEncoder::new_with_quality(&mut out, 100);
    let mut img = crate::crop_image(img, &self.props);
    if props.is_gray() {
    img = DynamicImage::from(img.into_luma8())
    }
    // this uses kamadak-exif writer to build the EXIF data
    let exif_bytes = props.as_exif(&img)?;
    encoder.set_exif_metadata(exif_bytes)?;
    img.write_with_encoder(encoder)?;
    img.write_to(writer, image::ImageFormat::Jpeg)?;
    Ok()
}

When i do that and then try to read the EXIF data of the stored file with the Linux command line tool exif, then i get an error

I'm not really sure the EXIF data is actually applied to the image, because as far as i can see the write_exif function of JpegEncoder is only called in encode_image and not in encode. But DynamicImage.write_with_encoder calls write_image whichs implementation of ImageEncoder calls encode.
In that chain write_exif doesn't seem to be called anywhere, or did i miss something?

[fintelia]

The encode method calls encode_image, so that part seems correct. Is it possible that the exif_bytes you're passing to this library aren't valid?

[Weasy666]

Oh... guess i missed that call.

Thanks for trying to figure this out with me. 🙂
They should be valid, i use kamadak-exif to create the bytes and when i use them with the functions provided by the img_part crate everything works.

let img_bytes = img.into_inner();
let mut jpeg = Jpeg::from_bytes(img_bytes )?;
jpeg.set_exif(Some(exif_bytes));
jpeg.encoder().write_to(writer)?;

That's why i was thinking that the EXIF bytes might not be applied by write_with_encoder

[197g]

Do you have a minimally reproduceable example for the image? It should be easier to verify if the App1 segment is there as planned or if the structure is messed up . The code sketch in the initial comment is somewhat hard to follow—apart from the obvious syntax mistake in Ok()—and I find it unlikely to represent your implementation's intended structure. It writes the same image into two different targets:

• img.write_with_encoder(encoder)?: which is what I assume you wanted to do, since that encoder has been configured with exif data. However, that same encoder writes into out which is a Cursor into an allocated Vec. Both the cursor and Vec are discarded on return, so none of this written data actually survives the call.
• img.write_to(writer, image::ImageFormat::Jpeg)? writes only the image with none of the metadata as is creates an encoder internally.

You'll likely want something similar to this instead:

pub fn write_to<W>(self, writer: W) -> anyhow::Result<()>
where
    W: std::io::Write + std::io::Seek,
{
    let mut img = crate::crop_image(img, &self.props);
    if props.is_gray() {
        img = DynamicImage::from(img.into_luma8())
    }

    let mut encoder = JpegEncoder::new_with_quality(&mut writer, 100);
    // this uses kamadak-exif writer to build the EXIF data
    let exif_bytes = props.as_exif(&img)?;
    encoder.set_exif_metadata(exif_bytes)?;
    img.write_with_encoder(encoder)?;
    Ok(())
}

[Weasy666]

Thanks, the second target was the problem. I thought img.write_with_encoder(encoder)?; will just set the encoder for the following call of img.write_to(writer, image::ImageFormat::Jpeg)?;. That's why i used a separate out target as a "buffer".
Thank you for the help!

[197g]

Huh, I do see how the API could be understood that way given it uses with, usually used in a builder pattern, and not denoting that the write stream is wrapped in the encoder. I suppose we should expand the documentation a bit to clarify, no clear alternative name for the method comes to mind immediately (write_to_encoder is a little odd but maybe that's just my familiarity bias).

[Weasy666]

Maybe the other way around would be more obvious

let mut encoder = JpegEncoder::new_with_quality(&mut writer, 100);
encoder.set_exif_metadata(exif_bytes)?;
encoder.write_image(&img)?;

But i'm not sure if this is feasible and if it really is an improvement. 🤔
I think write_to_encoder would have made it more obvious to me that it uses the writer embedded in the encoder, even if it's not 100% correct, because it's not really writing to the encoder, but only to the embedded/wrapped writer.