diff --git a/src/rawtoaces_util/image_converter.cpp b/src/rawtoaces_util/image_converter.cpp index 7fa5c188..589b0a3c 100644 --- a/src/rawtoaces_util/image_converter.cpp +++ b/src/rawtoaces_util/image_converter.cpp @@ -47,8 +47,6 @@ struct CameraIdentifier * * @param path The filesystem path to check * @param batch Reference to a vector of strings to add valid file paths to - * @return true if the file was processed (either added to batch or filtered out), - * false if the file should be ignored */ void check_and_add_file( const std::filesystem::path &path, std::vector &batch ) @@ -1441,11 +1439,11 @@ bool ImageConverter::configure( return false; } - bool spectral_white_balance = + bool is_spectral_white_balance = settings.WB_method == Settings::WBMethod::Illuminant; - bool spectral_matrix = matrix_method == Settings::MatrixMethod::Spectral; + bool is_spectral_matrix = matrix_method == Settings::MatrixMethod::Spectral; - if ( spectral_white_balance || spectral_matrix ) + if ( is_spectral_white_balance || is_spectral_matrix ) { if ( !prepare_transform_spectral( image_spec, @@ -1459,7 +1457,7 @@ bool ImageConverter::configure( return false; } - if ( spectral_white_balance ) + if ( is_spectral_white_balance ) { float custom_WB[4]; @@ -1582,9 +1580,9 @@ bool ImageConverter::configure( if ( settings.crop_box[2] > 0 && settings.crop_box[3] > 0 ) { - std::cerr << " Crop box: [" << settings.crop_box[0] << ", " - << settings.crop_box[1] << ", " << settings.crop_box[2] - << ", " << settings.crop_box[3] << "]" << std::endl; + std::cerr << " Crop box: [" + << OIIO::Strutil::join( settings.crop_box, ", " ) << "]" + << std::endl; } std::cerr << " Demosaic: " << settings.demosaic_algorithm << std::endl; diff --git a/tests/test_image_converter.cpp b/tests/test_image_converter.cpp index cb5391e5..8481136e 100644 --- a/tests/test_image_converter.cpp +++ b/tests/test_image_converter.cpp @@ -18,11 +18,96 @@ #include #include #include -#include // for mkfifo #include +#ifdef WIN32 +# include +# include +# include // for _getcwd +#else +# include // for mkfifo +# include // for getcwd +#endif + using namespace rta::util; +std::string convert_linux_path_to_windows_path( const std::string &path ) +{ + std::vector segments; + OIIO::Strutil::split( path, segments, ":" ); + + for ( auto &segment: segments ) + { + // Convert forward slashes to backslashes + std::replace( segment.begin(), segment.end(), '/', '\\' ); + // Add drive letter + segment = "c:" + segment; + } + + return OIIO::Strutil::join( segments, ";" ); +} + +FILE *platform_popen( const char *command, const char *mode ) +{ +#ifdef WIN32 + return _popen( command, mode ); +#else + return popen( command, mode ); +#endif +} + +int platform_pclose( FILE *pipe ) +{ +#ifdef WIN32 + return _pclose( pipe ); +#else + int status = pclose( pipe ); + return WEXITSTATUS( status ); +#endif +} + +/// Executes a rawtoaces command with the given arguments and captures its output. +/// +/// @param args Vector of command-line arguments to pass to rawtoaces (excluding program name). +/// For example, {"--list-cameras"} or {"--list-illuminants", "--verbose"}. +/// +/// @return std::string containing the captured stdout output from the rawtoaces execution +std::string run_rawtoaces_command( const std::vector &args ) +{ + // Build the command line - try multiple possible paths for different environments + std::string command; +#ifdef WIN32 + // Use the standard path from build directory + command = "..\\src\\rawtoaces\\Release\\rawtoaces.exe"; +#else + command = "../src/rawtoaces/rawtoaces"; +#endif + + for ( const auto &arg: args ) + { + command += " " + arg; + } + + // Execute command using platform-specific functions + FILE *pipe = platform_popen( command.c_str(), "r" ); + OIIO_CHECK_ASSERT( + pipe != nullptr && "Failed to execute rawtoaces command" ); + + // Read output + std::string output; + char buffer[4096]; + while ( fgets( buffer, sizeof( buffer ), pipe ) != nullptr ) + { + output += buffer; + } + + // Get exit status and validate + int exit_status = platform_pclose( pipe ); + OIIO_CHECK_EQUAL( exit_status, 0 ); + + return output; +} + // Cross-platform environment variable helpers /* Standard C Library vs POSIX @@ -30,22 +115,34 @@ getenv() - Part of standard C library (C89/C99) - available everywhere setenv()/unsetenv() - Part of POSIX standard - only on Unix-like systems */ #ifdef WIN32 -void set_env_var( const char *name, const char *value ) +void set_env_var( const std::string &name, const std::string &value ) { - _putenv_s( name, value ); + _putenv_s( name.c_str(), value.c_str() ); } -void unset_env_var( const char *name ) + +std::string to_os_path( std::string linux_path ) +{ + return convert_linux_path_to_windows_path( linux_path ); +} + +void unset_env_var( const std::string &name ) { - _putenv_s( name, "" ); + _putenv_s( name.c_str(), "" ); } #else -void set_env_var( const char *name, const char *value ) +std::string to_os_path( std::string linux_path ) { - setenv( name, value, 1 ); + return linux_path; } -void unset_env_var( const char *name ) + +void set_env_var( const std::string &name, const std::string &value ) +{ + setenv( name.c_str(), value.c_str(), 1 ); +} + +void unset_env_var( const std::string &name ) { - unsetenv( name ); + unsetenv( name.c_str() ); } #endif @@ -394,14 +491,14 @@ void test_database_paths_default() OIIO_CHECK_EQUAL( paths.empty(), false ); // On Unix systems, should have both new and legacy paths -#if !defined( WIN32 ) && !defined( WIN64 ) - OIIO_CHECK_EQUAL( paths.size(), 2 ); - OIIO_CHECK_EQUAL( paths[0], "/usr/local/share/rawtoaces/data" ); - OIIO_CHECK_EQUAL( paths[1], "/usr/local/include/rawtoaces/data" ); -#else +#ifdef WIN32 // On Windows, should have just the current directory OIIO_CHECK_EQUAL( paths.size(), 1 ); OIIO_CHECK_EQUAL( paths[0], "." ); +#else + OIIO_CHECK_EQUAL( paths.size(), 2 ); + OIIO_CHECK_EQUAL( paths[0], "/usr/local/share/rawtoaces/data" ); + OIIO_CHECK_EQUAL( paths[1], "/usr/local/include/rawtoaces/data" ); #endif } @@ -410,24 +507,15 @@ void test_database_paths_rawtoaces_env() { std::cout << std::endl << "test_database_paths_rawtoaces_env()" << std::endl; - // Set RAWTOACES_DATA_PATH -#ifdef WIN32 - set_env_var( "RAWTOACES_DATA_PATH", "C:\\custom\\path1;C:\\custom\\path2" ); -#else - set_env_var( "RAWTOACES_DATA_PATH", "/custom/path1:/custom/path2" ); -#endif + set_env_var( + "RAWTOACES_DATA_PATH", to_os_path( "/custom/path1:/custom/path2" ) ); unset_env_var( "AMPAS_DATA_PATH" ); std::vector paths = database_paths(); OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\custom\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\custom\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/custom/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/custom/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/custom/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/custom/path2" ) ); // Clean up unset_env_var( "RAWTOACES_DATA_PATH" ); @@ -439,23 +527,15 @@ void test_database_paths_ampas_env() std::cout << std::endl << "test_database_paths_ampas_env()" << std::endl; // Set AMPAS_DATA_PATH (deprecated) unset_env_var( "RAWTOACES_DATA_PATH" ); -#ifdef WIN32 set_env_var( - "AMPAS_DATA_PATH", "C:\\deprecated\\path1;C:\\deprecated\\path2" ); -#else - set_env_var( "AMPAS_DATA_PATH", "/deprecated/path1:/deprecated/path2" ); -#endif + "AMPAS_DATA_PATH", + to_os_path( "/deprecated/path1:/deprecated/path2" ) ); std::vector paths = database_paths(); OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\deprecated\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\deprecated\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/deprecated/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/deprecated/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/deprecated/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/deprecated/path2" ) ); // Clean up unset_env_var( "AMPAS_DATA_PATH" ); @@ -466,33 +546,78 @@ void test_database_paths_both_env() { std::cout << std::endl << "test_database_paths_both_env()" << std::endl; // Set both environment variables -#ifdef WIN32 set_env_var( - "RAWTOACES_DATA_PATH", "C:\\preferred\\path1;C:\\preferred\\path2" ); + "RAWTOACES_DATA_PATH", + to_os_path( "/preferred/path1:/preferred/path2" ) ); set_env_var( - "AMPAS_DATA_PATH", "C:\\deprecated\\path1;C:\\deprecated\\path2" ); -#else - set_env_var( "RAWTOACES_DATA_PATH", "/preferred/path1:/preferred/path2" ); - set_env_var( "AMPAS_DATA_PATH", "/deprecated/path1:/deprecated/path2" ); -#endif + "AMPAS_DATA_PATH", + to_os_path( "/deprecated/path1:/deprecated/path2" ) ); std::vector paths = database_paths(); // RAWTOACES_DATA_PATH should take precedence OIIO_CHECK_EQUAL( paths.size(), 2 ); -#ifdef WIN32 - OIIO_CHECK_EQUAL( paths[0], "C:\\preferred\\path1" ); - OIIO_CHECK_EQUAL( paths[1], "C:\\preferred\\path2" ); -#else - OIIO_CHECK_EQUAL( paths[0], "/preferred/path1" ); - OIIO_CHECK_EQUAL( paths[1], "/preferred/path2" ); -#endif + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/preferred/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/preferred/path2" ) ); + + // Clean up + unset_env_var( "RAWTOACES_DATA_PATH" ); + unset_env_var( "AMPAS_DATA_PATH" ); +} + +/// Tests database_paths with override_path parameter (--data-dir functionality) +/// Verifies that override_path takes precedence over environment variables +void test_database_paths_override_path() +{ + std::cout << std::endl + << "test_database_paths_override_path()" << std::endl; + // Set environment variables to ensure they are overridden + + set_env_var( "RAWTOACES_DATA_PATH", to_os_path( "/env/path1:/env/path2" ) ); + set_env_var( + "AMPAS_DATA_PATH", + to_os_path( "/deprecated/path1:/deprecated/path2" ) ); + + // Test with override path - should take precedence over environment variables + std::string override_path = + to_os_path( "/override/path1:/override/path2:/override/path3" ); + std::vector paths = database_paths( override_path ); + + // Should have 3 paths from override + OIIO_CHECK_EQUAL( paths.size(), 3 ); + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/override/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/override/path2" ) ); + OIIO_CHECK_EQUAL( paths[2], to_os_path( "/override/path3" ) ); + + // Test with empty override path - should fall back to environment variables + paths = database_paths( "" ); + + // Should have 2 paths from RAWTOACES_DATA_PATH environment variable + OIIO_CHECK_EQUAL( paths.size(), 2 ); + OIIO_CHECK_EQUAL( paths[0], to_os_path( "/env/path1" ) ); + OIIO_CHECK_EQUAL( paths[1], to_os_path( "/env/path2" ) ); // Clean up unset_env_var( "RAWTOACES_DATA_PATH" ); unset_env_var( "AMPAS_DATA_PATH" ); } +/// Tests convert_linux_path_to_windows_path utility function +void test_convert_linux_path_to_windows_path() +{ + std::cout << std::endl + << "test_convert_linux_path_to_windows_path()" << std::endl; + + // Test single path with forward slashes + std::string result = + convert_linux_path_to_windows_path( "/usr/local/share" ); + OIIO_CHECK_EQUAL( result, "c:\\usr\\local\\share" ); + + // Test multiple paths separated by ':' + result = convert_linux_path_to_windows_path( "/path1:/path2:/path3" ); + OIIO_CHECK_EQUAL( result, "c:\\path1;c:\\path2;c:\\path3" ); +} + /// Tests fix_metadata with both Make and Model attributes void test_fix_metadata_both_attributes() { @@ -573,79 +698,43 @@ void test_fix_metadata_unsupported_type() OIIO_CHECK_EQUAL( spec.find_attribute( "Make" ), nullptr ); } -/// Helper function to set up test environment and capture output for parse_parameters tests -struct ParseParametersTestResult +std::string run_rawtoaces_with_data_dir( + std::vector &args, + const std::string &datab_path, + bool use_dir_path_arg = false ) { - bool success; - std::string output; -}; - -/// Executes a parse_parameters test with the given command-line arguments and captures its output. -/// -/// This helper function encapsulates all the common setup required for testing ImageConverter::parse_parameters, -/// including environment configuration, argument parsing, stdout capture, and cleanup. -/// -/// @param args Vector of command-line arguments to pass to parse_parameters (excluding program name). -/// For example, {"--list-cameras"} or {"--list-illuminants", "--verbose"}. -/// @param database_path Path to the test database directory (optional, uses default if not provided) -/// -/// @return ParseParametersTestResult containing: -/// - success: true if parse_parameters executed successfully, false if argument parsing failed -/// - output: captured stdout output from the parse_parameters execution -ParseParametersTestResult run_parse_parameters_test( - const std::vector &args, - const std::string &database_path = "" ) -{ - // Set up test data path to use the provided database path or default - set_env_var( "RAWTOACES_DATA_PATH", database_path.c_str() ); - - // Create ImageConverter instance - rta::util::ImageConverter converter; - - // Create argument parser and initialize it - OIIO::ArgParse arg_parser; - converter.init_parser( arg_parser ); - - // Convert args to char* array for parsing - std::vector argv; - argv.push_back( "rawtoaces" ); // Program name - for ( const auto &arg: args ) + if ( use_dir_path_arg ) { - argv.push_back( arg.c_str() ); + args.push_back( "--data-dir" ); + args.push_back( datab_path ); + unset_env_var( "RAWTOACES_DATA_PATH" ); } - - // Parse the arguments - int parse_result = - arg_parser.parse_args( static_cast( argv.size() ), argv.data() ); - if ( parse_result != 0 ) + else { - unset_env_var( "RAWTOACES_DATA_PATH" ); - return { false, "" }; + set_env_var( "RAWTOACES_DATA_PATH", datab_path ); } - // Capture stdout to verify the output - std::ostringstream captured_output; - std::streambuf *original_cout = std::cout.rdbuf(); - std::cout.rdbuf( captured_output.rdbuf() ); + std::string output = run_rawtoaces_command( args ); + OIIO::string_view output_view = output; + OIIO::Strutil::trim_whitespace( output_view ); - // Call parse_parameters - bool result = converter.parse_parameters( arg_parser ); - - // Restore original cout - std::cout.rdbuf( original_cout ); - - // Clean up environment variable - unset_env_var( "RAWTOACES_DATA_PATH" ); + if ( !use_dir_path_arg ) + { + // Clean up environment variable + unset_env_var( "RAWTOACES_DATA_PATH" ); + } - return { result, captured_output.str() }; + return output_view; } /// This test verifies that when --list-cameras is provided, the method /// calls supported_cameras() and outputs the camera list, then exits -void test_parse_parameters_list_cameras() +void test_parse_parameters_list_cameras( bool use_dir_path_arg = false ) { std::cout << std::endl - << "test_parse_parameters_list_cameras()" << std::endl; + << "test_parse_parameters_list_cameras(" + << ( use_dir_path_arg ? "with data dir" : "without data dir" ) + << ")" << std::endl; // Create test directory with dynamic database TestDirectory test_dir; @@ -656,41 +745,34 @@ void test_parse_parameters_list_cameras() test_dir.create_test_data_file( "camera", { { "manufacturer", "Mamiya" }, { "model", "Mamiya 7" } } ); - // Run the test with --list-cameras argument using the dynamic database - auto result = run_parse_parameters_test( - { "--list-cameras" }, test_dir.get_database_path() ); + std::vector args = { "--list-cameras" }; + auto output = run_rawtoaces_with_data_dir( + args, test_dir.get_database_path(), use_dir_path_arg ); - // The method should return true (though it calls exit in real usage) - OIIO_CHECK_EQUAL( result.success, true ); + std::vector lines; + OIIO::Strutil::split( output, lines, "\n" ); - // Verify the output contains expected camera list information + OIIO_CHECK_EQUAL( lines.size(), 3 ); OIIO_CHECK_EQUAL( - result.output.find( - "Spectral sensitivity data is available for the following cameras:" ) != - std::string::npos, - true ); + lines[0], + "Spectral sensitivity data is available for the following cameras:" ); - // Verify that actual camera names from test data are present - // The format is "manufacturer / model" as defined in supported_cameras() - OIIO_CHECK_EQUAL( - result.output.find( "Canon / EOS R6" ) != std::string::npos, true ); - OIIO_CHECK_EQUAL( - result.output.find( "Mamiya / Mamiya 7" ) != std::string::npos, true ); - - // Count occurrences of " / " to verify we have 2 camera entries - size_t camera_count = 0; - size_t pos = 0; - while ( ( pos = result.output.find( " / ", pos ) ) != std::string::npos ) + // Check that both cameras are present (order doesn't matter) + bool found_canon = false, found_mamiya = false; + for ( size_t i = 1; i < lines.size(); ++i ) { - camera_count++; - pos += 3; // Move past " / " + if ( lines[i] == "Canon / EOS R6" ) + found_canon = true; + if ( lines[i] == "Mamiya / Mamiya 7" ) + found_mamiya = true; } - OIIO_CHECK_EQUAL( camera_count, 2 ); + OIIO_CHECK_EQUAL( found_canon, true ); + OIIO_CHECK_EQUAL( found_mamiya, true ); } /// This test verifies that when --list-illuminants is provided, the method /// calls supported_illuminants() and outputs the illuminant list, then exits -void test_parse_parameters_list_illuminants() +void test_parse_parameters_list_illuminants( bool use_dir_path_arg = false ) { std::cout << std::endl << "test_parse_parameters_list_illuminants()" << std::endl; @@ -702,60 +784,20 @@ void test_parse_parameters_list_illuminants() test_dir.create_test_data_file( "illuminant", { { "illuminant", "my-illuminant" } } ); - // Run the test with --list-illuminants argument using the dynamic database - auto result = run_parse_parameters_test( - { "--list-illuminants" }, test_dir.get_database_path() ); - - // The method should return true (though it calls exit in real usage) - OIIO_CHECK_EQUAL( result.success, true ); - - // Verify the output contains expected illuminant list information - OIIO_CHECK_EQUAL( - result.output.find( "The following illuminants are supported:" ) != - std::string::npos, - true ); + std::vector args = { "--list-illuminants" }; - // Verify that actual illuminant names from test data are present - // The hardcoded illuminant types should be present - OIIO_CHECK_EQUAL( - result.output.find( "Day-light (e.g., D60, D6025)" ) != - std::string::npos, - true ); - OIIO_CHECK_EQUAL( - result.output.find( "Blackbody (e.g., 3200K)" ) != std::string::npos, - true ); + // Run the test with --list-illuminants argument using the dynamic database + auto output = run_rawtoaces_with_data_dir( + args, test_dir.get_database_path(), use_dir_path_arg ); - // Verify that the specific illuminant from our test data is present - OIIO_CHECK_EQUAL( - result.output.find( "my-illuminant" ) != std::string::npos, true ); - - // Verify we have exactly 3 illuminants total (2 hardcoded + 1 from test data) - // Count newlines in the illuminant list section to verify count - size_t illuminant_count = 0; - size_t start_pos = - result.output.find( "The following illuminants are supported:" ); - if ( start_pos != std::string::npos ) - { - size_t end_pos = result.output.find( - "\n\n", start_pos ); // Look for double newline (end of list) - if ( end_pos == std::string::npos ) - { - end_pos = result.output.length(); // If no double newline, go to end - } + std::vector lines; + OIIO::Strutil::split( output, lines, "\n" ); - std::string illuminant_section = - result.output.substr( start_pos, end_pos - start_pos ); - size_t pos = 0; - while ( ( pos = illuminant_section.find( "\n", pos ) ) != - std::string::npos ) - { - illuminant_count++; - pos += 1; - } - // Subtract 1 for the header line - illuminant_count = ( illuminant_count > 0 ) ? illuminant_count - 1 : 0; - } - OIIO_CHECK_EQUAL( illuminant_count, 3 ); + OIIO_CHECK_EQUAL( lines.size(), 4 ); + OIIO_CHECK_EQUAL( lines[0], "The following illuminants are supported:" ); + OIIO_CHECK_EQUAL( lines[1], "Day-light (e.g., D60, D6025)" ); + OIIO_CHECK_EQUAL( lines[2], "Blackbody (e.g., 3200K)" ); + OIIO_CHECK_EQUAL( lines[3], "my-illuminant" ); } int main( int, char ** ) @@ -776,6 +818,10 @@ int main( int, char ** ) test_database_paths_rawtoaces_env(); test_database_paths_ampas_env(); test_database_paths_both_env(); + test_database_paths_override_path(); + + // Tests for utility functions + test_convert_linux_path_to_windows_path(); // Tests for fix_metadata test_fix_metadata_both_attributes(); @@ -786,7 +832,9 @@ int main( int, char ** ) // Tests for parse_parameters test_parse_parameters_list_cameras(); + test_parse_parameters_list_cameras( true ); test_parse_parameters_list_illuminants(); + test_parse_parameters_list_illuminants( true ); } catch ( const std::exception &e ) {