From e63f1cbf7c7b8abcf9708bc0dad4f042027ab223 Mon Sep 17 00:00:00 2001
From: Kubat <mael.martin31@gmail.com>
Date: Fri, 9 Jul 2021 16:44:45 +0200
Subject: [PATCH] ASS: Fix the ASS style parsing

---
 src/Lib/Ass/AssFactory.cc | 61 ++++++++++++++++++++++-----------------
 src/Lib/Ass/AssFactory.hh |  9 +++---
 src/Lib/Ass/Style.cc      | 51 ++++++++++++++++++--------------
 3 files changed, 68 insertions(+), 53 deletions(-)

diff --git a/src/Lib/Ass/AssFactory.cc b/src/Lib/Ass/AssFactory.cc
index 91a61f1c..d034e0d9 100644
--- a/src/Lib/Ass/AssFactory.cc
+++ b/src/Lib/Ass/AssFactory.cc
@@ -10,17 +10,19 @@ AssFactory::initFromStorage() noexcept
     QTextStream in(&diskStorage);
     QString currentSection{};
     quint64 lineIndex = 0;
+    QStringList stylesContent{};
+    QStringList eventsContent{};
 
     while (!in.atEnd()) {
         const QString line = in.readLine().trimmed();
 
-        /* Dectect comment */
+        // Dectect comment
         if (line.startsWith(";") || line.isEmpty()) {
             lineIndex++;
             continue;
         }
 
-        /* Dectect sections */
+        // Dectect sections
         else if (line.startsWith("[") && line.endsWith("]")) {
             currentSection = line.mid(1, line.size() - 2);
             qDebug() << "Parsing section" << currentSection;
@@ -30,31 +32,41 @@ AssFactory::initFromStorage() noexcept
             }
         }
 
-        /* Other lines */
+        // Other lines
         else if (!currentSection.isEmpty()) {
             const int separatorIndex = line.indexOf(": ");
             const int baseValueIndex = separatorIndex + 2;
-            if (separatorIndex < 0) {
+
+            // Easy way to see if the line was invalid
+            if (separatorIndex < 0)
                 qWarning() << "Invalid line #" << lineIndex << ":" << line;
-            } else {
-                assContent[currentSection].insert(line.mid(0, separatorIndex),
-                                                  line.mid(baseValueIndex));
+
+            // Script's info
+            else if (currentSection == sectionScriptInfo) {
+                assInfo.insert(line.mid(0, separatorIndex), line.mid(baseValueIndex));
                 qDebug() << "Got line #" << lineIndex << ":" << line;
             }
+
+            // Skip the headers and the comment lines
+            else if ((currentSection == sectionStyles) && line.startsWith("Style: ")) {
+                stylesContent.append(line);
+            } else if ((currentSection == sectionEvents) && line.startsWith("Dialogue: ")) {
+                eventsContent.append(line);
+            }
         }
 
         lineIndex++;
     }
 
-    /* Construct the styles and the lines */
-    for (const auto &styleLine : assContent[sectionStyles])
-        assStyles.push_back(std::make_shared<Style>(styleLine.toString()));
-    for (const auto &assLine : assContent[sectionEvents])
-        assLines.push_back(std::make_shared<Line>(assLine.toString()));
-
-    /* Get back some memory */
-    assContent.remove(sectionStyles);
-    assContent.remove(sectionEvents);
+    // Construct the styles and the lines
+    try {
+        for (const auto &styleLine : stylesContent)
+            assStyles.push_back(std::make_shared<Style>(styleLine));
+        for (const auto &assLine : eventsContent)
+            assLines.push_back(std::make_shared<Line>(assLine));
+    } catch (const std::runtime_error &e) {
+        qCritical() << "Failed to create ASS style or events with error:" << e.what();
+    }
 
     return true;
 }
@@ -62,26 +74,23 @@ AssFactory::initFromStorage() noexcept
 bool
 AssFactory::checkValidity() const noexcept
 {
-    if (assContent.isEmpty()) {
+    if (assInfo.isEmpty())
         return false;
-    }
-
-    const SectionContent &scriptInfo = assContent[sectionScriptInfo];
 
-    /* Check for fields that must be integers */
+    // Check for fields that must be integers
     bool ok                                  = false;
-    SectionContent::const_iterator it        = scriptInfo.begin();
-    const SectionContent::const_iterator end = scriptInfo.end();
+    SectionContent::const_iterator it        = assInfo.begin();
+    const SectionContent::const_iterator end = assInfo.end();
     while (it != end) {
         if (intTypeFields.contains(it.key()) && (it.value().toInt(&ok), !ok))
             return false;
         ++it;
     }
 
-    /* Check for fixed values fields */
+    // Check for fixed values fields
     for (const auto &fixedValues : checkedValues) {
         if (const auto &first = fixedValues.first;
-            scriptInfo.contains(first) && scriptInfo[first] != fixedValues.second)
+            assInfo.contains(first) && assInfo[first] != fixedValues.second)
             return false;
     }
 
@@ -120,5 +129,5 @@ AssFactory::getLines(QVector<AssFactory::LinePtr> &ret) const noexcept
 AssFactory::SectionContent
 AssFactory::getInfoSection() const noexcept
 {
-    return assContent[sectionScriptInfo];
+    return assInfo;
 }
diff --git a/src/Lib/Ass/AssFactory.hh b/src/Lib/Ass/AssFactory.hh
index 48f849b0..b30189c8 100644
--- a/src/Lib/Ass/AssFactory.hh
+++ b/src/Lib/Ass/AssFactory.hh
@@ -11,9 +11,8 @@
 #include <QVariant>
 #include <QString>
 
