Rawtherapee: Multi-frame handling

[Hombre57]

This is the place to discuss the multiframe-handling branch. See my comment added to the commit f23be9345cbd45e51b64675438637024ad22f9a2 and let's discuss what's wrong in this patch (I renamed some classes to make them more self-explicit), and what's missing.

[Floessie]

It's actually f23be9345cbd45e51b64675438637024ad22f9a2. I'll only comment on the coding aspects.

[Hombre57]

@Floessie Sorry, comment fixed. You're coding advice will be much appreciated.

[Hombre57]

@Floessie @Beep6581 Thanks for the review. I'm done with the fix requested (see commit 1a296b763f0b3f032a7115e9cfb112918d3b9851). I still have to add icons on thumbnails and it will be complete.

[Floessie]

Good job, @Hombre57!

Only nitpicking:

• Shouldn't IsRGBSourceModified() be isRGBSourceModified()?
• You removed const from virtual rtexif::TagDirectory* getExifData (unsigned int frame = 0) const = 0;, but if it compiled correctly before, it makes sense in this place, because it guarantees, noone will change the TagDirectory state from outside. If you return a value (not a reference/pointer) though, the const makes no sense, because the value (as a temporary) can be copy-assigned to a non-const variable. To conclude, taking the example above and adding some pointer fun:
  
  
  
  • Wrong: virtual rtexif::TagDirectory* const getExifData (unsigned int frame = 0) const
  
  
  • Right: virtual const rtexif::TagDirectory* getExifData (unsigned int frame = 0) const
  
  
• Your use of std::unique_ptr<rtengine::RawMetaDataLocation> rml(new rtengine::RawMetaDataLocation(0)); in ExifManager::parseTIFF() shadows the member variable. I was rather thinking about turning the member rml into a std::unique_ptr<> and using std::unique_ptr<>::reset() in that very place, thus preserving the previous logic. The code as-is won't work, because parse() needs the rml member. Here's a patch:

diff --git a/rtengine/dfmanager.cc b/rtengine/dfmanager.cc
index 68123d9b..1ce43637 100644
--- a/rtengine/dfmanager.cc
+++ b/rtengine/dfmanager.cc
@@ -382,8 +382,7 @@ dfInfo* DFManager::addFileInfo (const Glib::ustring& filename, bool pool)
             return &(iter->second);
         }

-        RawMetaDataLocation rml(ri.get_exifBase(), ri.get_ciffBase(), ri.get_ciffLen());
-        FramesData idata(filename, &rml, true);
+        FramesData idata(filename, std::unique_ptr<RawMetaDataLocation>(new RawMetaDataLocation(ri.get_exifBase(), ri.get_ciffBase(), ri.get_ciffLen())), true);
         /* Files are added in the map, divided by same maker/model,ISO and shutter*/
         std::string key( dfInfo::key(((Glib::ustring)idata.getMake()).uppercase(), ((Glib::ustring)idata.getModel()).uppercase(), idata.getISOSpeed(), idata.getShutterSpeed()) );
         iter = dfList.find( key );
diff --git a/rtengine/ffmanager.cc b/rtengine/ffmanager.cc
index 6acdc1b0..8f8d928c 100644
--- a/rtengine/ffmanager.cc
+++ b/rtengine/ffmanager.cc
@@ -338,8 +338,7 @@ ffInfo* FFManager::addFileInfo (const Glib::ustring& filename, bool pool)
             return &(iter->second);
         }

-        RawMetaDataLocation rml(ri.get_exifBase(), ri.get_ciffBase(), ri.get_ciffLen());
-        FramesData idata(filename, &rml, true);
+        FramesData idata(filename, std::unique_ptr<RawMetaDataLocation>(new RawMetaDataLocation(ri.get_exifBase(), ri.get_ciffBase(), ri.get_ciffLen())), true);
         /* Files are added in the map, divided by same maker/model,lens and aperture*/
         std::string key( ffInfo::key(idata.getMake(), idata.getModel(), idata.getLens(), idata.getFocalLen(), idata.getFNumber()) );
         iter = ffList.find( key );
