From 33e5ef5ffd494aa930acb95c80962638a37da00d Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 9 Feb 2023 14:45:15 -0800 Subject: [PATCH 1/4] remove constructors Just direct initialize Signed-off-by: Rosen Penev --- src/makernote_int.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/makernote_int.cpp b/src/makernote_int.cpp index 8144f632..6ffcb42e 100644 --- a/src/makernote_int.cpp +++ b/src/makernote_int.cpp @@ -842,9 +842,6 @@ TiffComponent* newCasio2Mn2(uint16_t tag, IfdId group, IfdId mnGroup) { struct NikonArrayIdx { //! Key for comparisons struct Key { - //! Constructor - Key(uint16_t tag, const char* ver, size_t size) : tag_(tag), ver_(ver), size_(size) { - } uint16_t tag_; //!< Tag number const char* ver_; //!< Version string size_t size_; //!< Size of the data (not the version string) @@ -910,7 +907,7 @@ int nikonSelector(uint16_t tag, const byte* pData, size_t size, TiffComponent* / if (size < 4) return -1; - auto ix = NikonArrayIdx::Key(tag, reinterpret_cast(pData), size); + auto ix = NikonArrayIdx::Key{tag, reinterpret_cast(pData), size}; auto it = Exiv2::find(nikonArrayIdx, ix); if (!it) return -1; @@ -923,7 +920,7 @@ DataBuf nikonCrypt(uint16_t tag, const byte* pData, size_t size, TiffComponent* if (size < 4) return buf; - auto nci = Exiv2::find(nikonArrayIdx, NikonArrayIdx::Key(tag, reinterpret_cast(pData), size)); + auto nci = Exiv2::find(nikonArrayIdx, NikonArrayIdx::Key{tag, reinterpret_cast(pData), size}); if (!nci || nci->start_ == NA || size <= nci->start_) return buf; From b6655746348125c46cfcf1624af4bd1c25b8ffc6 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 9 Feb 2023 16:43:24 -0800 Subject: [PATCH 2/4] simplify loop Just use upper Signed-off-by: Rosen Penev --- src/riffvideo.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/riffvideo.cpp b/src/riffvideo.cpp index 61abf6d0..c733ecd7 100644 --- a/src/riffvideo.cpp +++ b/src/riffvideo.cpp @@ -12,6 +12,7 @@ #include "error.hpp" #include "futils.hpp" #include "helper_functions.hpp" +#include "utils.hpp" namespace Exiv2::Internal { @@ -397,10 +398,8 @@ RiffVideo::HeaderReader::HeaderReader(BasicIo::UniquePtr& io) { bool RiffVideo::equal(const std::string& str1, const std::string& str2) { if (str1.size() != str2.size()) return false; - for (size_t i = 0; i < str1.size(); i++) - if (toupper(str1[i]) != str2[i]) - return false; - return true; + + return Internal::upper(str1) == str2; } void RiffVideo::readList(HeaderReader& header_) { From f21b2a2170a197bb642c5f44a33de8ff0f98127a Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 9 Feb 2023 15:03:27 -0800 Subject: [PATCH 3/4] std to Exiv2 find Signed-off-by: Rosen Penev --- src/image.cpp | 101 +++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 55 deletions(-) diff --git a/src/image.cpp b/src/image.cpp index a76e8120..4a8dfcdf 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -73,46 +73,46 @@ struct Registry { }; /// \todo Use std::unordered_map for implementing the registry. Avoid to use ImageType::none -constexpr auto registry = std::array{ +constexpr Registry registry[] = { // image type creation fct type check Exif mode IPTC mode XMP mode Comment mode //--------------- --------------- ---------- ----------- ----------- ----------- ------------ - Registry{ImageType::jpeg, newJpegInstance, isJpegType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, - Registry{ImageType::exv, newExvInstance, isExvType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, - Registry{ImageType::cr2, newCr2Instance, isCr2Type, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::crw, newCrwInstance, isCrwType, amReadWrite, amNone, amNone, amReadWrite}, - Registry{ImageType::mrw, newMrwInstance, isMrwType, amRead, amRead, amRead, amNone}, - Registry{ImageType::tiff, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::webp, newWebPInstance, isWebPType, amReadWrite, amNone, amReadWrite, amNone}, - Registry{ImageType::dng, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::nef, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::pef, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::arw, newTiffInstance, isTiffType, amRead, amRead, amRead, amNone}, - Registry{ImageType::rw2, newRw2Instance, isRw2Type, amRead, amRead, amRead, amNone}, - Registry{ImageType::sr2, newTiffInstance, isTiffType, amRead, amRead, amRead, amNone}, - Registry{ImageType::srw, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::orf, newOrfInstance, isOrfType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::jpeg, newJpegInstance, isJpegType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, + {ImageType::exv, newExvInstance, isExvType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, + {ImageType::cr2, newCr2Instance, isCr2Type, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::crw, newCrwInstance, isCrwType, amReadWrite, amNone, amNone, amReadWrite}, + {ImageType::mrw, newMrwInstance, isMrwType, amRead, amRead, amRead, amNone}, + {ImageType::tiff, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::webp, newWebPInstance, isWebPType, amReadWrite, amNone, amReadWrite, amNone}, + {ImageType::dng, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::nef, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::pef, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::arw, newTiffInstance, isTiffType, amRead, amRead, amRead, amNone}, + {ImageType::rw2, newRw2Instance, isRw2Type, amRead, amRead, amRead, amNone}, + {ImageType::sr2, newTiffInstance, isTiffType, amRead, amRead, amRead, amNone}, + {ImageType::srw, newTiffInstance, isTiffType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::orf, newOrfInstance, isOrfType, amReadWrite, amReadWrite, amReadWrite, amNone}, #ifdef EXV_HAVE_LIBZ - Registry{ImageType::png, newPngInstance, isPngType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, + {ImageType::png, newPngInstance, isPngType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, #endif // EXV_HAVE_LIBZ - Registry{ImageType::pgf, newPgfInstance, isPgfType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, - Registry{ImageType::raf, newRafInstance, isRafType, amRead, amRead, amRead, amNone}, - Registry{ImageType::eps, newEpsInstance, isEpsType, amNone, amNone, amReadWrite, amNone}, - Registry{ImageType::xmp, newXmpInstance, isXmpType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::gif, newGifInstance, isGifType, amNone, amNone, amNone, amNone}, - Registry{ImageType::psd, newPsdInstance, isPsdType, amReadWrite, amReadWrite, amReadWrite, amNone}, - Registry{ImageType::tga, newTgaInstance, isTgaType, amNone, amNone, amNone, amNone}, - Registry{ImageType::bmp, newBmpInstance, isBmpType, amNone, amNone, amNone, amNone}, - Registry{ImageType::jp2, newJp2Instance, isJp2Type, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::pgf, newPgfInstance, isPgfType, amReadWrite, amReadWrite, amReadWrite, amReadWrite}, + {ImageType::raf, newRafInstance, isRafType, amRead, amRead, amRead, amNone}, + {ImageType::eps, newEpsInstance, isEpsType, amNone, amNone, amReadWrite, amNone}, + {ImageType::xmp, newXmpInstance, isXmpType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::gif, newGifInstance, isGifType, amNone, amNone, amNone, amNone}, + {ImageType::psd, newPsdInstance, isPsdType, amReadWrite, amReadWrite, amReadWrite, amNone}, + {ImageType::tga, newTgaInstance, isTgaType, amNone, amNone, amNone, amNone}, + {ImageType::bmp, newBmpInstance, isBmpType, amNone, amNone, amNone, amNone}, + {ImageType::jp2, newJp2Instance, isJp2Type, amReadWrite, amReadWrite, amReadWrite, amNone}, // needs to be before bmff because some ftyp files are handled as qt and // the rest should fall through to bmff #ifdef EXV_ENABLE_VIDEO - Registry{ImageType::qtime, newQTimeInstance, isQTimeType, amRead, amNone, amRead, amNone}, - Registry{ImageType::asf, newAsfInstance, isAsfType, amRead, amNone, amRead, amNone}, - Registry{ImageType::riff, newRiffInstance, isRiffType, amRead, amNone, amRead, amNone}, - Registry{ImageType::mkv, newMkvInstance, isMkvType, amRead, amNone, amRead, amNone}, + {ImageType::qtime, newQTimeInstance, isQTimeType, amRead, amNone, amRead, amNone}, + {ImageType::asf, newAsfInstance, isAsfType, amRead, amNone, amRead, amNone}, + {ImageType::riff, newRiffInstance, isRiffType, amRead, amNone, amRead, amNone}, + {ImageType::mkv, newMkvInstance, isMkvType, amRead, amNone, amRead, amNone}, #endif // EXV_ENABLE_VIDEO #ifdef EXV_ENABLE_BMFF - Registry{ImageType::bmff, newBmffInstance, isBmffType, amRead, amRead, amRead, amNone}, + {ImageType::bmff, newBmffInstance, isBmffType, amRead, amRead, amRead, amNone}, #endif // EXV_ENABLE_BMFF }; @@ -730,38 +730,29 @@ const std::string& Image::tagName(uint16_t tag) { } AccessMode ImageFactory::checkMode(ImageType type, MetadataId metadataId) { - auto r = std::find(registry.begin(), registry.end(), type); - if (r == registry.end()) + auto r = Exiv2::find(registry, type); + if (!r) throw Error(ErrorCode::kerUnsupportedImageType, static_cast(type)); - AccessMode am = amNone; switch (metadataId) { - case mdNone: - break; case mdExif: - am = r->exifSupport_; - break; + return r->exifSupport_; case mdIptc: - am = r->iptcSupport_; - break; + return r->iptcSupport_; case mdXmp: - am = r->xmpSupport_; - break; + return r->xmpSupport_; case mdComment: - am = r->commentSupport_; - break; - case mdIccProfile: - break; - - // no default: let the compiler complain + return r->commentSupport_; + default: + return amNone; } - return am; + return {}; } bool ImageFactory::checkType(ImageType type, BasicIo& io, bool advance) { - auto r = std::find(registry.begin(), registry.end(), type); - if (r != registry.end()) - return r->isThisType_(io, advance); - return false; + auto r = Exiv2::find(registry, type); + if (!r) + return false; + return r->isThisType_(io, advance); } ImageType ImageFactory::getType(const std::string& path) { @@ -859,8 +850,8 @@ Image::UniquePtr ImageFactory::create(ImageType type, BasicIo::UniquePtr io) { // BasicIo instance does not need to be open if (type == ImageType::none) return {}; - auto r = std::find(registry.begin(), registry.end(), type); - if (r == registry.end()) + auto r = Exiv2::find(registry, type); + if (!r) return {}; return r->newInstance_(std::move(io), true); } From dee0598edf3edd2dbd32690ad99ca5f5209dc738 Mon Sep 17 00:00:00 2001 From: Rosen Penev Date: Thu, 9 Feb 2023 13:14:39 -0800 Subject: [PATCH 4/4] clang-tidy fixes for recent stuff Signed-off-by: Rosen Penev --- include/exiv2/riffvideo.hpp | 8 ++++---- src/riffvideo.cpp | 9 ++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/include/exiv2/riffvideo.hpp b/include/exiv2/riffvideo.hpp index 3c6c2605..4112e11f 100644 --- a/include/exiv2/riffvideo.hpp +++ b/include/exiv2/riffvideo.hpp @@ -44,7 +44,7 @@ class EXIV2API RiffVideo : public Image { protected: class HeaderReader { - std::string id_ = ""; + std::string id_; uint64_t size_ = 0; public: @@ -54,7 +54,7 @@ class EXIV2API RiffVideo : public Image { return size_; } - [[nodiscard]] std::string& getId() { + [[nodiscard]] const std::string& getId() const { return id_; } }; @@ -66,7 +66,7 @@ class EXIV2API RiffVideo : public Image { void decodeBlocks(); private: - bool equal(const std::string& str1, const std::string& str2); + static bool equal(const std::string& str1, const std::string& str2); /*! @brief Interpret MainAVIHeader (avih) structure, and save it in the respective XMP container. @@ -130,7 +130,7 @@ class EXIV2API RiffVideo : public Image { */ void readJunk(uint64_t size_); - std::string getStreamType(uint32_t stream); + static std::string getStreamType(uint32_t stream); /*! @brief Calculates Duration of a video, and stores it in the respective XMP container. @param frame_rate Frame rate of the video. diff --git a/src/riffvideo.cpp b/src/riffvideo.cpp index c733ecd7..f7844430 100644 --- a/src/riffvideo.cpp +++ b/src/riffvideo.cpp @@ -740,14 +740,13 @@ void RiffVideo::readJunk(uint64_t size_) { std::string RiffVideo::getStreamType(uint32_t stream) { if (stream == 1) return "Mono"; - else if (stream == 2) + if (stream == 2) return "Stereo"; - else if (stream == 5) + if (stream == 5) return "5.1 Surround Sound"; - else if (stream == 7) + if (stream == 7) return "7.1 Surround Sound"; - else - return "Mono"; + return "Mono"; } void RiffVideo::fillDuration(double frame_rate, size_t frame_count) {