From 105517c56112aa2170c2b9903640ad2aaf870891 Mon Sep 17 00:00:00 2001 From: lewiatan Date: Fri, 16 Feb 2018 22:02:00 +0100 Subject: [PATCH 1/5] support rating tag from EXIF/XMP --- rtengine/imagedata.cc | 17 ++++++++++++++++- rtengine/imagedata.h | 5 ++++- rtengine/rtengine.h | 2 ++ rtexif/rtexif.cc | 6 ++++++ rtgui/cacheimagedata.cc | 2 +- rtgui/cacheimagedata.h | 2 ++ rtgui/thumbnail.cc | 1 + rtgui/thumbnail.h | 5 ++++- 8 files changed, 36 insertions(+), 4 deletions(-) diff --git a/rtengine/imagedata.cc b/rtengine/imagedata.cc index 4e38c612a..a84d6423c 100644 --- a/rtengine/imagedata.cc +++ b/rtengine/imagedata.cc @@ -71,7 +71,7 @@ FramesMetaData* FramesMetaData::fromFile (const Glib::ustring& fname, std::uniqu FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* rootDir, rtexif::TagDirectory* firstRootDir) : frameRootDir(frameRootDir_), iptc(nullptr), time(), timeStamp(), iso_speed(0), aperture(0.), focal_len(0.), focal_len35mm(0.), focus_dist(0.f), - shutter(0.), expcomp(0.), make("Unknown"), model("Unknown"), orientation("Unknown"), lens("Unknown"), + shutter(0.), expcomp(0.), make("Unknown"), model("Unknown"), orientation("Unknown"), rating(0), lens("Unknown"), sampleFormat(IIOSF_UNKNOWN), isPixelShift(false), isHDR(false) { memset (&time, 0, sizeof(time)); @@ -97,6 +97,7 @@ FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* serial.clear(); orientation.clear(); lens.clear(); + rating = 0; tag = newFrameRootDir->findTag("Make"); if (!tag) { @@ -187,6 +188,11 @@ FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* orientation = tag->valueToString (); } + tag = newFrameRootDir->findTagUpward("Rating"); + if (tag) { + rating = tag->toInt(); + } + tag = newFrameRootDir->findTagUpward("MakerNote"); rtexif::TagDirectory* mnote = nullptr; if (tag) { @@ -876,6 +882,11 @@ std::string FrameData::getOrientation () const return orientation; } +int FrameData::getRating () const +{ + return rating; +} + void FramesData::setDCRawFrameCount (unsigned int frameCount) @@ -1168,6 +1179,10 @@ std::string FramesData::getOrientation(unsigned int frame) const } ); } +int FramesData::getRating (unsigned int frame) const +{ + return frames.empty() || frame >= frames.size() ? 0 : frames.at(frame)->getRating (); +} //------inherited functions--------------// diff --git a/rtengine/imagedata.h b/rtengine/imagedata.h index c6889e653..b65049e2d 100644 --- a/rtengine/imagedata.h +++ b/rtengine/imagedata.h @@ -49,6 +49,7 @@ protected: std::string make, model, serial; std::string orientation; std::string lens; + int rating; IIOSampleFormat sampleFormat; // each frame has the knowledge of "being an" @@ -84,6 +85,7 @@ public: std::string getLens () const; std::string getSerialNumber () const; std::string getOrientation () const; + int getRating () const; }; class FramesData : public FramesMetaData { @@ -125,7 +127,8 @@ public: std::string getModel (unsigned int frame = 0) const override; std::string getLens (unsigned int frame = 0) const override; std::string getSerialNumber (unsigned int frame = 0) const; - std::string getOrientation (unsigned int frame = 0) const override; + std::string getOrientation (unsigned int frame = 0) const; + int getRating (unsigned int frame = 0) const override; }; diff --git a/rtengine/rtengine.h b/rtengine/rtengine.h index 6264d43ae..8df6e9ef5 100644 --- a/rtengine/rtengine.h +++ b/rtengine/rtengine.h @@ -133,6 +133,8 @@ public: virtual std::string getLens (unsigned int frame = 0) const = 0; /** @return the orientation of the image */ virtual std::string getOrientation (unsigned int frame = 0) const = 0; + /** @return the rating of the image */ + virtual int getRating (unsigned int frame = 0) const = 0; /** @return true if the file is a PixelShift shot (Pentax and Sony bodies) */ virtual bool getPixelShift () const = 0; diff --git a/rtexif/rtexif.cc b/rtexif/rtexif.cc index 571fd4e6b..312cbd1c7 100644 --- a/rtexif/rtexif.cc +++ b/rtexif/rtexif.cc @@ -3037,6 +3037,12 @@ void ExifManager::parse (bool isRaw, bool skipIgnored) } } + if (!root->getTag ("Rating")) { + Tag *t = new Tag (root, root->getAttrib("Rating")); + t->initInt (0, LONG); + root->addTag (t); + } + // --- detecting image root IFD based on SubFileType, or if not provided, on PhotometricInterpretation bool frameRootDetected = false; diff --git a/rtgui/cacheimagedata.cc b/rtgui/cacheimagedata.cc index 35aeb6c91..1c4ac83ea 100644 --- a/rtgui/cacheimagedata.cc +++ b/rtgui/cacheimagedata.cc @@ -27,7 +27,7 @@ CacheImageData::CacheImageData () : md5(""), supported(false), format(FT_Invalid), rankOld(-1), inTrashOld(false), recentlySaved(false), timeValid(false), year(0), month(0), day(0), hour(0), min(0), sec(0), exifValid(false), frameCount(1), - fnumber(0.0), shutter(0.0), focalLen(0.0), focalLen35mm(0.0), focusDist(0.f), iso(0), isHDR (false), + fnumber(0.0), shutter(0.0), focalLen(0.0), focalLen35mm(0.0), focusDist(0.f), iso(0), rating(0), isHDR (false), isPixelShift (false), sensortype(rtengine::ST_NONE), sampleFormat(rtengine::IIOSF_UNKNOWN), redAWBMul(-1.0), greenAWBMul(-1.0), blueAWBMul(-1.0), rotate(0), thumbImgType(0) { diff --git a/rtgui/cacheimagedata.h b/rtgui/cacheimagedata.h index d3aea803b..9de9d186f 100644 --- a/rtgui/cacheimagedata.h +++ b/rtgui/cacheimagedata.h @@ -54,6 +54,7 @@ public: double focalLen, focalLen35mm; float focusDist; unsigned iso; + int rating; bool isHDR; bool isPixelShift; int sensortype; @@ -108,6 +109,7 @@ public: std::string getModel (unsigned int frame = 0) const override { return camModel; } std::string getLens (unsigned int frame = 0) const override { return lens; } std::string getOrientation (unsigned int frame = 0) const override { return ""; } // TODO + int getRating (unsigned int frame = 0) const override { return rating; } // FIXME-piotr : missing rating bool getPixelShift () const override { return isPixelShift; } bool getHDR (unsigned int frame = 0) const override { return isHDR; } std::string getImageType (unsigned int frame) const override { return isPixelShift ? "PS" : isHDR ? "HDR" : "STD"; } diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc index 757708002..e0509f457 100644 --- a/rtgui/thumbnail.cc +++ b/rtgui/thumbnail.cc @@ -813,6 +813,7 @@ int Thumbnail::infoFromImage (const Glib::ustring& fname, std::unique_ptrgetLens(); cfs.camMake = idata->getMake(); cfs.camModel = idata->getModel(); + cfs.rating = idata->getRating(); if (idata->getOrientation() == "Rotate 90 CW") { deg = 90; diff --git a/rtgui/thumbnail.h b/rtgui/thumbnail.h index 0bcdd470a..2c0cba2b0 100644 --- a/rtgui/thumbnail.h +++ b/rtgui/thumbnail.h @@ -164,7 +164,10 @@ public: return cfs.md5; } - int getRank () const; + int getRank () const + { + return cfs.rating; + } void setRank (int rank); int getColorLabel () const; From 7512093c205090608c316cd3e866af9feaa0ce66 Mon Sep 17 00:00:00 2001 From: lewiatan Date: Sat, 17 Feb 2018 22:08:25 +0100 Subject: [PATCH 2/5] support rating tag from EXIF/XMP- fixes --- rtgui/cacheimagedata.cc | 5 +++++ rtgui/thumbnail.h | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/rtgui/cacheimagedata.cc b/rtgui/cacheimagedata.cc index 1c4ac83ea..47318990d 100644 --- a/rtgui/cacheimagedata.cc +++ b/rtgui/cacheimagedata.cc @@ -66,6 +66,10 @@ int CacheImageData::load (const Glib::ustring& fname) rankOld = keyFile.get_integer ("General", "Rank"); } + if (keyFile.has_key ("General", "Rating")) { + rating = keyFile.get_integer ("General", "Rating"); + } + if (keyFile.has_key ("General", "InTrash")) { inTrashOld = keyFile.get_boolean ("General", "InTrash"); } @@ -227,6 +231,7 @@ int CacheImageData::save (const Glib::ustring& fname) keyFile.set_boolean ("General", "Supported", supported); keyFile.set_integer ("General", "Format", format); keyFile.set_boolean ("General", "RecentlySaved", recentlySaved); + keyFile.set_integer ("General", "Rating", rating); // remove the old implementation of Rank and InTrash from cache if (keyFile.has_key ("General", "Rank")) { diff --git a/rtgui/thumbnail.h b/rtgui/thumbnail.h index 2c0cba2b0..75b1ecdd5 100644 --- a/rtgui/thumbnail.h +++ b/rtgui/thumbnail.h @@ -168,7 +168,13 @@ public: { return cfs.rating; } - void setRank (int rank); + void setRank (int rank) + { + if (cfs.rating != rank) { + cfs.rating = rank; + } + pparamsValid = true; // FIXME-piotr Is this the right way to refresh the cache? Probably not since pparams and cache should be 2 different things + } int getColorLabel () const; void setColorLabel (int colorlabel); From 820024972aaf9e3dc92e385bd13da2039a1a826a Mon Sep 17 00:00:00 2001 From: Eric Jiang Date: Thu, 16 May 2019 17:09:46 -0700 Subject: [PATCH 3/5] Use rating from EXIF/XMP where available --- rtengine/imagedata.cc | 10 +++++++++- rtengine/imagedata.h | 2 +- rtengine/procparams.cc | 5 ++++- rtgui/thumbnail.cc | 11 +++++++++-- rtgui/thumbnail.h | 13 ++----------- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/rtengine/imagedata.cc b/rtengine/imagedata.cc index a84d6423c..b9720ec8a 100644 --- a/rtengine/imagedata.cc +++ b/rtengine/imagedata.cc @@ -188,10 +188,18 @@ FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* orientation = tag->valueToString (); } + // Look for Rating metadata in the following order: + // 1. EXIF + // 2. XMP + // 3. pp3 sidecar file tag = newFrameRootDir->findTagUpward("Rating"); - if (tag) { + if (tag && tag->toInt() != 0) { rating = tag->toInt(); } + char sXMPRating[64]; + if (newFrameRootDir->getXMPTagValue("xmp:Rating", sXMPRating)) { + rating = atoi(sXMPRating); + } tag = newFrameRootDir->findTagUpward("MakerNote"); rtexif::TagDirectory* mnote = nullptr; diff --git a/rtengine/imagedata.h b/rtengine/imagedata.h index b65049e2d..097a5cc80 100644 --- a/rtengine/imagedata.h +++ b/rtengine/imagedata.h @@ -48,8 +48,8 @@ protected: double expcomp; std::string make, model, serial; std::string orientation; - std::string lens; int rating; + std::string lens; IIOSampleFormat sampleFormat; // each frame has the knowledge of "being an" diff --git a/rtengine/procparams.cc b/rtengine/procparams.cc index 6d914bb27..f492e5900 100644 --- a/rtengine/procparams.cc +++ b/rtengine/procparams.cc @@ -2832,7 +2832,10 @@ void ProcParams::setDefaults() exif.clear(); iptc.clear(); - rank = 0; + // -1 means that there's no pp3 data with rank yet. In this case, the + // embedded Rating metadata should take precedence. -1 should never be + // written to pp3 on disk. + rank = -1; colorlabel = 0; inTrash = false; diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc index e0509f457..39f4ca093 100644 --- a/rtgui/thumbnail.cc +++ b/rtgui/thumbnail.cc @@ -1006,15 +1006,22 @@ void Thumbnail::setFileName (const Glib::ustring &fn) int Thumbnail::getRank () const { - return pparams->rank; + // prefer the user-set rank over the embedded Rating + // pparams->rank == -1 means that there is no saved rank yet, so we should + // next look for the embedded Rating metadata. + if (pparams->rank != -1) { + return pparams->rank; + } else { + return cfs.rating; + } } void Thumbnail::setRank (int rank) { if (pparams->rank != rank) { pparams->rank = rank; - pparamsValid = true; } + pparamsValid = true; } int Thumbnail::getColorLabel () const diff --git a/rtgui/thumbnail.h b/rtgui/thumbnail.h index 75b1ecdd5..0bcdd470a 100644 --- a/rtgui/thumbnail.h +++ b/rtgui/thumbnail.h @@ -164,17 +164,8 @@ public: return cfs.md5; } - int getRank () const - { - return cfs.rating; - } - void setRank (int rank) - { - if (cfs.rating != rank) { - cfs.rating = rank; - } - pparamsValid = true; // FIXME-piotr Is this the right way to refresh the cache? Probably not since pparams and cache should be 2 different things - } + int getRank () const; + void setRank (int rank); int getColorLabel () const; void setColorLabel (int colorlabel); From a25655cafdd5f96cb11c4f0012ff2304095c0192 Mon Sep 17 00:00:00 2001 From: Eric Jiang Date: Thu, 16 May 2019 19:37:27 -0700 Subject: [PATCH 4/5] Check that metadata rating is between 0 and 5 Does not handle Rating=-1 meaning Rejected yet. --- rtengine/imagedata.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/rtengine/imagedata.cc b/rtengine/imagedata.cc index b9720ec8a..26fe80f03 100644 --- a/rtengine/imagedata.cc +++ b/rtengine/imagedata.cc @@ -200,6 +200,16 @@ FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* if (newFrameRootDir->getXMPTagValue("xmp:Rating", sXMPRating)) { rating = atoi(sXMPRating); } + // guard against out-of-range values + if (rating > 5) { + rating = 5; + } + // Currently, Rating=-1 is not supported. A value of -1 should mean + // "Rejected" according to the specification. Maybe in the future, Rating=-1 + // sets InTrash=true? + if (rating < 0) { + rating = 0; + } tag = newFrameRootDir->findTagUpward("MakerNote"); rtexif::TagDirectory* mnote = nullptr; From 789edc5bd32c9a604ce71544836d06695b576c49 Mon Sep 17 00:00:00 2001 From: Eric Jiang Date: Tue, 18 Jun 2019 18:02:10 -0700 Subject: [PATCH 5/5] Apply code-cleanup patch from @Floessie See https://github.com/Beep6581/RawTherapee/pull/5325 --- rtengine/imagedata.cc | 73 +++++++++++++++++++++-------------------- rtengine/imagedata.h | 2 +- rtgui/cacheimagedata.cc | 37 +++++++++++++++++---- 3 files changed, 70 insertions(+), 42 deletions(-) diff --git a/rtengine/imagedata.cc b/rtengine/imagedata.cc index 26fe80f03..faa7ad5ed 100644 --- a/rtengine/imagedata.cc +++ b/rtengine/imagedata.cc @@ -69,37 +69,40 @@ FramesMetaData* FramesMetaData::fromFile (const Glib::ustring& fname, std::uniqu return new FramesData (fname, std::move(rml), firstFrameOnly); } -FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* rootDir, rtexif::TagDirectory* firstRootDir) - : frameRootDir(frameRootDir_), iptc(nullptr), time(), timeStamp(), iso_speed(0), aperture(0.), focal_len(0.), focal_len35mm(0.), focus_dist(0.f), - shutter(0.), expcomp(0.), make("Unknown"), model("Unknown"), orientation("Unknown"), rating(0), lens("Unknown"), - sampleFormat(IIOSF_UNKNOWN), isPixelShift(false), isHDR(false) +FrameData::FrameData(rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* rootDir, rtexif::TagDirectory* firstRootDir) : + frameRootDir(frameRootDir_), + iptc(nullptr), + time{}, + timeStamp{}, + iso_speed(0), + aperture(0.), + focal_len(0.), + focal_len35mm(0.), + focus_dist(0.f), + shutter(0.), + expcomp(0.), + make("Unknown"), + model("Unknown"), + orientation("Unknown"), + rating(0), + lens("Unknown"), + sampleFormat(IIOSF_UNKNOWN), + isPixelShift(false), + isHDR(false) { - memset (&time, 0, sizeof(time)); - if (!frameRootDir) { return; } - rtexif::Tag* tag; - rtexif::TagDirectory* newFrameRootDir = frameRootDir; - - memset(&time, 0, sizeof(time)); - timeStamp = 0; - iso_speed = 0; - aperture = 0.0; - focal_len = 0.0; - focal_len35mm = 0.0; - focus_dist = 0.0f; - shutter = 0.0; - expcomp = 0.0; make.clear(); model.clear(); serial.clear(); orientation.clear(); lens.clear(); - rating = 0; - tag = newFrameRootDir->findTag("Make"); + rtexif::TagDirectory* newFrameRootDir = frameRootDir; + + rtexif::Tag* tag = newFrameRootDir->findTag("Make"); if (!tag) { newFrameRootDir = rootDir; tag = newFrameRootDir->findTag("Make"); @@ -198,17 +201,11 @@ FrameData::FrameData (rtexif::TagDirectory* frameRootDir_, rtexif::TagDirectory* } char sXMPRating[64]; if (newFrameRootDir->getXMPTagValue("xmp:Rating", sXMPRating)) { - rating = atoi(sXMPRating); - } - // guard against out-of-range values - if (rating > 5) { - rating = 5; - } - // Currently, Rating=-1 is not supported. A value of -1 should mean - // "Rejected" according to the specification. Maybe in the future, Rating=-1 - // sets InTrash=true? - if (rating < 0) { - rating = 0; + // Guard against out-of-range values (<0, >5) + rating = rtengine::max(0, rtengine::min(5, atoi(sXMPRating))); + // Currently, Rating=-1 is not supported. A value of -1 should mean + // "Rejected" according to the specification. Maybe in the future, Rating=-1 + // sets InTrash=true? } tag = newFrameRootDir->findTagUpward("MakerNote"); @@ -1197,15 +1194,21 @@ std::string FramesData::getOrientation(unsigned int frame) const } ); } -int FramesData::getRating (unsigned int frame) const + +int FramesData::getRating(unsigned int frame) const { - return frames.empty() || frame >= frames.size() ? 0 : frames.at(frame)->getRating (); + return getFromFrame( + frames, + frame, + [](const FrameData& frame_data) + { + return frame_data.getRating(); + } + ); } - //------inherited functions--------------// - std::string FramesMetaData::apertureToString (double aperture) { diff --git a/rtengine/imagedata.h b/rtengine/imagedata.h index 097a5cc80..d06820296 100644 --- a/rtengine/imagedata.h +++ b/rtengine/imagedata.h @@ -127,7 +127,7 @@ public: std::string getModel (unsigned int frame = 0) const override; std::string getLens (unsigned int frame = 0) const override; std::string getSerialNumber (unsigned int frame = 0) const; - std::string getOrientation (unsigned int frame = 0) const; + std::string getOrientation (unsigned int frame = 0) const override; int getRating (unsigned int frame = 0) const override; }; diff --git a/rtgui/cacheimagedata.cc b/rtgui/cacheimagedata.cc index 47318990d..1756f523b 100644 --- a/rtgui/cacheimagedata.cc +++ b/rtgui/cacheimagedata.cc @@ -24,12 +24,37 @@ #include "../rtengine/procparams.h" -CacheImageData::CacheImageData () - : md5(""), supported(false), format(FT_Invalid), rankOld(-1), inTrashOld(false), recentlySaved(false), - timeValid(false), year(0), month(0), day(0), hour(0), min(0), sec(0), exifValid(false), frameCount(1), - fnumber(0.0), shutter(0.0), focalLen(0.0), focalLen35mm(0.0), focusDist(0.f), iso(0), rating(0), isHDR (false), - isPixelShift (false), sensortype(rtengine::ST_NONE), sampleFormat(rtengine::IIOSF_UNKNOWN), - redAWBMul(-1.0), greenAWBMul(-1.0), blueAWBMul(-1.0), rotate(0), thumbImgType(0) +CacheImageData::CacheImageData() : + supported(false), + format(FT_Invalid), + rankOld(-1), + inTrashOld(false), + recentlySaved(false), + timeValid(false), + year(0), + month(0), + day(0), + hour(0), + min(0), + sec(0), + exifValid(false), + frameCount(1), + fnumber(0.0), + shutter(0.0), + focalLen(0.0), + focalLen35mm(0.0), + focusDist(0.f), + iso(0), + rating(0), + isHDR (false), + isPixelShift (false), + sensortype(rtengine::ST_NONE), + sampleFormat(rtengine::IIOSF_UNKNOWN), + redAWBMul(-1.0), + greenAWBMul(-1.0), + blueAWBMul(-1.0), + rotate(0), + thumbImgType(0) { }