Bug 461995

Summary: Time of day stripped from subtitles on load
Product: [Applications] kdenlive Reporter: Bill Smith <bill.smith14159>
Component: Video Effects & TransitionsAssignee: Jean-Baptiste Mardelle <jb>
Status: RESOLVED FIXED    
Severity: normal CC: jb
Priority: NOR    
Version First Reported In: 22.08.3   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In: 22.12.0
Sentry Crash Report:

Description Bill Smith 2022-11-18 14:23:50 UTC
SUMMARY
***
NOTE: If you are reporting a crash, please try to attach a backtrace with debug symbols.
See https://community.kde.org/Guidelines_and_HOWTOs/Debugging/How_to_create_useful_crash_reports
***


STEPS TO REPRODUCE
1. Add subtitle with the text /d:/d/d (e.g. a time like 4:15)
2. Save
3. Delete subtitle
4. Reload subtitles

OBSERVED RESULT
Any subtitle with characters matching regex ([0-9]{1,2}):([0-9]{2}) is ignored. (code treats lines that contain this regex as times on the timeline rather than text to be rendered.  This means that you can't have a subtitle that contains the time of day, for example)

EXPECTED RESULT
Presumably kdenlive should render any subtitle that contains printable characters.


SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: Linux 5.15.0-53-generic #59-Ubuntu SMP Mon Oct 17 18:53:30 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 5.92.0
Qt Version: 5.15.3

ADDITIONAL INFORMATION
The issue occurs in subtitlemodel.cpp commit SHA 81d3e6610c9998f56dbbba80a37807fa7bd5b6c2  line 201
https://invent.kde.org/multimedia/kdenlive/-/blob/d592484a4e2dac7c723bc8ad088f8ac33fb63183/src/bin/model/subtitlemodel.cpp#L201

where QRegularExpression rx("([0-9]{1,2}):([0-9]{2})") is used as a criterion for identifying a line of the srt file as being a timecode line for the subtitle.  This is too general a criterion in that it prevents users from having subtitles containing the time of day for example.

I don't have experience with .srt files (in terms of how major software packages are conforming to the 'standard') but judging by the srt file spec, if you multiline match 
(\d+)\n(\d\d:\d\d:\d\d),(\d+) --> (\d\d:\d\d:\d\d),(\d+)\n(.*)\n\n[EOF|(\d+)\n(\d\d:\d\d:\d\d),(\d+) --> (\d\d:\d\d:\d\d),(\d+)\n
or something like this, then (.*)  would allow the user to render most printable text (after making sure that everything in (.*) is actually renderable) -- everything except something that looked like the first two lines of a subtitle entry in an srt file.
Comment 1 Jean-Baptiste Mardelle 2022-11-20 06:44:44 UTC
Thanks for your report, confirmed
Comment 2 Jean-Baptiste Mardelle 2022-11-20 15:53:11 UTC
Git commit 45042d1a6e101b6148e43865ad28922b1e0a9ac1 by Jean-Baptiste Mardelle.
Committed on 20/11/2022 at 15:52.
Pushed by mardelle into branch 'release/22.12'.

Fix incorrect loading of subtitle with two dots.
FIXED-IN: 22.12.0

M  +9    -3    src/bin/model/subtitlemodel.cpp
A  +10   -0    tests/dataset/01.sbv
A  +7    -0    tests/dataset/01.vtt
M  +1    -0    tests/dataset/02.srt
A  +7    -0    tests/dataset/subs-with-two-dots.srt
M  +34   -2    tests/subtitlestest.cpp

https://invent.kde.org/multimedia/kdenlive/commit/45042d1a6e101b6148e43865ad28922b1e0a9ac1