Exoplayer: Crash with ArrayIndexOutOfBoundsException from SsaDecoder

[stari4ek]

Issue description

App crashes with callstack

Critical error: java.lang.ArrayIndexOutOfBoundsException
length=7; index=10
com.google.android.exoplayer2.text.ssa.SsaDecoder.parseDialogueLine (SsaDecoder.java:186)
com.google.android.exoplayer2.text.ssa.SsaDecoder.parseEventBody (SsaDecoder.java:120)
com.google.android.exoplayer2.text.ssa.SsaDecoder.decode (SsaDecoder.java:83)
com.google.android.exoplayer2.text.ssa.SsaDecoder.decode (SsaDecoder.java:35)
com.google.android.exoplayer2.text.SimpleSubtitleDecoder.decode (SimpleSubtitleDecoder.java:70)
com.google.android.exoplayer2.text.SimpleSubtitleDecoder.decode (SimpleSubtitleDecoder.java:25)
com.google.android.exoplayer2.decoder.SimpleDecoder.decode (SimpleDecoder.java:222)
com.google.android.exoplayer2.decoder.SimpleDecoder.run (SimpleDecoder.java:188)
com.google.android.exoplayer2.decoder.SimpleDecoder.decode (SimpleDecoder.java:25)
com.google.android.exoplayer2.decoder.SimpleDecoder$1.run (SimpleDecoder.java:66)

Reproduction steps

Link to test content

I'm getting this issues as automatically submitted in crash reporting system, so I don't have repro or failing media.
It's SSA track (mimeType=text/x-ssa, language=eng) from mkv media streamed from _vaders.tv_ service

Version of ExoPlayer being used

2.6.1

Device(s) and version(s) of Android being used

NVidia Shield, Android 7.0

Notes

Is it expected behavior of exoplayer that all exceptions raised in SimpleDecoder aren't routed via callbacks? There were issues before: https://github.com/google/ExoPlayer/issues/3396, https://github.com/google/ExoPlayer/issues/2957 which got fixes in processors/parsers. But are those parsers (subtitles and co.) ready for users' media, which can be broken?

[ojw28]

There are two parts to this:

1. We intend for our text decoders to be robust against malformed content, meaning we want them to log warnings and carry on rather than failing playbacks. An approach where decoder errors are caught and suppressed at the top level doesn't work well for achieving this, since it's hard to verify that the decoder state is sensible to carry on (playback would never fail, but equally the text decoder might never recover to decode subsequent samples that are formatted correctly). Hence this should be implemented within each individual decoder. I'll make a change to SsaDecoder to add robustness for the failure above.
2. If a decoder encounters malformed content that it's not robust against then we should probably not crash the process. As you note, it would be better to route the failure via callbacks in this case. We will fix this also.

[ojw28]

SsaDecoder has been made robust against this issue, and this change has been included in 2.6.1. A subsequent change avoids crashing the process will land in dev-v2 shortly, for inclusion in subsequent releases.