# Нельзя просто так взять и отредактировать субтитры Сколько людей пользуются субтитрами по всему миру? Вероятно, очень много\. В образовательных целях или просто из\-за любви к оригинальной озвучке, в интернете можно найти субтитры практически к любому фильму и на многих языках\. Создаётся всё это в специальных программах\. Как и в большинстве программ, в Subtitle Edit не обошлось без сюрпризов в виде багов\. ![0562_SubtitleEdit_ru/image1.png](https://import.viva64.com/docx/blog/0562_SubtitleEdit_ru/image1.png) ## Введение Subtitle Edit \- бесплатный редактор с огромным списком возможностей\. Это большой проект на языке C\# с открытым исходным кодом\. Программа очень популярна и выдаётся в первых строках выдачи результатов поисковиков, на сайте проекта перечислено множество наград\. В репозитории на [GitHub](https://github.com/SubtitleEdit/subtitleedit) можно увидеть, что проект активно развивается, имеет много звёзд и форков\. В общем, это хороший проект, чтобы поучаствовать в его развитии\. Изначально я просто искал себе библиотеку для парсинга субтитров, потому что большинство форматов не текстовые, но теперь вернусь к своему проекту немного позже\. На странице проекта на GitHub открыто 310 Issues\. Возможно, работа с результатами анализа позволит что\-то исправить\. Статический анализатор [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/), который использовался для исследования кода, выдал 460 предупреждений \(суммарно для всех уровней важности\)\. Практически всё можно и нужно поправить\. Это связано с тем, что в анализаторе почти нет рекомендательных диагностик\. Найденные результаты обычно сигнализируют о реальных проблемах в коде\. В статье я буду приводить примеры кода, но выберу только те ошибки, которые могут сильно влиять на работу\. Для более\-менее понятных фрагментов кода я отправлю Pull Request с исправлениями\. Но автору проекта лучше ознакомиться со всеми результатами анализа, проверив проект самостоятельно\. ## Игнорирование стилей Так выглядит фрагмент формы для задания стиля субтитров: ![0562_SubtitleEdit_ru/image2.png](https://import.viva64.com/docx/blog/0562_SubtitleEdit_ru/image2.png) А вот предупреждение анализатора на код, который связан с этой формой: [V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/) CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 300, 302\. SubStationAlphaStyles\.cs 300 ```cpp public static void AddStyle(ListView lv, SsaStyle ssaStyle, Subtitle subtitle, bool isSubstationAlpha) { .... if (ssaStyle.Bold || ssaStyle.Italic) subItem.Font = new Font(...., FontStyle.Bold | FontStyle.Italic); else if (ssaStyle.Bold) subItem.Font = new Font(...., FontStyle.Bold); else if (ssaStyle.Italic) subItem.Font = new Font(...., FontStyle.Italic); else if (ssaStyle.Italic) subItem.Font = new Font(...., FontStyle.Regular); .... } ``` Всего анализатор выдал 4 предупреждения на этот фрагмент кода\. И это не удивительно, ведь тут ошибка почти в каждой строке\. Более того, тут не рассмотрен вариант с _ssaStyle\.Underline_\! Код лучше переписать на что\-то подобное и сделать это очень внимательно: ```cpp .... if (ssaStyle.Bold) fontStyles |= FontStyle.Bold; .... subItem.Font = new Font(...., fontStyles); .... ``` ## Не удаляется последний параграф текста [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) CWE\-570 Expression '\_networkSession \!\= null && \_networkSession\.LastSubtitle \!\= null && i < \_networkSession\.LastSubtitle\.Paragraphs\.Count' is always false\. Main\.cs 7242 ```cpp private void DeleteSelectedLines() { .... if (_networkSession != null) // <= { _networkSession.TimerStop(); NetworkGetSendUpdates(indices, 0, null); } else { indices.Reverse(); foreach (int i in indices) { _subtitle.Paragraphs.RemoveAt(i); if (_networkSession != null && // <= _networkSession.LastSubtitle != null && i < _networkSession.LastSubtitle.Paragraphs.Count) _networkSession.LastSubtitle.Paragraphs.RemoveAt(i); } .... } .... } ``` Переменная _\_networkSession_ уже проверена в первом условии, следовательно, в ветви _else_ она гарантированно будет иметь значение _null_\. Такое сочетание проверок привело к ложному условию и недостижимому коду\. ## Потеря функционала из\-за опечаток [V3003](https://pvs-studio.ru/ru/docs/warnings/v3003/) CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 113, 115\. SsaStyle\.cs 113 ```cpp public string ToRawSsa(string styleFormat) { var sb = new StringBuilder(); sb.Append("Style: "); var format = ....; for (int i = 0; i < format.Length; i++) { string f = format[i].Trim(); if (f == "name") sb.Append(Name); .... else if (f == "shadow") // <= sb.Append(OutlineWidth); // <= else if (f == "shadow") // <= sb.Append(ShadowWidth); // <= .... } .... } ``` Опечатки в каскаде условий приводят к появлению недостижимых ветвей кода\. Часто такой код является следствием Copy\-Paste программирования\. В приведённом примере второе продублированное условие никогда не будет выполнено\. И это самый простой и компактный пример, который я выбрал для статьи\. Таких примеров в проекте было найдено достаточно много, чтобы описать проблему в отдельном разделе\. Вот весь список Copy\-Paste кода, требующего исправления: * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 268, 270\. ExportCustomTextFormat\.cs 268 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 278, 280\. ExportCustomTextFormat\.cs 278 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 220, 252\. SetSyncPoint\.cs 220 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 712, 743\. ExportPngXml\.cs 712 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 712, 743\. ExportPngXml\.cs 712 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 162, 178\. LambdaCap\.cs 162 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 166, 182\. LambdaCap\.cs 166 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 170, 186\. LambdaCap\.cs 170 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 174, 190\. LambdaCap\.cs 174 * V3003 CWE\-570 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 398, 406\. Ebu\.cs 398 * V3021 CWE\-561 There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless FinalCutProTest2Xml\.cs 22 * V3021 CWE\-561 There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless FinalCutProTextXml\.cs 21 * V3021 CWE\-561 There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless FinalCutProXml\.cs 22 ## Что\-то не то с картинкой размером 720x480 [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) CWE\-570 Expression 'param\.Bitmap\.Width \=\= 720 && param\.Bitmap\.Width \=\= 480' is always false\. Probably the '\|\|' operator should be used here\. ExportPngXml\.cs 1808 ```cpp private static string FormatFabTime(TimeCode time, MakeBitmapParameter param) { if (param.Bitmap.Width == 720 && param.Bitmap.Width == 480) return $"...."; // drop frame if (Math.Abs(param.... - 24 * (999 / 1000)) < 0.01 || Math.Abs(param.... - 29 * (999 / 1000)) < 0.01 || Math.Abs(param.... - 59 * (999 / 1000)) < 0.01) return $"...."; return $"...."; } ``` Путаница с _Width_ и _Height_ является классическим примером опечатки\. Но в этой функции подозрительно ещё кое\-что\. Все сокращения строки, которые я заменил четырьмя многоточиями, это одна и та же строка: _\{time\.Hours:00\};\{time\.Minutes:00\};\{time\.Seconds:00\};\{SubtitleFormat\.MillisecondsToFramesMaxFrameRate\(time\.Milliseconds\):00\}_\. Т\.е\. наличие двух условий никак не влияет на результат функции, функция всегда возвращает одно и то же\. ## Загрузка "матрёшки" всегда успешна [V3009](https://pvs-studio.ru/ru/docs/warnings/v3009/) CWE\-393 It's odd that this method always returns one and the same value of 'true'\. Main\.cs 10153 ```cpp private bool LoadTextSTFromMatroska( MatroskaTrackInfo matroskaSubtitleInfo, MatroskaFile matroska, bool batchMode) { .... _fileDateTime = new DateTime(); _converted = true; if (batchMode) return true; SubtitleListview1.Fill(_subtitle, _subtitleAlternate); if (_subtitle.Paragraphs.Count > 0) SubtitleListview1.SelectIndexAndEnsureVisible(0); ShowSource(); return true; } ``` Найдена функция, которая всегда возвращает значение _true_\. Вероятно, это ошибка\. Значение этой функции проверяется в четырёх местах программы\. Также рядом в коде присутствуют похожие функции, например, _LoadDvbFromMatroska\(\),_ и она возвращает разные значения\. ## Бесполезный или ошибочный код [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) CWE\-571 Expression 'listBoxVobFiles\.Items\.Count \> 0' is always true\. DvdSubRip\.cs 533 ```cpp private void DvdSubRip_Shown(object sender, EventArgs e) { if (string.IsNullOrEmpty(_initialFileName)) return; if (_initialFileName.EndsWith(".ifo", ....)) { OpenIfoFile(_initialFileName); } else if (_initialFileName.EndsWith(".vob", ....)) { listBoxVobFiles.Items.Add(_initialFileName); buttonStartRipping.Enabled = listBoxVobFiles.Items.Count > 0; } _initialFileName = null; } ``` В список _listBoxVobFiles_ добавляется элемент, а потом проверяется, не пустой ли список\. Очевидно, что там будет как минимум один элемент\. И подобных проверок, которые всегда истинны или ложны, в проекте более тридцати\. ## Просто забавный пример [V3005](https://pvs-studio.ru/ru/docs/warnings/v3005/) The 'positionInfo' variable is assigned to itself\. WebVTT\.cs 79 ```cpp internal static string GetPositionInfoFromAssTag(Paragraph p) { .... if (!string.IsNullOrEmpty(line)) { if (positionInfo == null) positionInfo = " line:" + line; else positionInfo = positionInfo += " line:" + line; } .... } ``` Выбирая между вариантами записи "A \= A \+ n" и "A \+\= n", автор этого кода выбрал компромиссный вариант "A \= A \+\= n" :D ## Заключение Для понимания, как исправить то или иное предупреждение анализатора, нужно немного разбираться в форматах субтитров и особенностях их обработки\. Поэтому если есть желающие поддержать проект и покидать Pull Request'ы с исправлениями автору проекта на [GitHub](https://github.com/SubtitleEdit/subtitleedit), то вот [ссылка для загрузки](http://cppfiles.com/subtitleedit-html-log.7z) HTML отчёта с предупреждениями PVS\-Studio уровня High/Medium\.