olympus makernotes get lost or corrupt with 0.27.6

[tenzap]

Describe the bug

olympus makernotes get lost or corrupt with 0.27.6 (with 0.27.5 there is no problem)

To Reproduce

$ exiv2 --version
exiv2 0.27.6

$ exiv2 -pa 42_IndexError.jpg | grep -c Olympus
132

→ 132 lines of Olympus Makernotes

$ exiv2 -M"add  Exif.Photo.UserComment MyComment4" 42_IndexError.jpg

$ exiv2 -pa 42_IndexError.jpg | grep Olympus -c
Error: Failed to read Olympus IFD Makernote header.
0

→ 0 line of Olympus Makernotes + error

Input file can be taken here: https://github.com/ianare/exif-samples/raw/master/jpg/tests/42_IndexError.jpg

Expected behavior

Makernotes should still be there after adding a tag.

Desktop (please complete the following information):

• Debian bookworm
• Exiv2 version and source: 0.27.6 from debian bookworm

[clanmills]

@tenzap Well written bug report.

It looks as though the data is in the modified file, however the makernote header has been incorrectly rewritten. Using tvisitor (the program in my book), the modified and original file have:

569 rmills@rmillsm1:~/temp $ tvisitor -pRU 42_IndexError.jpg | grep -i makernote
     address |    tag                                  |      type |    count |    offset | value
        1150 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |     9412 |      1450 | OM SYSTEM___II._.__.._.___j___..._ _ +++
