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

Unable to set Exif resolution metadata in tiff files. #2794

Open
henryborchers opened this issue Oct 17, 2023 · 4 comments
Open

Unable to set Exif resolution metadata in tiff files. #2794

henryborchers opened this issue Oct 17, 2023 · 4 comments
Labels
help wanted request feature request or any other kind of wish

Comments

@henryborchers
Copy link
Contributor

Describe the bug

I'm unable to set the Exif.Image.XResolution and the Exif.Image.YResolution for tiff files with either than cli or the C++ api.
However, this technique works as expected jp2 files.

To Reproduce

Steps to reproduce the behavior:

cli:

exiv2 -M"set Exif.Image.XResolution 300/1" somefile.tif
exiv2 -pa --grep XResolution somefile.tif

C++ Api:

const std::string filename = "somefile.tif";
auto image = Exiv2::ImageFactory::open(filename);
image->readMetadata();
auto exifData = image->exifData();
exifData["Exif.Image.XResolution"] = Exiv2::URational(300, 1);
image->setExifData(exifData);
image->writeMetadata();

Expected behavior

I would expect that XResolution would be changed to 300 or 300/1.

Desktop (please complete the following information):

Additional context

This was originally addressed in #463.

@clanmills clanmills added request feature request or any other kind of wish and removed bug labels Oct 19, 2023
@clanmills
Copy link
Collaborator

@henryborchers That is intentional behaviour. The API image->isImageTag() says "treat this tag as readonly because changing it would damage the image!" For example, you cannot change ImageWidth or ImageHeight because you'd have to rewrite the image in the file. Exiv2 has no capability to process pixels.

The XResolution code is in src/tiffimage_int.cpp:

bool TiffHeaderBase::isImageTag(uint16_t /*tag*/, IfdId /*group*/, const PrimaryGroups* /*primaryGroups*/) const {
  return false;
}

static bool isTiffImageTagLookup(uint16_t tag, IfdId group) {
  if (group != IfdId::ifd0Id) {
    return false;
  }
  //! List of TIFF image tags
  switch (tag) {
...
    case 0x011a:  // Exif.Image.XResolution
    case 0x011b:  // Exif.Image.YResolution
...
      return true;
    default:
      return false;
  }
}

bool isTiffImageTag(uint16_t tag, IfdId group) {
  const bool result = isTiffImageTagLookup(tag, group);
#ifdef EXIV2_DEBUG_MESSAGES
  if (result) {
    ExifKey key(tag, groupName(group));
    std::cerr << "Image tag: " << key << " (3)\n";
  } else {
    std::cerr << "Not an image tag: " << tag << " (4)\n";
  }
#endif
  return result;
}

Including Resolution in that table is debatable, and the implementation of the JP2 image handler is otherwise. It would be reasonable to remove Resolution (and possibly other tags) from this restriction. I'm surprised at the inconsistency between different image handlers and that's worth investigation.

You and I discussed this 5 years ago. At that time, I was very busy with the release of Exiv2 0.27 (RC1, RC2 etc) and said "I'll deal with this later". I didn't participate in the v0.28 project and worked on "the dots" (0.27.1, 0.27.2 etc).

As I've retired, I recommend that you submit a PR with the necessary changes. You'll enjoy working with the splendid folks who now manage Exiv2.

@henryborchers
Copy link
Contributor Author

@clanmills As always, I appreciate everything you've done for this project and all the help you've personally provided me.

I completely agree that we would not want to have a program that breaks the image file. I do feel that it should be reasonable to change the x and y resolution and the resolution unit for a tiff file like we can do for jp2 images. I often have image scans of books and other archival collections that are generated with the incorrect dots per inch as part of the pipeline and need to be corrected before they become part of the digital archive.

I'm mostly a python programmer that knows enough to consume a c++ library enough to generate simple wrappers. However I'm not super proficient in generating production quality c++ code. That said it would be good practice for me to try a PR for a C++ project such this.

@clanmills
Copy link
Collaborator

@henryborchers Thank you for the kind comments about my Exiv2 efforts. I'm really happy to have contributed and happy that Team Exiv2 maintain and develop the code. You'll be surprised at what you can achieve with your PR. As a minimum, you could remove two lines of code. If that doesn't break the test suite, it might be accepted and you're done!

My usual experience is that there is more and usually much more to get a change accepted. You'll investigate every image handler, update the test suite and some documentation.

However, submitting a PR to remove two lines of code will receive the attention of Team Exiv2. You will learn if the team is willing to engage with this topic. Remember, like you, they have demanding full-time jobs.

Here's the minimum you need:

+++ b/src/tiffimage_int.cpp
@@ -2006,8 +2006,8 @@ namespace Exiv2 {
             { 0x0115, ifd0Id }, // Exif.Image.SamplesPerPixel
             { 0x0116, ifd0Id }, // Exif.Image.RowsPerStrip
             { 0x0117, ifd0Id }, // Exif.Image.StripByteCounts
-            { 0x011a, ifd0Id }, // Exif.Image.XResolution
-            { 0x011b, ifd0Id }, // Exif.Image.YResolution
+        //  { 0x011a, ifd0Id }, // Exif.Image.XResolution
+        //  { 0x011b, ifd0Id }, // Exif.Image.YResolution
             { 0x011c, ifd0Id }, // Exif.Image.PlanarConfiguration
             { 0x0122, ifd0Id }, // Exif.Image.GrayResponseUnit
             { 0x0123, ifd0Id }, // Exif.Image.GrayResponseCurve
543 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance/build $ ctest
Test project /Users/rmills/gnu/github/exiv2/0.27-maintenance/build
    Start 1: bashTests
1/4 Test #1: bashTests ........................   Passed    7.73 sec
    Start 2: bugfixTests
2/4 Test #2: bugfixTests ......................   Passed    3.34 sec
    Start 3: tiffTests
3/4 Test #3: tiffTests ........................   Passed    0.08 sec
    Start 4: versionTests
4/4 Test #4: versionTests .....................   Passed    0.07 sec

100% tests passed, 0 tests failed out of 4

Total Test time (real) =  11.22 sec
544 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance/build $ 

@henryborchers
Copy link
Contributor Author

@clanmills , you are welcome. I came to the same conclusion by patching those lines out (something you suggested to me 5 year ago ) and got it work!

When I get a moment, I'll see if I can get a PR together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted request feature request or any other kind of wish
Projects
None yet
Development

No branches or pull requests

3 participants