Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse Subtitles During Extraction BUG #2052

Closed
1 task
FongMi opened this issue Jan 17, 2025 · 7 comments
Closed
1 task

Parse Subtitles During Extraction BUG #2052

FongMi opened this issue Jan 17, 2025 · 7 comments
Assignees
Labels

Comments

@FongMi
Copy link

FongMi commented Jan 17, 2025

Version

Media3 1.5.1

More version details

This video file can't not play after 1.4.0-alpha02
1.4.0-alpha01 can play well even if have error.
https://drive.google.com/file/d/18fYNCarTAbN84679hBS_KKobqRDWKlir/view?usp=drive_link

textTrackTranscodingEnabled need to set false
parseSubtitlesDuringExtraction need to set false
legacyDecodingEnabled need to set true

The above settings can be made to play in 1.5.1

Devices that reproduce the issue

Xiaomi 13

Devices that do not reproduce the issue

None

Reproducible in the demo app?

Yes

Reproduction steps

  1. add file path to media.exolist.json

Expected result

Play well

Actual result

See error

Media

https://drive.google.com/file/d/18fYNCarTAbN84679hBS_KKobqRDWKlir/view?usp=drive_link

Bug Report

@icbaker
Copy link
Collaborator

icbaker commented Jan 17, 2025

Actual result

See error

What is the error?

@icbaker icbaker self-assigned this Jan 17, 2025
@FongMi
Copy link
Author

FongMi commented Jan 17, 2025

Image

1.4.0-alpha01 can see the error too, but can play video.
1.4.0-alpha02~1.5.1 can see the error and stop play.

@icbaker
Copy link
Collaborator

icbaker commented Jan 17, 2025

This looks like a duplicate of #1722

Can you please try with 1.6.0-alpha01?

@FongMi
Copy link
Author

FongMi commented Jan 17, 2025

1.6.0-alpha01 is same. not play continued

Image

@icbaker
Copy link
Collaborator

icbaker commented Jan 17, 2025

Thanks - the changes in #1722 don't work here because the exception takes down the whole MatroskaExtractor and its Loadable, not just a Loadable being used only for subtitle playback.

Playback of your broken video can be permitted by adding try / catch (RuntimeException) around the call to parse here:

currentSubtitleParser.parse(
sampleData,
sampleStart,
size,
SubtitleParser.OutputOptions.allCues(),
cuesWithTiming -> outputSample(cuesWithTiming, timeUs, flags));

But I'm not sure the best way to signal that error out of the player in a programmatically visible, non-fatal way (rather than just logging it and continuing) - for consistency with the way we handle other subtitle parsing errors.

@FongMi FongMi closed this as completed Jan 19, 2025
@tonihei tonihei reopened this Jan 21, 2025
@icbaker
Copy link
Collaborator

icbaker commented Jan 22, 2025

Digging a bit further into the provided file, it has a cue that starts like this (i.e. has duration zero):

Dialogue: 0:00:00:00,0:00:00:00

The cue looks like it contains some conversion metadata, and is not intended to be displayed.

This currently trips up some fiddly logic in SsaParser that's designed to support overlapping subtitles (see google/ExoPlayer#6595), and results in it creating a single, empty, cue list - which then violates the assumption that every SSA cue line results in at least two lists of cues, one containing the text and another containing no cues (to signal the end), which ends up throwing the exception you're seeing.

I've got a change in review which fixes playback for your file by just ignoring cues where startTimeUs >= endTimeUs as invalid.

I'm also looking at submitting a change that suppresses+logs all parsing errors that bubble out as far as SubtitleTranscodingTrackOutput as described above - in order to maintain parity with 'legacy' subtitle decoding (where errors that bubble out this far are suppressed and logged in TextRenderer).

@FongMi
Copy link
Author

FongMi commented Jan 22, 2025

Thank you for your hard testing.

copybara-service bot pushed a commit that referenced this issue Jan 22, 2025
The file in Issue: #2052 contains a cue with the following timecode:

```
0:00:00:00,0:00:00:00
```

The content of this cue seems to be some 'converted by' metadata, i.e.
it's basically a comment and clearly not intended to be shown on
screen (since it has zero duration).

There is some fiddly logic later in `SsaParser` to support overlapping
cues with the old `Subtitle` structure [1], and this logic gets tripped
up by the start and end time being equal, which results in a
**single**, empty `List<Cue>` being added - which trips up another
assumption that every SSA cue line results in at least two `List<Cue>`
entries (one containing the cue text, and another containing an empty
list to signal the end of the cues).

This fiddly logic is no longer required, because overlapping
`CuesWithTiming` objects can now be merged in `TextRenderer`, so there
is a possible future simplification to `SsaParser` which removes a lot
of this complexity.

[1] Added in <unknown commit>

PiperOrigin-RevId: 718380386
copybara-service bot pushed a commit that referenced this issue Jan 23, 2025
This is equivalent to the error suppression for legacy subtitles in
`TextRenderer`:
https://github.com/androidx/media/blob/76088cd6af7f263aba238b7a48d64bd4f060cb8b/libraries/exoplayer/src/main/java/androidx/media3/exoplayer/text/TextRenderer.java#L357-L359

This new suppression only affects errors thrown from files with
subtitles muxed together with audio/video. Standalone subtitle
files, and containers containing only text tracks, are handled
by the existing error suppression/reporting added in
49dec5d.

Issue: #2052
PiperOrigin-RevId: 718930243
@icbaker icbaker closed this as completed Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants