Bug 190638 – Safari is not able to adapt between H264 streams with EditList and without EditList

• Component: Media
• Status: RESOLVED FIXED

[KongQun Yang]

To re-pro, 

1. Go to https://shaka-player-demo.appspot.com/demo/#asset=https://storage.googleapis.com/wvtest/shaka-packager/historical/v2.2/misc/ClearPackaged/clear_tears.mpd;lang=en-US;build=uncompiled
2. select Load and Begin playback.
3. Tears begins and after a few seconds into playback responds with: Shaka Error MEDIA.VIDEO_ERROR (3,,) with spinner on screen

The problem occurs when player was trying to adapt to a higher resolution.

It is common to see H264 streams with base profile and main profile in the same AdaptationSet. The H264 stream with base profile will not have EditList while it is common for the H264 stream with main profile to contain an EditList.


The same manifest works in other browsers. I don't see it being disallowed in the spec either.

[Radar WebKit Bug Importer]

<rdar://problem/45342208>

[Jer Noble]

Here's the logging from a debug build of WebKit:

SourceBufferPrivateAVFObjC::layerDidReceiveError(0x553ef7d00): layer(0x7f8d9f75cbb0), error(Error Domain=AVFoundationErrorDomain Code=-11800 "The operation could not be completed" UserInfo={AVErrorMediaSubTypeKey=(
    1635148593
), NSLocalizedDescription=The operation could not be completed, NSLocalizedFailureReason=An unknown error occurred (-8969), AVErrorMediaTypeKey=vide, AVErrorPresentationTimeStampKey=CMTime: {184832/15360 = 12.033}, NSUnderlyingError=0x7f8da0d28030 {Error Domain=NSOSStatusErrorDomain Code=-8969 "codecBadDataErr"}})

Looking for that timestamp in the enqueue log leads to:

SourceBufferPrivateAVFObjC::enqueueSample(0x553ef7d00) - sample({PTS({184320/15360 = 12.000000}), OPTS({184320/15360 = 12.000000}), DTS({184320/15360 = 12.000000}), duration({512/15360 = 0.033333}), flags(1), presentationSize(318.133026x142.000000)})
SourceBufferPrivateAVFObjC::enqueueSample(0x553ef7d00) - sample({PTS({185344/15360 = 12.066667}), OPTS({185344/15360 = 12.066667}), DTS({184320/15360 = 12.000000}), duration({512/15360 = 0.033333}), flags(1), presentationSize(1917.759644x856.000000)})
SourceBufferPrivateAVFObjC::enqueueSample(0x553ef7d00) - sample({PTS({184832/15360 = 12.033333}), OPTS({184832/15360 = 12.033333}), DTS({184832/15360 = 12.033333}), duration({512/15360 = 0.033333}), flags(0), presentationSize(318.133026x142.000000)})
SourceBufferPrivateAVFObjC::enqueueSample(0x553ef7d00) - sample({PTS({186368/15360 = 12.133333}), OPTS({186368/15360 = 12.133333}), DTS({184832/15360 = 12.033333}), duration({512/15360 = 0.033333}), flags(0), presentationSize(1917.759644x856.000000)})

So somehow we are leaving in the decoding queue two sets of samples with the same DTS.

[Jer Noble]

I don't think this has anything to do with edit lists, per se. But rather, there's a bug either in the spec or in our implementation that's particular to B-frames. The removal algorithm mandates the UA remove samples who overlap in presentation time, but in this case, the samples do not overlap in PTS at all. Rather, they overlap in decode time.

[Jer Noble]

Actually, those samples should not be removed, because that would result in a "gap" in presentation order from 184320/15360 -> 185344/15360. One other solution would be to re-order the decodeQueue to enqueue samples out-of-DTS order, but that might cause problems in the decoder.

[Jer Noble]

I think the best solution we can hope for is, when were are re-enqueuing samples after a removal, we simply fail to enqueue samples which indicate a size change, but are not sync-samples.

[Jer Noble]

Okay, I've come around 180º from my previous opinion. I think that those frames whose decode times are after the replacement frame's decode times should be removed as well. I have a patch which implements and tests this.

[Jer Noble]

Created attachment 352706 [details]
Patch

[Eric Carlson]

Comment on attachment 352706 [details]
Patch

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

> Source/WebCore/Modules/mediasource/SampleMap.cpp:316
> +    // endKey is not inclusive, so use lower_bound to exclude samples which start exactly at endKey.

Nit: don't you mean "... so use upper_bound to ..."

> LayoutTests/media/media-source/media-source-append-overlapping-dts.html:4
> +    <title>mock-media-source</title>

Nit: a more descriptive title would be nice.

[Jer Noble]

Comment on attachment 352706 [details]
Patch

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

>> Source/WebCore/Modules/mediasource/SampleMap.cpp:316
>> +    // endKey is not inclusive, so use lower_bound to exclude samples which start exactly at endKey.
> 
> Nit: don't you mean "... so use upper_bound to ..."

Nope, lower_bound.  STL iterators are always inclusive-start, exclusive-end, so using lower_bound ensures that anything exactly matching endKey is excluded from the range.

>> LayoutTests/media/media-source/media-source-append-overlapping-dts.html:4
>> +    <title>mock-media-source</title>
> 
> Nit: a more descriptive title would be nice.

good point.

[Jer Noble]

Created attachment 352741 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 352741 [details]
Patch for landing

Clearing flags on attachment: 352741

Committed r237271: <https://trac.webkit.org/changeset/237271>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alicia Boya García]

(In reply to KongQun Yang from comment #0)
> It is common to see H264 streams with base profile and main profile in the
> same AdaptationSet. The H264 stream with base profile will not have EditList
> while it is common for the H264 stream with main profile to contain an
> EditList.

This comment scared me a bit, since edit list displace the timestamps of the movies and in general, you don't want adaption sets to be offsetted with each other.

Later I checked the files and it's fine in this case: the edit list in the main profile AdaptationSet comes just from the fact that the main profile (unlike the baseline profile) uses B-frames, and coding B-frames in MP4 causes offsetting of frames which has to be compensated with an edit list.