Bug 133365 – Captions during HLS playback flash/are difficult to read

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Brent Fulgham]

Playing certain HLS media with captions results in "flashing" cues, which are difficult to read.

This is happening for two reasons:

1. HLS streams may send multiple copies of the same cue, with small differences in time values. For example, a closed caption encoding device on a live video feed might inject track cues at roughly 24 fps, with multiple cues containing the same text content, but having different start and end times. This results in cues with a 'duration' on the order of hundredths of a second, which are too quick to read.
2. A bug I introduced in <http://trac.webkit.org/changeset/166180> caused us to miss some non-WebVTT cues. This would happen when WebKit improperly decided a new cue matched an existing cue. These missing cues contributed to the "flashing" behavior.

This bug corrects both of these issues.

[Brent Fulgham]

<rdar://problem/16584132>

[Brent Fulgham]

Created attachment 232223 [details]
Patch

[Eric Carlson]

Comment on attachment 232223 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232223&action=review

Thanks!

> Source/WebCore/html/track/DataCue.cpp:-131
> -    if (cue.cueType() != TextTrackCue::Data)
> -        return false;
> -

Is it necessary to remove this? If so, how can it be safe given that we call toDataCue() below (which will ASSERT if passed a non-datacue)?

> Source/WebCore/html/track/TextTrackCue.cpp:242
> +bool TextTrackCue::isStartOfCue(const TextTrackCue& cue) const

I am not wild about this name because I didn't know what it was supposed to check until I found it and read the code. How about something like "isCueExtension", or "extendsCue"? Or something else...

[Brent Fulgham]

Comment on attachment 232223 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232223&action=review

>> Source/WebCore/html/track/DataCue.cpp:-131
>> -
> 
> Is it necessary to remove this? If so, how can it be safe given that we call toDataCue() below (which will ASSERT if passed a non-datacue)?

It's still part of the "isEqual" method (see below), but I'll add it back here so we can avoid ASSERTing.

>> Source/WebCore/html/track/TextTrackCue.cpp:242
>> +bool TextTrackCue::isStartOfCue(const TextTrackCue& cue) const
> 
> I am not wild about this name because I didn't know what it was supposed to check until I found it and read the code. How about something like "isCueExtension", or "extendsCue"? Or something else...

I'll use "doesExtendCue".

[Brent Fulgham]

Committed r169440: <http://trac.webkit.org/changeset/169440>