From 3343dd21d362c5d5c5b9fbb2cc6a985dd552ac8d Mon Sep 17 00:00:00 2001 From: Brad Kartchner Date: Fri, 3 Apr 2020 14:13:45 -0600 Subject: [PATCH 1/2] Improved Profile parsing I modified the Profile constructor to better identify invalid or malformed Profile files and throw more informative exceptions. I will also be pushing a modification to openshot-qt to better handle these exceptions. I also modified the InterpolateBetween function in KeyFrame.cpp to remove a compilation warning. --- src/KeyFrame.cpp | 11 +++++++--- src/Profiles.cpp | 57 +++++++++++++++++++++++++++--------------------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/KeyFrame.cpp b/src/KeyFrame.cpp index 57e424cf7..1b7b6c38a 100644 --- a/src/KeyFrame.cpp +++ b/src/KeyFrame.cpp @@ -88,11 +88,16 @@ namespace { double InterpolateBetween(Point const & left, Point const & right, double target, double allowed_error) { assert(left.co.X < target); assert(target <= right.co.X); + + double returnValue = 0.00; + switch (right.interpolation) { - case CONSTANT: return left.co.Y; - case LINEAR: return InterpolateLinearCurve(left, right, target); - case BEZIER: return InterpolateBezierCurve(left, right, target, allowed_error); + case CONSTANT: returnValue = left.co.Y; + case LINEAR: returnValue = InterpolateLinearCurve(left, right, target); + case BEZIER: returnValue = InterpolateBezierCurve(left, right, target, allowed_error); } + + return returnValue; } diff --git a/src/Profiles.cpp b/src/Profiles.cpp index 5351520e9..0285e59c1 100644 --- a/src/Profiles.cpp +++ b/src/Profiles.cpp @@ -36,11 +36,7 @@ using namespace openshot; // @brief Constructor for Profile. // @param path The folder path / location of a profile file Profile::Profile(std::string path) { - - bool read_file = false; - - try - { + try { // Initialize info values info.description = ""; info.height = 0; @@ -54,21 +50,32 @@ Profile::Profile(std::string path) { info.display_ratio.den = 0; info.interlaced_frame = false; + // Open the file for reading QFile inputFile(path.c_str()); - if (inputFile.open(QIODevice::ReadOnly)) + if (!inputFile.open(QIODevice::ReadOnly)) { + throw std::runtime_error("Failed to open the file for reading"); + } + + // Iterate over each line in the file + QTextStream in(&inputFile); + while (!in.atEnd()) { - QTextStream in(&inputFile); - while (!in.atEnd()) - { - QString line = in.readLine(); + QString line = in.readLine(); + + // Trim any extra whitespace from the line + line = line.trimmed(); - if (line.length() <= 0) - continue; + // If this line is not empty, or a comment + if (line.length() > 0 && line.at(0) != QChar('#')) { + // If the line does not contain an '=' character, it's invalid + if (line.contains(QChar('=')) == false) { + throw std::runtime_error("Invalid line encountered"); + } // Split current line QStringList parts = line.split( "=" ); - std::string setting = parts[0].toStdString(); - std::string value = parts[1].toStdString(); + std::string setting = parts[0].trimmed().toStdString(); + std::string value = parts[1].trimmed().toStdString(); int value_int = 0; // update struct (based on line number) @@ -113,23 +120,23 @@ Profile::Profile(std::string path) { else if (setting == "colorspace") { std::stringstream(value) >> value_int; info.pixel_format = value_int; + } + else { + // Any other setting makes the file invalid + std::ostringstream oss; + oss << "Unrecognized setting \"" << setting << "\" encountered"; + throw std::runtime_error(oss.str()); } } - read_file = true; + inputFile.close(); } - } - catch (const std::exception& e) - { - // Error parsing profile file - throw InvalidFile("Profile could not be found or loaded (or is invalid).", path); + + // If a runtime error was thrown, pass it along as an InvalidFile exception + catch (const std::exception& e) { + throw InvalidFile(e.what(), path); } - - // Throw error if file was not read - if (!read_file) - // Error parsing profile file - throw InvalidFile("Profile could not be found or loaded (or is invalid).", path); } // Generate JSON string of this object From ccc31c112e73012888ffaafe743801d260070586 Mon Sep 17 00:00:00 2001 From: Brad Kartchner Date: Fri, 3 Apr 2020 14:17:30 -0600 Subject: [PATCH 2/2] Rethought my changes to KeyFrame I made my changes to InterpolateBetween in KeyFrame.cpp less intrusive. --- src/KeyFrame.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/KeyFrame.cpp b/src/KeyFrame.cpp index 1b7b6c38a..7a2d5b562 100644 --- a/src/KeyFrame.cpp +++ b/src/KeyFrame.cpp @@ -87,17 +87,15 @@ namespace { double InterpolateBetween(Point const & left, Point const & right, double target, double allowed_error) { assert(left.co.X < target); - assert(target <= right.co.X); - - double returnValue = 0.00; - + assert(target <= right.co.X); switch (right.interpolation) { - case CONSTANT: returnValue = left.co.Y; - case LINEAR: returnValue = InterpolateLinearCurve(left, right, target); - case BEZIER: returnValue = InterpolateBezierCurve(left, right, target, allowed_error); + case CONSTANT: return left.co.Y; + case LINEAR: return InterpolateLinearCurve(left, right, target); + case BEZIER: return InterpolateBezierCurve(left, right, target, allowed_error); } - return returnValue; + // Control should never reach this point + return 0.0; }