135855 2014-08-12 14:50:25 -0700 [MSE] YouTube will lose audio, video after seeking backwards to an unbuffered range. 2018-04-19 09:20:09 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 135867 1 jer.noble jer.noble aboya calvaris commit-queue eric.carlson glenn ltilve philipj sergio webkit-bug-importer oldest_to_newest 1028612 0 jer.noble 2014-08-12 14:50:25 -0700 [MSE] YouTube will lose audio, video after seeking backwards to an unbuffered range. 1028616 1 236471 jer.noble 2014-08-12 14:56:50 -0700 Created attachment 236471 Patch 1028666 2 jer.noble 2014-08-12 16:37:50 -0700 Committed r172506: <http://trac.webkit.org/changeset/172506> 1056090 3 jer.noble 2014-12-16 13:26:53 -0800 <rdar://problem/19269013> 1415184 4 236471 aboya 2018-04-17 09:46:05 -0700 Comment on attachment 236471 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236471&action=review > Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1389 > + // Do not enqueue samples spanning a significant unbuffered gap: Why not avoid pushing such samples to the decodeQueue in the first place? 1415188 5 236471 jer.noble 2018-04-17 09:56:19 -0700 Comment on attachment 236471 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=236471&action=review >> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1389 >> + // Do not enqueue samples spanning a significant unbuffered gap: > > Why not avoid pushing such samples to the decodeQueue in the first place? You would also have to guarantee that removing a range of samples from the decodeQueue (as in removeSamplesFromTrackBuffer) also removed any samples that resulted in a large gap. 1415940 6 aboya 2018-04-19 09:20:09 -0700 (In reply to Jer Noble from comment #5) > Comment on attachment 236471 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=236471&action=review > > >> Source/WebCore/Modules/mediasource/SourceBuffer.cpp:1389 > >> + // Do not enqueue samples spanning a significant unbuffered gap: > > > > Why not avoid pushing such samples to the decodeQueue in the first place? > > You would also have to guarantee that removing a range of samples from the > decodeQueue (as in removeSamplesFromTrackBuffer) also removed any samples > that resulted in a large gap. Thanks for replying. I think I get it; but let's use an example to make sure we're on the same page. For this example PTS=DTS and all frames are sync frames (as is the case with audio) and every frame is 1 second long to make it simpler. Playback has not started yet and the first 10 seconds of audio were just appended. decodeQueue == trackBuffer.samples == [0, 1, 2, 3, 4, 5, 6, 7, 8, 9] Now the user calls SourceBuffer.remove(4, 7), which in turns calls removeSamplesFromTrackBuffer([4, 5, 6]). These are the resulting frames in the track buffer: decodeQueue == trackBuffer.samples = [0, 1, 2, 3, 7, 8, 9] So the condition discussed above is necessary to avoid [7, 8, 9] to be actually enqueued until [4, 5, 6] are re-appended, right? Let's say right after this we append a frame with DTS=10. sourceBufferPrivateDidReceiveSample() is called and we reach this condition: if (trackBuffer.lastEnqueuedDecodeEndTime.isInvalid() || decodeTimestamp >= trackBuffer.lastEnqueuedDecodeEndTime) { DecodeOrderSampleMap::KeyType decodeKey(sample.decodeTime(), sample.presentationTime()); trackBuffer.decodeQueue.insert(DecodeOrderSampleMap::MapType::value_type(decodeKey, &sample)); } decodeTimestamp == lastEnqueuedDecodeEndTime == 10, so the condition is true and 10 is added to the decodeQueue. decodeQueue == trackBuffer.samples == [0, 1, 2, 3, 7, 8, 9, 10] lastEnqueuedDecodeEndTime == 4 At the end of the append, provideMediaData() is called. The loop enqueues the first 4 frames successfully. In the next iteration it reaches DTS=7 and breaks the loop in the condition we're discussing. The problem I see is that DTS=7 is still removed from the decodeQueue because the condition is after the `.erase()`. The next frames (8 to 10) remain in the decodeQueue, at least until the next call. I'm not the first person to point this problem: https://bugs.webkit.org/show_bug.cgi?id=180643 236471 2014-08-12 14:56:50 -0700 2014-08-12 16:20:27 -0700 Patch bug-135855-20140812145639.patch text/plain 5729 jer.noble U3VidmVyc2lvbiBSZXZpc2lvbjogMTcyNDcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYjRmOGZjNGEwMDgxNjVm MjEzNjBmMDc1YjVmYzU3YWI5YmExMzNiZC4uZWYwMjlhM2E4YTAwYTdhOWY1YjJjNmY4MzcyZmRj MmYyZmJmYzZmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI5IEBACisyMDE0LTA4LTEyICBKZXIg Tm9ibGUgIDxqZXIubm9ibGVAYXBwbGUuY29tPgorCisgICAgICAgIFtNU0VdIFlvdVR1YmUgd2ls bCBsb3NlIGF1ZGlvLCB2aWRlbyBhZnRlciBzZWVraW5nIGJhY2t3YXJkcyB0byBhbiB1bmJ1ZmZl cmVkIHJhbmdlLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTM1ODU1CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgV2hlbiBzZWVraW5nIGludG8gYW4gdW5idWZmZXJlZCBvciBwYXJ0aWFsbHkgYnVmZmVyZWQg cmFuZ2UsIHdlIHdpbGwgdW5jb25kaXRpb25hbGx5IHBhc3Mgc2FtcGxlcyB0byB0aGUKKyAgICAg ICAgZGVjb2RlIHF1ZXVlIGV2ZW4gaWYgdGhlcmUgZXhpc3QgbGFyZ2UgZ2FwcyBiZXR3ZWVuIHRo b3NlIHNhbXBsZXMuIFN1YnNlcXVlbnRseSwgdGhlIGRlY29kZXIgd2lsbCBub3QKKyAgICAgICAg bm90aWZ5IHVzIHRoYXQgaXQgaGFzIGJlY29tZSByZWFkeSBmb3IgbmV3IHNhbXBsZXMgdW50aWwg cGxheWJhY2sgcmVhY2hlcyB0aG9zZSBsYXRlciBzYW1wbGVzIGFuZCB0aGUgc2FtcGxlcworICAg ICAgICBhcmUgZGlzY2FyZGVkLgorCisgICAgICAgIFdoZW4gc2VuZGluZyBzYW1wbGVzIHRvIGJl IGRlY29kZWQgaW4gcHJvdmlkZU1lZGlhRGF0YSgpLCBzdG9wIGlmIHRoZXJlIGV4aXN0cyBhIGxh cmdlIGdhcCBpbiB0aGUgc2FtcGxlIHRpbWVsaW5lLgorICAgICAgICBEbyB0aGlzIGJ5IHRyYWNr aW5nIHRoZSBsYXN0IGVucXVldWVkIGRlY29kZSBlbmQgdGltZSwgYW5kIGxvb2sgdG8gc2VlIGlm IHRoZSBuZXh0IHNhbXBsZSdzIGRlY29kZSB0aW1lIGluZGljYXRlcworICAgICAgICBhIGdhcCBv ZiBncmVhdGVyIHRoYW4gMSBzZWNvbmQuCisKKyAgICAgICAgKiBNb2R1bGVzL21lZGlhc291cmNl L1NvdXJjZUJ1ZmZlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpTb3VyY2VCdWZmZXI6OlRyYWNr QnVmZmVyOjpUcmFja0J1ZmZlcik6IEluaXRpYWxpemUgbGFzdEVucXVldWVkRGVjb2RlRW5kVGlt ZS4KKyAgICAgICAgKFdlYkNvcmU6OlNvdXJjZUJ1ZmZlcjo6c2Vla1RvVGltZSk6IFNldCBuZWVk c1JlZW5xdWV1ZWluZywgaW4gY2FzZSBzYW1wbGVzIGRvIG5vdCB5ZXQgZXhpc3QgaW4gdGhlIHRy YWNrQnVmZmVyCisgICAgICAgICAgICBzdWZmaWNpZW50IHRvIHJlLWVucXVldWUgZm9yIHRoZSBk ZXN0aW5hdGlvbiB0aW1lLCBzbyB0aGF0IHJlLWVucXVldWVpbmcgd2lsbCBvY2N1ciBhZnRlciB0 aGUgbmV4dCBhcHBlbmQuCisgICAgICAgIChXZWJDb3JlOjpTb3VyY2VCdWZmZXI6OnNvdXJjZUJ1 ZmZlclByaXZhdGVEaWRSZWNlaXZlU2FtcGxlKTogQ2hlY2sgYWdhaW5zdCBsYXN0RW5xdWV1ZWRE ZWNvZGVFbmRUaW1lLCByYXRoZXIgdGhhbgorICAgICAgICAgICAgbGFzdEVucXVldWVkUHJlc2Vu dGF0aW9uVGltZSBiZWZvcmUgYWRkaW5nIHNhbXBsZXMgdG8gdGhlIGRlY29kZVF1ZXVlLgorICAg ICAgICAoV2ViQ29yZTo6U291cmNlQnVmZmVyOjpwcm92aWRlTWVkaWFEYXRhKTogU3RvcCB3aGVu IHdlIHJlYWNoIGEgbGFyZ2UgZ2FwIGJldHdlZW4gc2FtcGxlcy4KKyAgICAgICAgKFdlYkNvcmU6 OlNvdXJjZUJ1ZmZlcjo6cmVlbnF1ZXVlTWVkaWFGb3JUaW1lKTogU2V0IG9yIGNsZWFyIGxhc3RF bnF1ZXVlZERlY29kZUVuZFRpbWUgYmFzZWQgb24gd2hldGhlciB3ZQorICAgICAgICAgICAgaGF2 ZSBhcHBlbmRlZCBhbnkgbm9uLWRpc3BsYXlpbmcgc2FtcGxlcyBhZnRlciBmbHVzaGluZy4KKwog MjAxNC0wOC0xMiAgRmFiaWVuIFZhbGzDqWUgIDxmdmFsbGVlQGNvbm5lY3RlZC1sYWJzLmNvbT4K IAogICAgICAgICBbR1N0cmVhbWVyXSBwbGF5YmFjayByYXRlIGlzIHJvdW5kZWQgdG8gaW50ZWdl cgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXNvdXJjZS9Tb3VyY2VC dWZmZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXNvdXJjZS9Tb3VyY2VCdWZm ZXIuY3BwCmluZGV4IGM3YTI0Mjc4MTc2MmNkMWNlOTQyMjhiNDY2ZGE0NDI1YTdkNDljZTguLjMw ZDllY2Y3MTQzYWI3ZGE2ODA2MGFlZGI4MWNhYmE3NTI3MzA2NmYgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzb3VyY2UvU291cmNlQnVmZmVyLmNwcAorKysgYi9Tb3Vy Y2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc291cmNlL1NvdXJjZUJ1ZmZlci5jcHAKQEAgLTcxLDYg KzcxLDcgQEAgc3RydWN0IFNvdXJjZUJ1ZmZlcjo6VHJhY2tCdWZmZXIgewogICAgIE1lZGlhVGlt ZSBsYXN0RnJhbWVEdXJhdGlvbjsKICAgICBNZWRpYVRpbWUgaGlnaGVzdFByZXNlbnRhdGlvblRp bWVzdGFtcDsKICAgICBNZWRpYVRpbWUgbGFzdEVucXVldWVkUHJlc2VudGF0aW9uVGltZTsKKyAg ICBNZWRpYVRpbWUgbGFzdEVucXVldWVkRGVjb2RlRW5kVGltZTsKICAgICBib29sIG5lZWRSYW5k b21BY2Nlc3NGbGFnOwogICAgIGJvb2wgZW5hYmxlZDsKICAgICBib29sIG5lZWRzUmVlbnF1ZXVl aW5nOwpAQCAtODMsNiArODQsNyBAQCBzdHJ1Y3QgU291cmNlQnVmZmVyOjpUcmFja0J1ZmZlciB7 CiAgICAgICAgICwgbGFzdEZyYW1lRHVyYXRpb24oTWVkaWFUaW1lOjppbnZhbGlkVGltZSgpKQog ICAgICAgICAsIGhpZ2hlc3RQcmVzZW50YXRpb25UaW1lc3RhbXAoTWVkaWFUaW1lOjppbnZhbGlk VGltZSgpKQogICAgICAgICAsIGxhc3RFbnF1ZXVlZFByZXNlbnRhdGlvblRpbWUoTWVkaWFUaW1l OjppbnZhbGlkVGltZSgpKQorICAgICAgICAsIGxhc3RFbnF1ZXVlZERlY29kZUVuZFRpbWUoTWVk aWFUaW1lOjppbnZhbGlkVGltZSgpKQogICAgICAgICAsIG5lZWRSYW5kb21BY2Nlc3NGbGFnKHRy dWUpCiAgICAgICAgICwgZW5hYmxlZChmYWxzZSkKICAgICAgICAgLCBuZWVkc1JlZW5xdWV1ZWlu ZyhmYWxzZSkKQEAgLTMxNyw2ICszMTksNyBAQCB2b2lkIFNvdXJjZUJ1ZmZlcjo6c2Vla1RvVGlt ZShjb25zdCBNZWRpYVRpbWUmIHRpbWUpCiAgICAgICAgIFRyYWNrQnVmZmVyJiB0cmFja0J1ZmZl ciA9IHRyYWNrQnVmZmVyUGFpci52YWx1ZTsKICAgICAgICAgY29uc3QgQXRvbWljU3RyaW5nJiB0 cmFja0lEID0gdHJhY2tCdWZmZXJQYWlyLmtleTsKIAorICAgICAgICB0cmFja0J1ZmZlci5uZWVk c1JlZW5xdWV1ZWluZyA9IHRydWU7CiAgICAgICAgIHJlZW5xdWV1ZU1lZGlhRm9yVGltZSh0cmFj a0J1ZmZlciwgdHJhY2tJRCwgdGltZSk7CiAgICAgfQogfQpAQCAtMTIwOCw3ICsxMjExLDcgQEAg dm9pZCBTb3VyY2VCdWZmZXI6OnNvdXJjZUJ1ZmZlclByaXZhdGVEaWRSZWNlaXZlU2FtcGxlKFNv dXJjZUJ1ZmZlclByaXZhdGUqLCBQYXMKICAgICAgICAgLy8gQWRkIHRoZSBjb2RlZCBmcmFtZSB3 aXRoIHRoZSBwcmVzZW50YXRpb24gdGltZXN0YW1wLCBkZWNvZGUgdGltZXN0YW1wLCBhbmQgZnJh bWUgZHVyYXRpb24gdG8gdGhlIHRyYWNrIGJ1ZmZlci4KICAgICAgICAgdHJhY2tCdWZmZXIuc2Ft cGxlcy5hZGRTYW1wbGUoc2FtcGxlKTsKIAotICAgICAgICBpZiAodHJhY2tCdWZmZXIubGFzdEVu cXVldWVkUHJlc2VudGF0aW9uVGltZS5pc0ludmFsaWQoKSB8fCBwcmVzZW50YXRpb25UaW1lc3Rh bXAgPiB0cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWRQcmVzZW50YXRpb25UaW1lKSB7CisgICAgICAg IGlmICh0cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWREZWNvZGVFbmRUaW1lLmlzSW52YWxpZCgpIHx8 IGRlY29kZVRpbWVzdGFtcCA+PSB0cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWREZWNvZGVFbmRUaW1l KSB7CiAgICAgICAgICAgICBEZWNvZGVPcmRlclNhbXBsZU1hcDo6S2V5VHlwZSBkZWNvZGVLZXko ZGVjb2RlVGltZXN0YW1wLCBwcmVzZW50YXRpb25UaW1lc3RhbXApOwogICAgICAgICAgICAgdHJh Y2tCdWZmZXIuZGVjb2RlUXVldWUuaW5zZXJ0KERlY29kZU9yZGVyU2FtcGxlTWFwOjpNYXBUeXBl Ojp2YWx1ZV90eXBlKGRlY29kZUtleSwgc2FtcGxlKSk7CiAgICAgICAgIH0KQEAgLTEzODMsNyAr MTM4NiwxMyBAQCB2b2lkIFNvdXJjZUJ1ZmZlcjo6cHJvdmlkZU1lZGlhRGF0YShUcmFja0J1ZmZl ciYgdHJhY2tCdWZmZXIsIEF0b21pY1N0cmluZyB0cmFjawogICAgICAgICB9CiAKICAgICAgICAg UmVmUHRyPE1lZGlhU2FtcGxlPiBzYW1wbGUgPSBzYW1wbGVJdC0+c2Vjb25kOworICAgICAgICAv LyBEbyBub3QgZW5xdWV1ZSBzYW1wbGVzIHNwYW5uaW5nIGEgc2lnbmlmaWNhbnQgdW5idWZmZXJl ZCBnYXA6CisgICAgICAgIE1lZGlhVGltZSBvbmVTZWNvbmQoMSwgMSk7CisgICAgICAgIGlmICh0 cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWREZWNvZGVFbmRUaW1lLmlzVmFsaWQoKSAmJiBzYW1wbGUt PmRlY29kZVRpbWUoKSAtIHRyYWNrQnVmZmVyLmxhc3RFbnF1ZXVlZERlY29kZUVuZFRpbWUgPiBv bmVTZWNvbmQpCisgICAgICAgICAgICBicmVhazsKKwogICAgICAgICB0cmFja0J1ZmZlci5sYXN0 RW5xdWV1ZWRQcmVzZW50YXRpb25UaW1lID0gc2FtcGxlLT5wcmVzZW50YXRpb25UaW1lKCk7Cisg ICAgICAgIHRyYWNrQnVmZmVyLmxhc3RFbnF1ZXVlZERlY29kZUVuZFRpbWUgPSBzYW1wbGUtPmRl Y29kZVRpbWUoKSArIHNhbXBsZS0+ZHVyYXRpb24oKTsKICAgICAgICAgbV9wcml2YXRlLT5lbnF1 ZXVlU2FtcGxlKHNhbXBsZS5yZWxlYXNlKCksIHRyYWNrSUQpOwogI2lmICFMT0dfRElTQUJMRUQK ICAgICAgICAgKytlbnF1ZXVlZFNhbXBsZXM7CkBAIC0xNDI1LDggKzE0MzQsMTMgQEAgdm9pZCBT b3VyY2VCdWZmZXI6OnJlZW5xdWV1ZU1lZGlhRm9yVGltZShUcmFja0J1ZmZlciYgdHJhY2tCdWZm ZXIsIEF0b21pY1N0cmluZwogCiAgICAgbV9wcml2YXRlLT5mbHVzaEFuZEVucXVldWVOb25EaXNw bGF5aW5nU2FtcGxlcyhub25EaXNwbGF5aW5nU2FtcGxlcywgdHJhY2tJRCk7CiAKLSAgICBpZiAo IW5vbkRpc3BsYXlpbmdTYW1wbGVzLmlzRW1wdHkoKSkKKyAgICBpZiAoIW5vbkRpc3BsYXlpbmdT YW1wbGVzLmlzRW1wdHkoKSkgewogICAgICAgICB0cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWRQcmVz ZW50YXRpb25UaW1lID0gbm9uRGlzcGxheWluZ1NhbXBsZXMubGFzdCgpLT5wcmVzZW50YXRpb25U aW1lKCk7CisgICAgICAgIHRyYWNrQnVmZmVyLmxhc3RFbnF1ZXVlZERlY29kZUVuZFRpbWUgPSBu b25EaXNwbGF5aW5nU2FtcGxlcy5sYXN0KCktPmRlY29kZVRpbWUoKTsKKyAgICB9IGVsc2Ugewor ICAgICAgICB0cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWRQcmVzZW50YXRpb25UaW1lID0gTWVkaWFU aW1lOjppbnZhbGlkVGltZSgpOworICAgICAgICB0cmFja0J1ZmZlci5sYXN0RW5xdWV1ZWREZWNv ZGVFbmRUaW1lID0gTWVkaWFUaW1lOjppbnZhbGlkVGltZSgpOworICAgIH0KIAogICAgIC8vIEZp bGwgdGhlIGRlY29kZSBxdWV1ZSB3aXRoIHRoZSByZW1haW5pbmcgc2FtcGxlcy4KICAgICB0cmFj a0J1ZmZlci5kZWNvZGVRdWV1ZS5jbGVhcigpOwo=