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);