Skip to content

Commit

Permalink
fix(lens): canon lens detecton should be independent of locale
Browse files Browse the repository at this point in the history
  • Loading branch information
hassec committed Mar 29, 2024
1 parent b6e1360 commit 6565352
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 6 deletions.
8 changes: 8 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,14 @@ if(EXIV2_BUILD_EXIV2_COMMAND)
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests
COMMAND cmake -E env EXIV2_BINDIR=${CMAKE_RUNTIME_OUTPUT_DIRECTORY} ${Python3_EXECUTABLE} runner.py --verbose lens_tests
)
# Repeat lens test with a DE locale that uses "," as float delimiter
# to check that all float conversions are done independently of locale
# See GitHub issue #2746
add_test(
NAME lensTestsLocaleDE
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests
COMMAND cmake -E env LC_ALL=de_DE.UTF-8 EXIV2_BINDIR=${CMAKE_RUNTIME_OUTPUT_DIRECTORY} ${Python3_EXECUTABLE} runner.py --verbose lens_tests
)
add_test(
NAME tiffTests
WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/tests
Expand Down
38 changes: 32 additions & 6 deletions src/canonmn_int.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <regex>
#include <sstream>
#include <string>

// *****************************************************************************
// class member definitions
namespace Exiv2::Internal {
Expand Down Expand Up @@ -2851,6 +2850,33 @@ std::ostream& printCsLensFFFF(std::ostream& os, const Value& value, const ExifDa
return EXV_PRINT_TAG(canonCsLensType)(os, value, metadata);
}

/**
* @brief convert string to float w/o considering locale
*
* Using std:stof to convert strings to float takes into account the locale
* and thus leads to wrong results when converting e.g. "5.6" with a DE locale
* which expects "," as decimal instead of ".". See GitHub issue #2746
*
* Use std::from_chars once that's properly supported by compilers.
*
* @param str string to convert
* @return float value of string
*/
float string_to_float(std::string const& str) {
float val{};
std::stringstream ss;
std::locale c_locale("C");
ss.imbue(c_locale);
ss << str;
ss >> val;

if (ss.fail()) {
throw Error(ErrorCode::kerErrorMessage, std::string("canonmn_int.cpp:string_to_float failed for: ") + str);
}

return val;
}

std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, const ExifData* metadata) {
if (!metadata || value.typeId() != unsignedShort || value.count() == 0)
return os << value;
Expand Down Expand Up @@ -2914,13 +2940,13 @@ std::ostream& printCsLensTypeByMetadata(std::ostream& os, const Value& value, co
throw Error(ErrorCode::kerErrorMessage, std::string("Lens regex didn't match for: ") + std::string(label));
}

auto tc = base_match[5].length() > 0 ? std::stof(base_match[5].str()) : 1.f;
auto tc = base_match[5].length() > 0 ? string_to_float(base_match[5].str()) : 1.f;

auto flMax = static_cast<int>(std::stof(base_match[2].str()) * tc);
int flMin = base_match[1].length() > 0 ? static_cast<int>(std::stof(base_match[1].str()) * tc) : flMax;
auto flMax = static_cast<int>(string_to_float(base_match[2].str()) * tc);
int flMin = base_match[1].length() > 0 ? static_cast<int>(string_to_float(base_match[1].str()) * tc) : flMax;

auto aperMaxTele = std::stof(base_match[4].str()) * tc;
auto aperMaxShort = base_match[3].length() > 0 ? std::stof(base_match[3].str()) * tc : aperMaxTele;
auto aperMaxTele = string_to_float(base_match[4].str()) * tc;
auto aperMaxShort = base_match[3].length() > 0 ? string_to_float(base_match[3].str()) * tc : aperMaxTele;

if (flMin != exifFlMin || flMax != exifFlMax || exifAperMax < (aperMaxShort - .1 * tc) ||
exifAperMax > (aperMaxTele + .1 * tc)) {
Expand Down

0 comments on commit 6565352

Please sign in to comment.