diff --git a/rtengine/imagedata.cc b/rtengine/imagedata.cc
index 9c27ddd5..66cd3874 100644
--- a/rtengine/imagedata.cc
+++ b/rtengine/imagedata.cc
@@ -41,9 +41,9 @@ Glib::ustring to_utf8 (const std::string& str)

 }

-FramesMetaData* FramesMetaData::fromFile (const Glib::ustring& fname, RawMetaDataLocation* rml, bool firstFrameOnly)
+FramesMetaData* FramesMetaData::fromFile (const Glib::ustring& fname, std::unique_ptr<RawMetaDataLocation> rml, bool firstFrameOnly)
 {
-    return new FramesData (fname, rml, firstFrameOnly);
+    return new FramesData (fname, std::move(rml), firstFrameOnly);
 }

 FrameData::FrameData ()
@@ -1000,18 +1000,20 @@ failure:

 }

-FramesData::FramesData (Glib::ustring fname, RawMetaDataLocation* rml, bool firstFrameOnly, bool loadAll) : dcrawFrameCount (0)
+FramesData::FramesData (const Glib::ustring& fname, std::unique_ptr<RawMetaDataLocation> rml, bool firstFrameOnly, bool loadAll) :
+    dcrawFrameCount (0)
 {
     if (rml && (rml->exifBase >= 0 || rml->ciffBase >= 0)) {
         FILE* f = g_fopen (fname.c_str (), "rb");

         if (f) {
-            rtexif::ExifManager exifManager (f, rml, firstFrameOnly);
+            const bool has_rml_exif_base = rml->exifBase >= 0;
+            rtexif::ExifManager exifManager (f, std::move(rml), firstFrameOnly);

-            if (rml->exifBase >= 0) {
+            if (has_rml_exif_base) {
                 FrameData *idata = new RawFrameData (exifManager);
                 frames.push_back(idata);
-                if (rml && !firstFrameOnly) {
+                if (!firstFrameOnly) {
                     while (exifManager.getNextIFDOffset ()) {
                         int nextIFD = exifManager.getNextIFDOffset ();
                         exifManager.setIFDOffset (nextIFD);
@@ -1026,7 +1028,8 @@ FramesData::FramesData (Glib::ustring fname, RawMetaDataLocation* rml, bool firs
         FILE* f = g_fopen (fname.c_str (), "rb");

         if (f) {
-            rtexif::ExifManager exifManager (f, rml, true);
+            rtexif::ExifManager exifManager (f, std::move(rml), true);
+
             FrameData *idata = new JpegFrameData (exifManager);
             frames.push_back(idata);
             fclose (f);
@@ -1035,10 +1038,12 @@ FramesData::FramesData (Glib::ustring fname, RawMetaDataLocation* rml, bool firs
         FILE* f = g_fopen (fname.c_str (), "rb");

         if (f) {
-            rtexif::ExifManager exifManager (f, rml, firstFrameOnly);
+            const bool has_rml = static_cast<bool>(rml);
+            rtexif::ExifManager exifManager (f, std::move(rml), firstFrameOnly);
+
             FrameData *idata = new TiffFrameData (exifManager);
             frames.push_back(idata);
-            if (rml && !firstFrameOnly) {
+            if (has_rml && !firstFrameOnly) {
                 while (exifManager.getNextIFDOffset ()) {
                     exifManager.setIFDOffset (exifManager.getNextIFDOffset ());
                     idata = new TiffFrameData (exifManager);
diff --git a/rtengine/imagedata.h b/rtengine/imagedata.h
index 7474ecab..f8ba685c 100644
--- a/rtengine/imagedata.h
+++ b/rtengine/imagedata.h
@@ -20,12 +20,17 @@
 #define __IMAGEDATA_H__

 #include <cstdio>
-#include "rawimage.h"
+#include <memory>
 #include <string>
+
 #include <glibmm.h>
+
+#include <libiptcdata/iptc-data.h>
+
 #include "../rtexif/rtexif.h"
+
 #include "procparams.h"
-#include <libiptcdata/iptc-data.h>
+#include "rawimage.h"
 #include "rtengine.h"

 namespace rtengine
@@ -111,7 +116,7 @@ private:
     unsigned int dcrawFrameCount;

 public:
-    FramesData (Glib::ustring fname, RawMetaDataLocation* rml = nullptr, bool firstFrameOnly = false, bool loadAll = false);
+    FramesData (const Glib::ustring& fname, std::unique_ptr<RawMetaDataLocation> rml = nullptr, bool firstFrameOnly = false, bool loadAll = false);
     ~FramesData ();

     void setDCRawFrameCount (unsigned int frameCount);
diff --git a/rtengine/rawimagesource.cc b/rtengine/rawimagesource.cc
index 6641cf51..2289a95d 100644
--- a/rtengine/rawimagesource.cc
+++ b/rtengine/rawimagesource.cc
@@ -1698,12 +1698,12 @@ int RawImageSource::load (const Glib::ustring &fname, int imageNum, bool batch)
     }


-    //Load complete Exif informations
-    RawMetaDataLocation rml;
-    rml.exifBase = ri->get_exifBase();
-    rml.ciffBase = ri->get_ciffBase();
-    rml.ciffLength = ri->get_ciffLen();
-    idata = new FramesData (fname, &rml);
+    // Load complete Exif informations
+    std::unique_ptr<RawMetaDataLocation> rml(new RawMetaDataLocation);
+    rml->exifBase = ri->get_exifBase();
+    rml->ciffBase = ri->get_ciffBase();
+    rml->ciffLength = ri->get_ciffLen();
+    idata = new FramesData (fname, std::move(rml));
     idata->setDCRawFrameCount (numFrames);

     green(W, H);
diff --git a/rtengine/rtengine.h b/rtengine/rtengine.h
index 8e6e8430..8f5a490d 100644
--- a/rtengine/rtengine.h
+++ b/rtengine/rtengine.h
@@ -131,7 +131,7 @@ public:
       * Use it only for raw files. In caseof jpgs and tiffs pass a NULL pointer.
       * @param firstFrameOnly must be true to get the MetaData of the first frame only, e.g. for a PixelShift file.
       * @return The metadata */
-    static FramesMetaData* fromFile (const Glib::ustring& fname, RawMetaDataLocation* rml, bool firstFrameOnly = false);
+    static FramesMetaData* fromFile (const Glib::ustring& fname, std::unique_ptr<RawMetaDataLocation> rml, bool firstFrameOnly = false);
 };

 /** This listener interface is used to indicate the progress of time consuming operations */
diff --git a/rtexif/rtexif.cc b/rtexif/rtexif.cc
index b242c2cb..441d017d 100644
--- a/rtexif/rtexif.cc
+++ b/rtexif/rtexif.cc
@@ -852,11 +852,11 @@ Tag::Tag (TagDirectory* p, FILE* f, int base)
     }

     if (tag == 0x002e) { // location of the embedded preview image in raw files of Panasonic cameras
-        TagDirectory* previewdir;
-        {
-        ExifManager exifManager(f, 0, true);
-        previewdir = exifManager.parseJPEG (ftell (f)); // try to parse the exif data from the preview image
-        }
+        const TagDirectory* const previewdir =
+            [&f]()
+            {
+                return ExifManager(f, nullptr, true).parseJPEG(ftell(f)); // try to parse the exif data from the preview image
+            }();

         if (previewdir) {
             if (previewdir->getTag ("Exif")) {
@@ -1977,9 +1977,16 @@ void ExifManager::parseCIFF (int length, TagDirectory* root)
         fseek (f, rml->ciffBase + get4 (f, INTEL), SEEK_SET);

         if ((((type >> 8) + 8) | 8) == 0x38) {
-            rtengine::RawMetaDataLocation rml2(ftell (f), len);
-            ExifManager exifManager(f, &rml2, true);
-            exifManager.parseCIFF (len, root);   // Parse a sub-table
+            ExifManager(
+                f,
+                std::unique_ptr<rtengine::RawMetaDataLocation>(
+                    new rtengine::RawMetaDataLocation(
+                        ftell(f),
+                        len
+                    )
+                ),
+                true
+            ).parseCIFF(len, root); // Parse a sub-table
         }

         if (type == 0x0810) {
@@ -2010,7 +2017,7 @@ void ExifManager::parseCIFF (int length, TagDirectory* root)

         }

-        ExifManager exifManager(f, 0, true);
+        ExifManager exifManager(f, nullptr, true);
         if (type == 0x102d) {
             Tag* t = exifManager.saveCIFFMNTag (root, len, "CanonCameraSettings");
             int mm = t->toInt (34, SHORT);
@@ -2856,16 +2863,14 @@ TagDirectory* ExifManager::parseJPEG (int offset)
                         tiffbase = ftell (f);

                         // We need a RawMetaDataLocation to put the 'tiffbase' value
-                        bool rmlCreated = false;
-                        if (!rml) {
-                            rml = new rtengine::RawMetaDataLocation (0);
-                            rmlCreated = true;
+                        const bool rmlCreated = !rml;
+                        if (rmlCreated) {
+                            rml.reset(new rtengine::RawMetaDataLocation(0));
                         }
                         rml->exifBase = tiffbase;
                         TagDirectory*  tagDir = parse ();
                         if (rmlCreated) {
-                            delete rml;
-                            rml = nullptr;
+                            rml.reset();
                         }
                         return tagDir;
                     }
@@ -2879,10 +2884,11 @@ TagDirectory* ExifManager::parseJPEG (int offset)

 TagDirectory* ExifManager::parseTIFF (bool skipIgnored)
 {
-
     if (!rml) {
-        std::unique_ptr<rtengine::RawMetaDataLocation> rml(new rtengine::RawMetaDataLocation(0));
-        return parse (skipIgnored);
+        rml.reset(new rtengine::RawMetaDataLocation(0));
+        TagDirectory* const res = parse(skipIgnored);
+        rml.reset();
+        return res;
     } else {
         return parse (skipIgnored);
     }
diff --git a/rtexif/rtexif.h b/rtexif/rtexif.h
index 5821c58f..bc949518 100644
--- a/rtexif/rtexif.h
+++ b/rtexif/rtexif.h
@@ -19,14 +19,16 @@
 #ifndef _MEXIF3_
 #define _MEXIF3_

+#include <cmath>
 #include <cstdio>
-#include <vector>
+#include <cstdlib>
+#include <iomanip>
 #include <map>
-#include <string>
+#include <memory>
 #include <sstream>
-#include <iomanip>
-#include <cstdlib>
-#include <cmath>
+#include <string>
+#include <vector>
+
 #include <glibmm.h>

 #include "../rtengine/procparams.h"
@@ -316,14 +318,14 @@ class ExifManager

 public:
     FILE* f;
-    rtengine::RawMetaDataLocation *rml;
+    std::unique_ptr<rtengine::RawMetaDataLocation> rml;
     ByteOrder order;
     bool onlyFirst;  // Only first IFD
     unsigned int IFDOffset;
     unsigned int nextIFDOffset;

-    ExifManager (FILE* fHandle, rtengine::RawMetaDataLocation *rml, bool onlyFirstIFD)
-        : f(fHandle), rml(rml), order(UNKNOWN), onlyFirst(onlyFirstIFD),
+    ExifManager (FILE* fHandle, std::unique_ptr<rtengine::RawMetaDataLocation> _rml, bool onlyFirstIFD)
+        : f(fHandle), rml(std::move(_rml)), order(UNKNOWN), onlyFirst(onlyFirstIFD),
           IFDOffset(0), nextIFDOffset(0) {}

     void setIFDOffset(unsigned int offset)
diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc
index 320424bf..283f80be 100644
--- a/rtgui/thumbnail.cc
+++ b/rtgui/thumbnail.cc
@@ -145,7 +145,7 @@ void Thumbnail::_generateThumbnailImage ()
         if (tpp) {
             cfs.format = FT_Raw;
             cfs.thumbImgType = quick ? CacheImageData::QUICK_THUMBNAIL : CacheImageData::FULL_THUMBNAIL;
-            infoFromImage (fname, &ri);
+            infoFromImage (fname, std::unique_ptr<rtengine::RawMetaDataLocation>(new rtengine::RawMetaDataLocation(ri)));
         }
     }

@@ -229,9 +229,8 @@ rtengine::procparams::ProcParams* Thumbnail::createProcParamsForUpdate(bool retu
         if (defProf == DEFPROFILE_DYNAMIC && create && cfs && cfs->exifValid) {
             rtengine::FramesMetaData* imageMetaData;
             if (getType() == FT_Raw) {
-                rtengine::RawMetaDataLocation metaData = rtengine::Thumbnail::loadMetaDataFromRaw(fname);
                 // Should we ask all frame's MetaData ?
-                imageMetaData = rtengine::FramesMetaData::fromFile (fname, &metaData, true);
+                imageMetaData = rtengine::FramesMetaData::fromFile (fname, std::unique_ptr<rtengine::RawMetaDataLocation>(new rtengine::RawMetaDataLocation(rtengine::Thumbnail::loadMetaDataFromRaw(fname))), true);
             } else {
                 // Should we ask all frame's MetaData ?
                 imageMetaData = rtengine::FramesMetaData::fromFile (fname, nullptr, true);
@@ -255,9 +254,8 @@ rtengine::procparams::ProcParams* Thumbnail::createProcParamsForUpdate(bool retu
         rtengine::FramesMetaData* imageMetaData;

         if (getType() == FT_Raw) {
-            rtengine::RawMetaDataLocation metaData = rtengine::Thumbnail::loadMetaDataFromRaw(fname);
             // Should we ask all frame's MetaData ?
-            imageMetaData = rtengine::FramesMetaData::fromFile (fname, &metaData, true);
+            imageMetaData = rtengine::FramesMetaData::fromFile (fname, std::unique_ptr<rtengine::RawMetaDataLocation>(new rtengine::RawMetaDataLocation(rtengine::Thumbnail::loadMetaDataFromRaw(fname))), true);
         } else {
             // Should we ask all frame's MetaData ?
             imageMetaData = rtengine::FramesMetaData::fromFile (fname, nullptr, true);
@@ -713,10 +711,9 @@ ThFileType Thumbnail::getType ()
     return (ThFileType) cfs.format;
 }

-int Thumbnail::infoFromImage (const Glib::ustring& fname, rtengine::RawMetaDataLocation* rml)
+int Thumbnail::infoFromImage (const Glib::ustring& fname, std::unique_ptr<rtengine::RawMetaDataLocation> rml)
 {
-
-    rtengine::FramesMetaData* idata = rtengine::FramesMetaData::fromFile (fname, rml);
+    rtengine::FramesMetaData* idata = rtengine::FramesMetaData::fromFile (fname, std::move(rml));

     if (!idata) {
         return 0;
diff --git a/rtgui/thumbnail.h b/rtgui/thumbnail.h
index eb9e38f7..70bd6079 100644
--- a/rtgui/thumbnail.h
+++ b/rtgui/thumbnail.h
@@ -70,7 +70,7 @@ class Thumbnail
     void            _loadThumbnail (bool firstTrial = true);
     void            _saveThumbnail ();
     void            _generateThumbnailImage ();
-    int             infoFromImage (const Glib::ustring& fname, rtengine::RawMetaDataLocation* rml = nullptr);
+    int             infoFromImage (const Glib::ustring& fname, std::unique_ptr<rtengine::RawMetaDataLocation> rml = nullptr);
     void            loadThumbnail (bool firstTrial = true);
     void            generateExifDateTimeStrings ();

HTH,
Flössie

[Hombre57]

Thanks for the help @Floessie. As soon as I get out of my habits and road, I'm way less productive :sweat: ... I'll test and commit tonight.

[Floessie]

@Hombre57 It's just a suggestion. If you feel more comfortable without the std::unique_ptr<> then keep it as it was. There's currently only the bug with the local rml in ExifManager::parseTIFF() which you should revert to the previous version if you want to stick with the plain pointer. 👍

[Hombre57]

@Floessie Even though it's quite complex and unusual for me, I committed your patch at least to have some reference code, just in case ;)

[Beep6581]

@Hombre57 could you include SVG versions of the icons in your branch? Remember to first convert the text to paths so that there is no font dependency.

[Hombre57]

They're already included, text already converted.

[Beep6581]

Ah yes, I missed them, thank you.

[Hombre57]

Fixed a bug already predicted by @Floessie. I wasn't expecting that some images wouldn't contain exif, which is a silly assumption of midnight work...

[Hombre57]

How should we handle the demozaicing method which could be set to pixelshift for whatever reason on an image that is not PS ? I know that the engine will switch to Amaze or lmmse, but I have a patch ready that remove the Pixeh Shift entry for non PS images in the combobox. However in that case, it will be set to _inconsistent_. When closing the image, if the user doesn't set a valid entry, it will remain set to pixelshift.

Should we fix the procparams somewhere to revert the demozaicing method to a valid one, or do we agree with having _inconsistent_ combobox, or should I remove my patch (no problem for me) ?

[Beep6581]

I like the idea of the PP3 being preserved, but if I understood the explanation correctly, that would lead to the problem of the user interface not being explicit to the user about what's happening.

If the PP3 uses demosaicing method Pixel Shift but the opened image is not a Pixel Shift raw, and the combobox says "Pixel Shift" or "Inconsistent", the user wouldn't know which demosaicing method is being used - better in that case for the combobox to show "AMaZE" if RT switched to AMaZE.

[Hombre57]

@Beep6581 The more I look at it, the more if feels tricky to implement : we have to care about copy/paste of ProcParams to thumbnails of images that can already be opened. I'd like to postpone this part (or drop it entirely) and commit the branch in its actual state (which is complete for me), after quick review of @heckflosse too.

[heckflosse]

@Hombre57 I will review it asap

[Hombre57]

As reported by @heckflosse in #3989, the branch doesn't work for PEF files. Believe it or not but I haven't tested them at all, I'm personally using DNG and I completely put PEF files out of my mind.

PEF files are structured differently, i.e. frame 2, 3 and 4 are embedded in frame 1 (yes, they did that), while DNG files are logically structured with 4 root IFDs.

It will take more time to fix than expected, I think it should be done for next Sunday.

The wrong image size in EditorPanel's info area is fixed. However I have yet to find out why PS files are reported as HDR files.

[heckflosse]

@Hombre57

PEF files are structured differently, i.e. frame 2, 3 and 4 are embedded in frame 1 (yes, they did that), while DNG files are logically structured with 4 root IFDs.

I can try to help you for this part.

[Hombre57]

@heckflosse Thanks for the offer ! I'll try by myself, but if too complex for me, I'll ring at your door.

[Hombre57]

Something went wrong with the merge of dev into multiframe-handling. PEF's PS files that was correctly recognized before the merge now doesn't work (works fine for DNG's PS files). I'll try to solve that before the end of Sunday.

[Hombre57]

@Beep6581 @heckflosse @Floessie

The branch should be usable now. I only need to test with various file to see if the EditorPanel's Info display contains at least something (no "No exif" message) for all kind of files - and wouldn't mind some help on that front.

I still have to find out how to handle Exif & IPTC modification. Logically, the whole metadata tree should be saved in the output file. Don't know if that's what we want.

@Floessie I saw that you removed std::move from the code (only some of them). Should the one you introduced in the multiframe-handling branch be removed as well ? They compile fine and seem to work.

[Floessie]

@Hombre57

I saw that you removed std::move from the code (only some of them). Should the one you introduced in the multiframe-handling branch be removed as well ? They compile fine and seem to work.

No, they shouldn't be removed. I was a bit overzealous with std::move on the lensfun branch. You don't need std::move to move a temporary, but you definitly need it when the value is bound to a variable, and that's the case in the above patch. So, all should be fine. 😃

[Beep6581]

I still have to find out how to handle Exif & IPTC modification. Logically, the whole metadata tree should be saved in the output file. Don't know if that's what we want.

Not sure what you're asking. Saving one JPEG from 4 pixel-shift frames should just copy over the metadata from either of them, amended with the user's metadata changes if tunnelMetaData is enabled.

There are related issues:

• On the front-end side, there is #3647 "Move "Copy metadata unchanged" to Metadata tab and to PP3" which I struggled with yesterday but didn't get anywhere yet. It would have the additional option of wiping all metadata from the saved file.
• On the back-end side there is #3801 "Adopt libexiv2 or ExifTool for metadata handling".

[Hombre57]

@Beep6581 @heckflosse @Floessie I'm in the testing phase. My branch doesn't save correctly the special chars for IPTC data (i.e. "éàêç" etc), but I guess it's there in dev too. There's a function named iptc_data_set_encoding_utf8 that could help for UTF-8 support I guess. Is there any reason why we don't use it yet ?

[heckflosse]

@Hombre57 I might be able to speedup the function you mentioned, but I've no idea what it does at all ;-)

[Floessie]

@Hombre57 If I read the documentation correctly, iptc-data-set-encoding-utf8 kind of tags the data you provided as being encoded in UTF-8. Have you checked (with a hex editor) that this really is the case? Only then it would indeed make sense to give iptc-data-set-encoding-utf8 a try.

[Floessie]

@Beep6581 Sorry for this dumb question, but how is saving IPTC data supposed to work? I edited a JPG and set some IPTC fields. Then i invoked "Save immediately" to a new JPG. Tested with exiv2 -p i: No IPTC. Read RawPedia, tried the "Reset" button, saved again to a new JPG, no IPTC, no clue. 😕

What am I missing?

[Beep6581]

There are bigger problems than just special chars. For one, we use libiptcdata which has been dead since 2009 which prevents us from updating RT's IPTC implementation to IPTC Core 1.2. See #3581.
For another, IPTC itself while still being used is made obsolete by XMP. I may be wrong, but I see it as being maintained on life-support.
https://www.iptc.org/std/photometadata/specification/IPTC-PhotoMetadata#history

@Floessie is "Copy Exif/IPTC/XMP unchanged to the output file" unchecked? See #3647

[Floessie]

@Beep6581 That's it. 👍 We talked about libexiv2 integration, I know it's high on your agenda, but that's going to be a big endeavor which I cannot accomplish with my "drive-by" coding ATM. 😞

@Hombre57 I can confirm, that all fields are written in UTF-8. Only the "Category" is odd in just accepting 3 codepoints but writing out exactly 3 bytes, thus truncating UTF-8 encoded codepoints.

[Hombre57]

I think I'm done with this branch. Waiting for a merge approval.

[Hombre57]

I guess we can close now.