search

carsonip / Penguin-Subtitle-Player

An open-source, cross-platform standalone subtitle player
GNU General Public License v3.0
304 stars 38 forks source link

SSA File Parser: Hard-Coding Related Failures #84

Closed murrayi closed 3 years ago

murrayi commented 3 years ago

First of all, really cool Subtitle Player! Just noticed a few improvements to make the ssa parser a bit more resilient. https://github.com/carsonip/Penguin-Subtitle-Player/blob/master/src/parsers/ssaparser.cpp

1.) New-Lines and White-Space Stripping Each line should be able to strip (or skip) new-lines and white-spaces 2.) Not Hard-coding Character Removals This runs the risk of assuming that word appeared on the expected line. For example, lines like formatLine = formatLine.remove(0, 8); should really be detecting if the line starts with "Format:" after stripping white-spacing/new-lines as suggested above in 1.)

That being said, I just want to point out a few related issues that lead to either crashes or failures to read the dialogue lines. (Note: "\n" represents a new line, and not literally those characters)

A.) Any leading/trailing white-space on the "[Events]" line will result in 00:00:00 timestamps as none of the dialogue lines will get read in. B.) "[Events]\n" causes the program to crash. This is because your parser expects the very next line after "[Events]" to contain the string "Format: " C.) "\nDialogue: " Will cut-off any subsequent Dialogue lines

carsonip commented 3 years ago

Thanks for your support! I really appreciate your taking the time to read the code. And you are right: the program should fail gracefully on invalid input. I'll fix the parsing code.