Bug 206412

Summary: [MSE] Decode glitches when watching videos on CNN.com
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: RESOLVED FIXED    
Severity: Normal CC: aboya, calvaris, commit-queue, eric.carlson, ews-watchlist, glenn, philipj, sergio
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Jer Noble
Reported 2020-01-17 06:41:42 PST
[MSE] Decode glitches when watching videos on CNN.com
Attachments
Patch (10.26 KB, patch)
2020-01-17 06:56 PST, Jer Noble 		no flags
DetailsFormatted DiffDiff
Patch for landing (11.01 KB, patch)
2020-01-17 10:11 PST, Jer Noble 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jer Noble
Comment 1 2020-01-17 06:42:49 PST
<rdar://problem/55685630>
Jer Noble
Comment 2 2020-01-17 06:56:55 PST
Created attachment 388034 [details] Patch
Alicia Boya García
Comment 3 2020-01-17 08:08:47 PST
Comment on attachment 388034 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388034&action=review Looks good to me. > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1680 > + // When appending media containing P-frames (media whose samples' presentation timestamps I think you mean B-frames. A movie with only I-frames and P-frames has monotonic presentation timestamps, and usually PTS=DTS. > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1691 > + auto nextDecodedSample = trackBuffer.samples.decodeOrder().findSampleAfterDecodeKey(decodeKey); nextDecodeSample, nextSampleDTS, nextSampleInDecodeOrder or even just nextSample. > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1699 > + if (nextSyncSample == nextDecodedSample) The only way this could be true is if nextDecodeSample == end(), but that was checked already above.
Xabier Rodríguez Calvar
Comment 4 2020-01-17 08:13:40 PST
Comment on attachment 388034 [details] Patch Thanks Alicia!
Jer Noble
Comment 5 2020-01-17 09:11:33 PST
(In reply to Alicia Boya García from comment #3) > Comment on attachment 388034 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=388034&action=review > > Looks good to me. > > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1680 > > + // When appending media containing P-frames (media whose samples' presentation timestamps > > I think you mean B-frames. A movie with only I-frames and P-frames has > monotonic presentation timestamps, and usually PTS=DTS. Whoops, looks like I got that... backwards. > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1691 > > + auto nextDecodedSample = trackBuffer.samples.decodeOrder().findSampleAfterDecodeKey(decodeKey); > > nextDecodeSample, nextSampleDTS, nextSampleInDecodeOrder or even just > nextSample. As you can tell from SampleMap, I don’t mind long, descriptive names, so maybe I’ll go with “nextSampleInDecodeOrder”. > > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1699 > > + if (nextSyncSample == nextDecodedSample) > > The only way this could be true is if nextDecodeSample == end(), but that > was checked already above. Good catch!
Jer Noble
Comment 6 2020-01-17 10:11:35 PST
Created attachment 388053 [details] Patch for landing
WebKit Commit Bot
Comment 7 2020-01-17 13:03:59 PST
Comment on attachment 388053 [details] Patch for landing Clearing flags on attachment: 388053 Committed r254761: <https://trac.webkit.org/changeset/254761>
WebKit Commit Bot
Comment 8 2020-01-17 13:04:01 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.