570 rmills@rmillsm1:~/temp $ tvisitor -pRU 42_IndexError.jpg.orig | grep -i makernote
         388 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    10368 |      3208 | OLYMPUS_II._.__.._.___..__..._ ___(. +++
571 rmills@rmillsm1:~/temp $ 

When the header is OM SYSTEM\0, exiv2 doesn't recognise the header and does not invoke the makernote parser. Hence the error message:

574 rmills@rmillsm1:~/temp $ exiv2 -pa 42_IndexError.jpg >/dev/null
Error: Failed to read Olympus IFD Makernote header.
575 rmills@rmillsm1:~/temp $ 

There's another issue in the rewrite. In the original file, there are 5 IFDs which are correctly identified as Olympus{Eq, Cs, Rd, Fi, Ip} and they have been rewritten as type LONG:

571 rmills@rmillsm1:~/temp $ tvisitor -pRU 42_IndexError.jpg.orig | grep -i IFD
       address |    tag                                  |      type |    count |    offset | value
            38 | 0x2010 Exif.Olympus.Equipment           |       IFD |        1 |           | 102
            50 | 0x2020 Exif.Olympus.CameraSettings      |       IFD |        1 |           | 408
            62 | 0x2030 Exif.Olympus.RawDevelopment      |       IFD |        1 |           | 1158
            74 | 0x2040 Exif.Olympus.ImageProcessing     |       IFD |        1 |           | 1332
            86 | 0x2050 Exif.Olympus.FocusInfo           |       IFD |        1 |           | 2358
572 rmills@rmillsm1:~/temp $ tvisitor -pRU 42_IndexError.jpg | grep -e Equipment -e CameraSettings -e RawDev -e ImageProc -e FocusInfo
            42 | 0x2010 Exif.Olympus.Equipment           |      LONG |        1 |           | 150
            54 | 0x2020 Exif.Olympus.CameraSettings      |      LONG |        1 |           | 738
            66 | 0x2030 Exif.Olympus.RawDevelopment      |      LONG |        1 |           | 2188
            78 | 0x2040 Exif.Olympus.ImageProcessing     |      LONG |        1 |           | 2394
            90 | 0x2050 Exif.Olympus.FocusInfo           |      LONG |        1 |           | 4576
573 rmills@rmillsm1:~/temp $ 

My suspicion is that the Olympus MakerNote parser has been in the code base for many years without attracting attention and this behaviour will also exist in the main branch.

I find software puzzles interesting. As I've retired from Exiv2, I will not be involved with fixing this issue.

[clanmills]

I've just noticed your remark that his behaviour is new in 0.27.6 and discovered the following code:

591 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance $ git blame src/makernote_int.cpp | grep -i olym | grep 202
916436903 src/makernote_int.cpp (Miloš Komarčević 2022-03-25 09:51:47 +0100  133)         { "OM Digital",     olympus2Id,  newOMSystemMn,  newOMSystemMn2  },
592 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance $ 

This added the Olympus2 parser, which results in:

599 rmills@rmillsm1:~/temp $ exiv2 -g Olympus2 42_IndexError.jpg.orig 
Exif.Olympus2.SpecialMode                    Long        3  Normal
Exif.Olympus2.CameraID                       Undefined  32  OLYMPUS DIGITAL CAMERA         
Exif.Olympus2.Equipment                      Ifd         1  102
Exif.Olympus2.CameraSettings                 Ifd         1  408
Exif.Olympus2.RawDevelopment                 Ifd         1  1158
Exif.Olympus2.ImageProcessing                Ifd         1  1332
Exif.Olympus2.FocusInfo                      Ifd         1  2358
600 rmills@rmillsm1:~/temp $ 

The code to define OM SYSTEM is in makernote_int.cpp:

    const byte OMSystemMnHeader::signature_[] = {
        'O', 'M', ' ', 'S', 'Y', 'S', 'T',  'E', 'M', 0x00, 0x00, 0x00, 'I', 'I', 0x04, 0x00
    };

which was added last year:

$ git blame src/makernote_int.cpp | grep 2022
916436903 src/makernote_int.cpp (Miloš Komarčević 2022-03-25 09:51:47 +0100  133)         { "OM Digital",     olympus2Id,  newOMSystemMn,  newOMSystemMn2  },
916436903 src/makernote_int.cpp (Miloš Komarčević 2022-03-25 09:51:47 +0100  334)     const byte OMSystemMnHeader::signature_[] = {
916436903 src/makernote_int.cpp (Miloš Komarčević 2022-03-25 09:51:47 +0100  335)         'O', 'M', ' ', 'S', 'Y', 'S', 'T',  'E', 'M', 0x00, 0x00, 0x00, 'I', 'I', 0x04, 0x00

OK. So, that's the post-mortem. Now we need to get this fixed and @kmilos has added the "help wanted" label which means he'd like somebody to volunteer as he is very busy.

@tenzap Will you accept the challenge to fix this?

[kmilos]

Yep, I added the support for "OM System" as an alias of Olympus for the read, but don't have time to dig into the writing side of things at the moment...

[clanmills]

@kmilos Milos: Thanks for your reply. Let's see if @tenzap is willing to work on this. I'll help and support him as I know you're busy.

[tenzap]

@tenzap Will you accept the challenge to fix this?

No sorry. I'll stay with 0.27.5 in the meantime. Hopefully someone else can help.

[clanmills]

I don't believe the fix is very difficult, and I will be happy to mentor a volunteer.

[clanmills]

I'm making progress with this and have discovered why this is going wrong. I don't yet know how to fix it, however I understand why #2545 has upset code the olympus code which worked in 0.27.5.

The issue concerns the "header" of the MakerNote tag 0x927c. In Olympus files, they can begin OLYMP\0 or OLYMPUS\0. The code didn't care. The Olympus makenote parse is triggered by the value of Exif.Image.Make which could be OLYMPUS IMAGING CORP, or OLYMPUS OPTICAL CO.,LTD and other values including the word OLYMPUS.

2545 deals with a new situation. We are using the Olympus makernote code to handle Exif.Image.Make = OM Digital Solutions which has the makernote header OM SYSTEM\0

646 rmills@rmillsm1:~/Pictures $ tvisitor -pRU ~/temp/OM.JPEG | grep Make
        22 | 0x010f Exif.Image.Make                  |     ASCII |       24 |      2552 | OM Digital Solutions   
         460 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    20156 |      5548 | OM SYSTEM___II._.__.._.___ .__..._ _ +++
647 rmills@rmillsm1:~/Pictures $ 

I recall a conversation in March 2022 with @kmilos about refactoring class OlympusMnHeader to avoid a new class. As @kmilos is very smart he ignored my advice and wisely introduced class OMSystemMnHeader. Good call, Milos.

There's something wrong with this function:

    TiffComponent* newOlympusMn(uint16_t    tag,
                                IfdId       group,
                                IfdId       /*mnGroup*/,
                                const byte* pData,
                                uint32_t    size,
                                ByteOrder   /*byteOrder*/)
    {
        if (size < 10 ||   std::string(reinterpret_cast<const char*>(pData), 10)
                        != std::string("OLYMPUS\0II", 10)) {
            // Require at least the header and an IFD with 1 entry
            if (size < OlympusMnHeader::sizeOfSignature() + 18) return 0;
            return newOlympusMn2(tag, group, olympusId);
        }
        // Require at least the header and an IFD with 1 entry
        if (size < Olympus2MnHeader::sizeOfSignature() + 18) return 0;
        return newOlympus2Mn2(tag, group, olympus2Id);
    }

When dealing with the the file from @tenzap, we have a makernote header of OLYMPUS\0II which is incorrectly treated as a OMSystem file.

649 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance $ tvisitor -pRU ~/temp/42_IndexError.jpg.orig | grep -e Image.Make -e MakerNote
        22 | 0x010f Exif.Image.Make                  |     ASCII |       24 |      2196 | OLYMPUS IMAGING CORP.  
         388 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    10368 |      3208 | OLYMPUS_II._.__.._.___..__..._ ___(. +++
650 rmills@rmillsm1:~/gnu/github/exiv2/0.27-maintenance $ 

There is an issue with this , 6 in the following code as it incorrectly presumes that the header is OLYMP\0. The header is OLYMPUS\0 which has 8 bytes.

    bool OlympusMnHeader::read(const byte* pData,
                               uint32_t size,
                               ByteOrder /*byteOrder*/)
    {
        if (!pData || size < sizeOfSignature()) return false;
        header_.alloc(sizeOfSignature());
        std::memcpy(header_.pData_, pData, header_.size_);
        if (   static_cast<uint32_t>(header_.size_) < sizeOfSignature()
            || 0 != memcmp(header_.pData_, signature_, 6)) {
            return false;
        }
        return true;
    } // OlympusMnHeader::read

The puzzle of the makernotes IFDs being ignored by the OMSystem Parser is caused by their absence in src/tags.cpp. I don't think we need to solve that issue as it will evaporate when we handle 42_IndexError.jpg with OlympusMnHeader instead of OMSystemMnHeader.

The file OM.JPEG which is part of #2545 requires IFD parsing support.

    //! List of all known Exif groups. Important: Group name (3rd column) must be unique!
    extern const GroupInfo groupInfo[] = {
...
        { olympusId,       "Makernote", "Olympus",      OlympusMakerNote::tagList      },
        { olympus2Id,      "Makernote", "Olympus2",     OlympusMakerNote::tagList      },
        { olympusCsId,     "Makernote", "OlympusCs",    OlympusMakerNote::tagListCs    },
        { olympusEqId,     "Makernote", "OlympusEq",    OlympusMakerNote::tagListEq    },
        { olympusRdId,     "Makernote", "OlympusRd",    OlympusMakerNote::tagListRd    },
        { olympusRd2Id,    "Makernote", "OlympusRd2",   OlympusMakerNote::tagListRd2   },
        { olympusIpId,     "Makernote", "OlympusIp",    OlympusMakerNote::tagListIp    },
        { olympusFiId,     "Makernote", "OlympusFi",    OlympusMakerNote::tagListFi    },
...
    };

I'm going to take a break for a couple of days (to work on some musical matters). I'll be back later in the week.

[clanmills]

I've got a patch for you to test out and give me feedback. Please do not use this patch for any production purposes. It looks more formidable than it is. Most of the changes were done by Xcode "Refactor/rename". It's not much more than renaming olympusId2 as omsystemId and updating the tiffvisitor state table.

The patch needs more refinement. For example, it's failing the test suite. @tenzap Your feedback will be appreciated.

diff --git a/src/makernote_int.cpp b/src/makernote_int.cpp
index 467ce205..d52c25a9 100644
--- a/src/makernote_int.cpp
+++ b/src/makernote_int.cpp
@@ -129,8 +129,8 @@ namespace Exiv2 {
         { "KONICA MINOLTA", minoltaId,   newIfdMn,       newIfdMn2       },
         { "Minolta",        minoltaId,   newIfdMn,       newIfdMn2       },
         { "NIKON",          ifdIdNotSet, newNikonMn,     0               }, // mnGroup_ is not used
-        { "OLYMPUS",        ifdIdNotSet, newOlympusMn,   0               }, // mnGroup_ is not used
-        { "OM Digital",     olympus2Id,  newOMSystemMn,  newOMSystemMn2  },
+        { "OLYMP",          ifdIdNotSet, newOlympusMn,   0               }, // mnGroup_ is not used
+        { "OM Digital",     ifdIdNotSet, newOMSystemMn,  newOMSystemMn2  },
         { "Panasonic",      panasonicId, newPanasonicMn, newPanasonicMn2 },
         { "PENTAX",         ifdIdNotSet, newPentaxMn,    0               }, // mnGroup_ is not used
         { "RICOH",          ifdIdNotSet, newPentaxMn,    0               }, // mnGroup_ is not used
@@ -145,7 +145,7 @@ namespace Exiv2 {
         { "-",              sony1Id,     0,              newSony1Mn2     },
         { "-",              sony2Id,     0,              newSony2Mn2     },
         { "-",              olympusId,   0,              newOlympusMn2   },
-        { "-",              olympus2Id,  0,              newOlympus2Mn2  },
+        // { "-",              omsystemId,  0,              newOlympus2Mn2  },
         { "-",              pentaxId,    0,              newPentaxMn2    },
         { "-",              pentaxDngId, 0,              newPentaxDngMn2 },
         { "-",              casioId,     0,              newIfdMn2       },
@@ -938,7 +938,7 @@ namespace Exiv2 {
         }
         // Require at least the header and an IFD with 1 entry
         if (size < Olympus2MnHeader::sizeOfSignature() + 18) return 0;
-        return newOlympus2Mn2(tag, group, olympus2Id);
+        return newOlympus2Mn2(tag, group, omsystemId);
     }

     TiffComponent* newOlympusMn2(uint16_t tag,
diff --git a/src/tags_int.cpp b/src/tags_int.cpp
index 12155921..f2196a33 100644
--- a/src/tags_int.cpp
+++ b/src/tags_int.cpp
@@ -132,7 +132,7 @@ namespace Exiv2 {
         { nikonLd3Id,      "Makernote", "NikonLd3",     Nikon3MakerNote::tagListLd3    },
         { nikonLd4Id,      "Makernote", "NikonLd4",     Nikon3MakerNote::tagListLd4    },
         { olympusId,       "Makernote", "Olympus",      OlympusMakerNote::tagList      },
-        { olympus2Id,      "Makernote", "Olympus2",     OlympusMakerNote::tagList      },
+        // { olympus2Id,      "Makernote", "Olympus2",     OlympusMakerNote::tagList      },
         { olympusCsId,     "Makernote", "OlympusCs",    OlympusMakerNote::tagListCs    },
         { olympusEqId,     "Makernote", "OlympusEq",    OlympusMakerNote::tagListEq    },
         { olympusRdId,     "Makernote", "OlympusRd",    OlympusMakerNote::tagListRd    },
diff --git a/src/tags_int.hpp b/src/tags_int.hpp
index 484a8be8..f3315f3b 100644
--- a/src/tags_int.hpp
+++ b/src/tags_int.hpp
@@ -142,7 +142,7 @@ namespace Exiv2 {
         nikonCb3Id,
         nikonCb4Id,
         olympusId,
-        olympus2Id,
+        omsystemId,
         olympusCsId,
         olympusEqId,
         olympusRdId,
diff --git a/src/tiffimage_int.cpp b/src/tiffimage_int.cpp
index e6f70208..9684a819 100644
--- a/src/tiffimage_int.cpp
+++ b/src/tiffimage_int.cpp
@@ -1025,24 +1025,24 @@ namespace Exiv2 {
         { Tag::root, ifd2Id,           ifd1Id,           Tag::next },
         { Tag::root, ifd3Id,           ifd2Id,           Tag::next },
         { Tag::root, olympusId,        exifId,           0x927c    },
-        { Tag::root, olympus2Id,       exifId,           0x927c    },
+        { Tag::root, omsystemId,       exifId,           0x927c    },
         { Tag::root, subThumb1Id,      ifd1Id,           0x014a    },
-        { Tag::root, olympusEqId,      olympus2Id,       0x2010    },
-        { Tag::root, olympusCsId,      olympus2Id,       0x2020    },
-        { Tag::root, olympusRdId,      olympus2Id,       0x2030    },
-        { Tag::root, olympusRd2Id,     olympus2Id,       0x2031    },
-        { Tag::root, olympusIpId,      olympus2Id,       0x2040    },
-        { Tag::root, olympusFiId,      olympus2Id,       0x2050    },
-        { Tag::root, olympusFe1Id,     olympus2Id,       0x2100    },
-        { Tag::root, olympusFe2Id,     olympus2Id,       0x2200    },
-        { Tag::root, olympusFe3Id,     olympus2Id,       0x2300    },
-        { Tag::root, olympusFe4Id,     olympus2Id,       0x2400    },
-        { Tag::root, olympusFe5Id,     olympus2Id,       0x2500    },
-        { Tag::root, olympusFe6Id,     olympus2Id,       0x2600    },
-        { Tag::root, olympusFe7Id,     olympus2Id,       0x2700    },
-        { Tag::root, olympusFe8Id,     olympus2Id,       0x2800    },
-        { Tag::root, olympusFe9Id,     olympus2Id,       0x2900    },
-        { Tag::root, olympusRiId,      olympus2Id,       0x3000    },
+        { Tag::root, olympusEqId,      olympusId,        0x2010    },
+        { Tag::root, olympusCsId,      olympusId,        0x2020    },
+        { Tag::root, olympusRdId,      olympusId,        0x2030    },
+        { Tag::root, olympusRd2Id,     olympusId,        0x2031    },
+        { Tag::root, olympusIpId,      olympusId,        0x2040    },
+        { Tag::root, olympusFiId,      olympusId,        0x2050    },
+        { Tag::root, olympusFe1Id,     olympusId,        0x2100    },
+        { Tag::root, olympusFe2Id,     olympusId,        0x2200    },
+        { Tag::root, olympusFe3Id,     olympusId,        0x2300    },
+        { Tag::root, olympusFe4Id,     olympusId,        0x2400    },
+        { Tag::root, olympusFe5Id,     olympusId,        0x2500    },
+        { Tag::root, olympusFe6Id,     olympusId,        0x2600    },
+        { Tag::root, olympusFe7Id,     olympusId,        0x2700    },
+        { Tag::root, olympusFe8Id,     olympusId,        0x2800    },
+        { Tag::root, olympusFe9Id,     olympusId,        0x2900    },
+        { Tag::root, olympusRiId,      olympusId,        0x3000    },
         { Tag::root, fujiId,           exifId,           0x927c    },
         { Tag::root, canonId,          exifId,           0x927c    },
         { Tag::root, canonCsId,        canonId,          0x0001    },
@@ -1317,26 +1317,26 @@ namespace Exiv2 {
         {  Tag::all, olympusId,        newTiffEntry                              },

         // Olympus2 makernote
-        {    0x0001, olympus2Id,       EXV_SIMPLE_BINARY_ARRAY(minoCsoCfg)       },
-        {    0x0003, olympus2Id,       EXV_SIMPLE_BINARY_ARRAY(minoCsnCfg)       },
-        {    0x2010, olympus2Id,       newTiffSubIfd<olympusEqId>                },
-        {    0x2020, olympus2Id,       newTiffSubIfd<olympusCsId>                },
-        {    0x2030, olympus2Id,       newTiffSubIfd<olympusRdId>                },
-        {    0x2031, olympus2Id,       newTiffSubIfd<olympusRd2Id>               },
-        {    0x2040, olympus2Id,       newTiffSubIfd<olympusIpId>                },
-        {    0x2050, olympus2Id,       newTiffSubIfd<olympusFiId>                },
-        {    0x2100, olympus2Id,       newTiffSubIfd<olympusFe1Id>               },
-        {    0x2200, olympus2Id,       newTiffSubIfd<olympusFe2Id>               },
-        {    0x2300, olympus2Id,       newTiffSubIfd<olympusFe3Id>               },
-        {    0x2400, olympus2Id,       newTiffSubIfd<olympusFe4Id>               },
-        {    0x2500, olympus2Id,       newTiffSubIfd<olympusFe5Id>               },
-        {    0x2600, olympus2Id,       newTiffSubIfd<olympusFe6Id>               },
-        {    0x2700, olympus2Id,       newTiffSubIfd<olympusFe7Id>               },
-        {    0x2800, olympus2Id,       newTiffSubIfd<olympusFe8Id>               },
-        {    0x2900, olympus2Id,       newTiffSubIfd<olympusFe9Id>               },
-        {    0x3000, olympus2Id,       newTiffSubIfd<olympusRiId>                },
-        { Tag::next, olympus2Id,       ignoreTiffComponent                       },
-        {  Tag::all, olympus2Id,       newTiffEntry                              },
+        {    0x0001, omsystemId,       EXV_SIMPLE_BINARY_ARRAY(minoCsoCfg)       },
+        {    0x0003, omsystemId,       EXV_SIMPLE_BINARY_ARRAY(minoCsnCfg)       },
+        {    0x2010, omsystemId,       newTiffSubIfd<olympusEqId>                },
+        {    0x2020, omsystemId,       newTiffSubIfd<olympusCsId>                },
+        {    0x2030, omsystemId,       newTiffSubIfd<olympusRdId>                },
+        {    0x2031, omsystemId,       newTiffSubIfd<olympusRd2Id>               },
+        {    0x2040, omsystemId,       newTiffSubIfd<olympusIpId>                },
+        {    0x2050, omsystemId,       newTiffSubIfd<olympusFiId>                },
+        {    0x2100, omsystemId,       newTiffSubIfd<olympusFe1Id>               },
+        {    0x2200, omsystemId,       newTiffSubIfd<olympusFe2Id>               },
+        {    0x2300, omsystemId,       newTiffSubIfd<olympusFe3Id>               },
+        {    0x2400, omsystemId,       newTiffSubIfd<olympusFe4Id>               },
+        {    0x2500, omsystemId,       newTiffSubIfd<olympusFe5Id>               },
+        {    0x2600, omsystemId,       newTiffSubIfd<olympusFe6Id>               },
+        {    0x2700, omsystemId,       newTiffSubIfd<olympusFe7Id>               },
+        {    0x2800, omsystemId,       newTiffSubIfd<olympusFe8Id>               },
+        {    0x2900, omsystemId,       newTiffSubIfd<olympusFe9Id>               },
+        {    0x3000, omsystemId,       newTiffSubIfd<olympusRiId>                },
+        { Tag::next, omsystemId,       ignoreTiffComponent                       },
+        {  Tag::all, omsystemId,       newTiffEntry                              },

         // Olympus2 equipment subdir
         {  Tag::all, olympusEqId,        newTiffEntry                            },

[tenzap]

I actually don't use the exiv2 binary but add thumbnails programmatically through my android app which builds exiv2 from sources to use it.

With this patch, the tests with my app report this warning: Invalid ifdId 0. If I bypass the warning and still process the file, the picture doesn't actually seem to be modified at all. Others get modified though, but not this one.

Maybe I missed something...

[clanmills]

@tenzap. Thanks that's valuable feedback. So you're using libexiv2 on an android device. Well done. I've never programmed 'native' apps on a mobile platform. Andreas (the founder of Exiv2) reported success with NDK about 10 years ago.

I hope to get this issue fixed by the weekend and then I'll prepare the PR with the code and updates to the test suite. With your permission, I'll extract the metadata from 42_IndexError.jpg and add it to the test suite (and a couple of other OLYMPUS files). It usually takes about one or two weeks for a PR to go through review.

The 0.27-maintenance change that resulted in this this issue has not been ported to main. Groan. Another job.

[kmilos]

Hi Robin, sorry your retirement got interrupted by my change, but hope this is still providing some challenging fun ;) In any case, may I suggest we again just keep adding anything related to "OM System" MakerNotes rather than deleting/renaming existing Olympus stuff?

[clanmills]

@kmilos. Thanks. I'm having fun with this. Software puzzles are fascinating. I'm a gladiator. I fight lions and wild beasts. None wilder than the mysterious 10,000 line makernote parser in exiv2/src/tiff*. I don't know how Andreas thought of something so complicated. That code is very robust and has stood the test of time.

There's almost nothing to be changed here. The code's a little out of balance, so the file 42_IndexError.jpg ends up in the OMSystem parser by mistake.

You get on with your paid work while I figure this out. When the PR is ready, you'll simplify my proposed changes.

[clanmills]

I'm going to walk away from this. @tenzap has a working solution using the 0.27.5 code. I think the lion has defeated me and @kmilos will kill him later.

We know that when the file 42_... is rewritten, the makernote signature has morphed from OLYMPUS\0II to OM SYSTEM\0.... Here's the proof:

953 rmills@rmillsm1:~/gnu/github/exiv2 $ tvisitor -pR ~/temp/42_IndexError.jpg.orig | grep -i makernote
         388 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |    10368 |      3208 | OLYMPUS_II._.__.._.___..__..._ ___(. +++
954 rmills@rmillsm1:~/gnu/github/exiv2 $ tvisitor -pR ~/temp/42_IndexError.jpg | grep -i makernote
        1150 | 0x927c Exif.Photo.MakerNote             | UNDEFINED |     9412 |      1450 | OM SYSTEM___II._.__.._.___j___..._ _ +++
955 rmills@rmillsm1:~/gnu/github/exiv2 $ 

There's very little wrong here. @kmilos is correct. We should extend existing machinery. I suspect the fix involves adding omsystemId to enum IfdId intags_int.hpp and modify TiffMnCreator::registry_ appropriately. This is what I unsuccessfully attempted in the patch above.

    const TiffMnRegistry TiffMnCreator::registry_[] = {
        { "Canon",          canonId,     newIfdMn,       newIfdMn2       },
...
        { "OLYMPUS",        ifdIdNotSet, newOlympusMn,   0               }, // mnGroup_ is not used
        { "OM Digital",     olympus2Id,  newOMSystemMn,  newOMSystemMn2  },
...
        { "CASIO",          ifdIdNotSet, newCasioMn,     0               }, // mnGroup_ is not used
        // Entries below are only used for lookup by group
        { "-",              nikon1Id,    0,              newIfdMn2       },
...
        { "-",              olympusId,   0,              newOlympusMn2   },
        { "-",              olympus2Id,  0,              newOlympus2Mn2  },
...
    };

If you'd like to consult with me about this, please email me because I don't monitor the Exiv2 project on github.

[kmilos]

Ok, I finally had a quick look and narrowed it down to:

https://github.com/Exiv2/exiv2/blob/0fa22ed55d1088eacc5ef063b7307fa1b4d031e6/src/makernote_int.cpp#L153

So by supporting the reading of "OM System" makernotes through mapping them to the existing IfdId::olympus2Id group (which was relatively easy to do as the mechanism to go from many options to one is there since the maker string is checked when parsing just a few lines above), I introduced bug on the write side: the first match for that group is now the "OM Digital" registry_ row with function creating the "OM System" header, and there seems to be no mechanism to de-alias this now as the maker string is not used when writing... :/

The obvious option is to keep duplicating everything related to "OM System" makernotes further and create and hook up a new IfdId::omsystemId group as @clanmills started (but that seems to require a lot of meticulous and boring work, ergo can lead to more bugs), or somehow get access to and use the maker string at this point as well (not obvious to me how just yet.)

P.S. Sorry, but I must have tested only with a "non-intrusive" metadata write like -M"set Exif.Image.Copyright 2023" that didn't require recreation of the IFDs and didn't go through this code path...

[kmilos]

Also, I don't know if https://github.com/Exiv2/exiv2/issues/1757 is somehow related as well, but the whole makernote handling seems somewhat complicated and in need of attention...

[kmilos]

A quick 'n' dirty workaround for this also comes to mind: we let the write side change the makernote header, but we support both headers on read regardless of the maker string? Any appetite for that? We could end up w/ slightly "strange" files, but at least the data remains accessible...

[kmilos]

@tenzap I have now backported the said workaround to the MSYS2 0.27.6 package, maybe you can ask Debian maintainers to do the same?

[tenzap]

@kmilos, debian has entered Hard freeze for the next release. Do you think it is worth asking the maintainer to cherry-pick the patch?

[kmilos]

Not familiar w/ their policies, so you'll just have to try I guess... Perhaps report it as an issue first, I don't see it here: https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=exiv2

[tenzap]

For anyone interested: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1034641

[clanmills]

Well done @kmilos, @kevinbackhouse and @tenzap for getting this fixed. I bought an Olympus Tough TG-6 which has GPS. I want to display my photos/gps files over Google Maps on my website. 0.27.6+your fix is working well.

https://clanmills.com/2023/Runs/RidgewayWalk/ I'm pleased the rugged TG-6 camera.

[kmilos]

@tenzap FYI, the above mentioned workaround has been released in 0.27.7

[benmccann]

@tenzap also FYI, the workaround has also been released in 0.28.0. That may be the better version for Debian to upgrade to as users will get a larger selection of new features and bug fixes. You might want to update https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1034641 to make the maintainers aware

[tenzap]

Thanks for the information but as expected these won't be in the next Debian release due in a couple of weeks. The patches are coming much too late I guess. Debian has been in freeze for a few months now. https://release.debian.org/testing/freeze_policy.html If you think it is really worth trying, please get yourself in touch directly with the maintainer, this will be much more efficient, you can argue because you have the knowledge. I really wouldn't bring any value here, sorry

[benmccann]

I won't try to get it into this next Debian if it's frozen. Still I wonder if you might want to try contacting the maintainers to get it into the subsequent release

[tenzap]

I'm not a user of exiv2 on Debian. I use it for an Android app and just do some cross checks with exiv2 in debian in case I face bugs in my app. So it's up to you for Debian. 😉

[kmilos]

So it's up to you for Debian.

No, it's up to a Debian user who has "skin in the game".

And it is not impossible for a patched 0.27.6, or 0.27.7 that is also now available and includes this workaround, to be backported as a bugfix update to Debian 12 in the future. It just takes time on their side.

OTOH, it is nearly impossible to ship 0.28.x as it breaks API and other dependent packages that are unlikely to be bumped as mere bugfix updates....

Again, distro packaging is not the focus and responsibility of Team Exiv2, but of its respective maintainers and users. We've done what we can to provide a workaround and a bugfix release.