-/* An ASS file is basically an INI file, but some keys are present multiple
- * times (V4+ Styles/Style, Events/Dialogue, Events/Comment).
- */
+// An ASS file is basically an INI file, but some keys are present multiple
+// times (V4+ Styles/Style, Events/Dialogue, Events/Comment).
 
 namespace Vivy::Ass
 {
@@ -28,14 +27,14 @@ public:
     };
 
     using LinePtr        = std::shared_ptr<Line>;
-    using LineWeakPtr    = std::weak_ptr<Line>;
     using StylePtr       = std::shared_ptr<Style>;
+    using LineWeakPtr    = std::weak_ptr<Line>;
     using StyleWeakPtr   = std::weak_ptr<Style>;
     using SectionContent = QMap<QString, QVariant>;
 
 private:
     QFile diskStorage;
-    QMap<QString, SectionContent> assContent{};
+    SectionContent assInfo{};
     QVector<LinePtr> assLines{};
     QVector<StylePtr> assStyles{};
 
diff --git a/src/Lib/Ass/Style.cc b/src/Lib/Ass/Style.cc
index 58d247a4..2652769b 100644
--- a/src/Lib/Ass/Style.cc
+++ b/src/Lib/Ass/Style.cc
@@ -6,7 +6,7 @@ using namespace Vivy::Ass;
 
 Style::Style(const QString &styleString)
 {
-    /* Some usefull defines only needed in that function */
+    // Some usefull defines only needed in that function
 
     enum StyleIndex : int {
         Name     = 0,
@@ -34,31 +34,32 @@ Style::Style(const QString &styleString)
         Shadow  = 17,
 
         Alignement = 18,
-        MarginL    = 18,
-        MarginR    = 19,
-        MarginV    = 20,
+        MarginL    = 19,
+        MarginR    = 20,
+        MarginV    = 21,
 
-        Encoding = 21,
+        Encoding = 22,
 
         PastLastCode
     };
 
-    static const QString lineHeader           = "Style: ";
-    static const QString headerSeparator      = ": ";
-    static constexpr int styleContentListSize = StyleIndex::PastLastCode;
+    static const QString lineHeader = "Style: ";
 
-    /* Check line header and content items number */
+    // Check line header and content items number
 
     if (!styleString.startsWith(lineHeader))
-        throw std::runtime_error("invalid style line header");
+        throw std::runtime_error(("invalid style line header: " + styleString).toStdString());
 
-    const QString lineContent = styleString.section(headerSeparator, 2, 2);
+    const QString lineContent = styleString.mid(styleString.indexOf(": "));
     const QStringList content = lineContent.split(",", Qt::KeepEmptyParts, Qt::CaseInsensitive);
 
-    if (lineContent.size() != styleContentListSize)
-        throw std::runtime_error("invalid line content: invalid number of intems");
+    if (content.size() != StyleIndex::PastLastCode)
+        throw std::runtime_error(("invalid number of items " + QString::number(content.size()) +
+                                  " instead of " + QString::number(PastLastCode) +
+                                  " for line: " + lineContent)
+                                     .toStdString());
 
-    /* Unpack data from the line */
+    // Unpack data from the line
 
     styleName = content[StyleIndex::Name];
     fontName  = content[StyleIndex::FontName];
@@ -83,10 +84,13 @@ Style::Style(const QString &styleString)
     outline = decodeLineToFloating(content[StyleIndex::Outline], "Outline is not a floating");
     shadow  = decodeLineToFloating(content[StyleIndex::Shadow], "Shadow is not a floating");
 
-    alignment = decodeLineToInteger(content[StyleIndex::Shadow], "Alignement is not an integer");
+    alignment =
+        decodeLineToInteger(content[StyleIndex::Alignement], "Alignement is not an integer");
     if (alignment < 1 || alignment > 9)
         throw std::runtime_error(
-            "invalid line content: Alignement must be between 1 and 9 included");
+            ("invalid line content: Alignement must be between 1 and 9 included, it was " +
+             QString::number(alignment))
+                .toStdString());
 
     marginL  = decodeLineToInteger(content[StyleIndex::MarginL], "MarginL is not an integer");
     marginR  = decodeLineToInteger(content[StyleIndex::MarginR], "MarginR is not an integer");
@@ -126,19 +130,22 @@ Style::decodeLineToFloating(const QString &item, const QString &error)
 QColor
 Color::fromString(const QString &colorString) noexcept
 {
-    /* Ignore the '&H' at the begeing of the color if needed */
+    // Ignore the '&H' at the begeing of the color if needed
     int startIndex = 0;
-    if (colorString.startsWith("&"))
+    if (colorString[startIndex] == '&')
         startIndex++;
-    if (colorString.startsWith("H") || colorString.startsWith("h"))
+    if (colorString[startIndex] == 'H' || colorString[startIndex] == 'h')
         startIndex++;
 
-    /* In some cases, the color can begin by a '#' */
-    if (colorString.startsWith("#"))
+    // In some cases, the color can begin by a '#'
+    if (colorString[startIndex] == '#')
         startIndex++;
 
-    /* A valid string color is like 'AARRGGBB' for now (skipped 'aH') */
+    // A valid string color is like 'AARRGGBB' for now (skipped 'aH')
     if (colorString.size() - startIndex != 8) {
+        qCritical()
+            << "Invalid color string: size - index_start =" << (colorString.size() - startIndex)
+            << "| string =" << colorString.mid(startIndex);
         qCritical() << "Found an invalid color string:" << colorString;
         return Color::defaultValue;
     }
-- 
GitLab