-
Notifications
You must be signed in to change notification settings - Fork 408
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
WebvttParser
creates duplicate CuesWithTiming
when handling cues sharing same start/end timestamps
#1177
Comments
The duplication is expected for cues that actually overlap, that's checked in a test case here: media/libraries/extractor/src/test/java/androidx/media3/extractor/text/webvtt/WebvttParserTest.java Line 338 in 12aa637
This duplication is currently required due to the way we implement WebVTT's simultaneous cues handling (by creating synthetic media/libraries/extractor/src/main/java/androidx/media3/extractor/text/webvtt/WebvttSubtitle.java Line 81 in 12aa637
If we didn't create these synthetic cues here, we would have lost too much information to correctly implement the layout rules later in the playback pipeline. It's still not perfect (see e.g. google/ExoPlayer#10980), and it would be better to implement this layout logic later, but it's not likely something we're going to change soon. However, this duplication is not expected for consecutive cues where one cue ends at the same time the next cue starts. I can reproduce that the duplication does occur in this case, I'll look into fixing it. |
Thanks for the quick and detailed response! Edited the title since I didn't intend to work with subtitles with overlapping timestamp |
WebvttParser
generates duplicated CuesWithTiming
when handling overlapped timestampsWebvttParser
creates duplicate CuesWithTiming
when handling cues sharing same start/end timestamps
It's a bit arguable whether the `Subtitle` implementation supports zero-duration events, since `getEventTimeCount` is documented as effectively "the number of times the cues returns by `getCues(long)` changes", and zero-duration events violate that. However, the current `WebvttSubtitle` impl **does** produce zero-duration events, so it seems safer to handle them gracefully here and then, as a possible follow-up, fix the `WebvttSubtitle` impl (or remove it completely). Issue: #1177 #minor-release PiperOrigin-RevId: 616095798
It's a bit arguable whether the `Subtitle` implementation supports zero-duration events, since `getEventTimeCount` is documented as effectively "the number of times the cues returns by `getCues(long)` changes", and zero-duration events violate that. However, the current `WebvttSubtitle` impl **does** produce zero-duration events, so it seems safer to handle them gracefully here and then, as a possible follow-up, fix the `WebvttSubtitle` impl (or remove it completely). Issue: androidx/media#1177 #minor-release PiperOrigin-RevId: 616095798
@icbaker as I understood this was just a preparation to handle overlapping vtt cues?
Do you have suggestions on the "layout logic"? |
@szaboa I'm not sure I completely understand the question - do you mind filing a new question issue with a bit more detail? (I'm likely to lose track of this closed one) |
It's a bit arguable whether the `Subtitle` implementation supports zero-duration events, since `getEventTimeCount` is documented as effectively "the number of times the cues returns by `getCues(long)` changes", and zero-duration events violate that. However, the current `WebvttSubtitle` impl **does** produce zero-duration events, so it seems safer to handle them gracefully here and then, as a possible follow-up, fix the `WebvttSubtitle` impl (or remove it completely). Issue: #1177 PiperOrigin-RevId: 616095798 (cherry picked from commit e9ed874)
It's a bit arguable whether the `Subtitle` implementation supports zero-duration events, since `getEventTimeCount` is documented as effectively "the number of times the cues returns by `getCues(long)` changes", and zero-duration events violate that. However, the current `WebvttSubtitle` impl **does** produce zero-duration events, so it seems safer to handle them gracefully here and then, as a possible follow-up, fix the `WebvttSubtitle` impl (or remove it completely). Issue: androidx#1177 PiperOrigin-RevId: 616095798 (cherry picked from commit e9ed874)
Version
Media3 1.3.0
More version details
media3-extractor
Devices that reproduce the issue
Android Studio
Devices that do not reproduce the issue
No response
Reproducible in the demo app?
Not tested
Reproduction steps
Use
WebvttParser.parse()
to parse the vtt captions below:Expected result
The
cuesWithTimingList
contains 3 items or linesActual result
The list contains 4 items, see screenshot
Media
Bug Report
adb bugreport
to android-media-github@google.com after filing this issue.The text was updated successfully, but these errors were encountered: