197355 2019-04-28 13:07:12 -0700 [GStreamer][MSE] Pausing video sometimes causes skip to finish 2019-06-24 04:34:32 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build PC Linux RESOLVED FIXED InRadar P2 Normal --- 1 mcatanzaro cturner aboya bugs-noreply commit-queue cturner ews-watchlist mcatanzaro pnormand webkit-bug-importer oldest_to_newest 1531075 0 mcatanzaro 2019-04-28 13:07:12 -0700 Since we enabled MSE, pausing a YouTube video occasionally (but not usually and not often) causes the video to improperly skip to the very end. 1539659 1 pnormand 2019-05-28 11:10:12 -0700 *** Bug 198054 has been marked as a duplicate of this bug. *** 1546548 2 372562 cturner 2019-06-20 09:50:43 -0700 Created attachment 372562 Patch WIP/demonstation patch 1546549 3 cturner 2019-06-20 09:51:58 -0700 This is easily reproducible on trunk. The way to trigger it is to seek very close to the end, let WebCore::MediaSource::endOfStream run, which will in turn call WebCore::MediaSource::streamEndedWithError, which in the GStreamerMSE player will set m_eosMarked = true, which in turn will set m_eosPending in updateStates(), which then breaks MediaPlayerPrivateGStreamerMSE::currentMediaTime, since after these steps it thinks the network resource is fully loaded and that the end has been reached (actually it just buffered to the end), and this in turn sets the cached playback position to the end and we get a jump. Looking at this code I see all sorts of strange issues. We have a downstream port that doesn't have such blatant problems on YouTube, and I see a large diff in these areas which we should work on getting upstream. The other complication here is that there's large MSE refactoring Alicia is doing in parallel that probably touches this, so the MSE folks will have a much better idea what is the best way to fix it. 1546550 4 ews-watchlist 2019-06-20 09:52:52 -0700 Attachment 372562 did not pass style-queue: ERROR: Source/WebCore/ChangeLog:10: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1546551 5 cturner 2019-06-20 09:54:06 -0700 To clarify, the WIP/demonstration patch is not for review, I put here to pinpoint where the bug is, this patch fixes the jumping issue for me, but I have no idea what else it may break, and the right thing to do is surely to focus on getting our downstream code up here and/or waiting for the refactoring. 1546582 6 mcatanzaro 2019-06-20 11:13:56 -0700 Thanks Charlie! Alicia, what do you think? 1546649 7 aboya 2019-06-20 14:00:53 -0700 I have no idea why `paused()` was there in that condition, so on first sight the patch LGTM. In the current state of my rework markEndOfStream() is empty, as it has no business with neither the AppendPipeline nor (directly) the playback pipeline. Instead I rely on allSamplesInTrackEnqueued(), which SourceBuffer calls when the source is "ended" (.endOfStream() has been called) AND the decode queue is empty. When this method is fired an EOS event is enqueued into WebKitMediaSrc. MediaPlayerPrivateGStreamerMSE::currentMediaTime() does not even exist anymore in my rework, so your patch won't cause a lot of trouble other than a rebase conflict. If it does not cause regressions on LayoutTests or YT tests I'm OK with it landing. 1546862 8 372627 cturner 2019-06-21 06:44:35 -0700 Created attachment 372627 Patch Try this patch in the bots commit queues. Locally I see a huge number of failures, but it may be my local environment 1546870 9 mcatanzaro 2019-06-21 07:21:44 -0700 None of the test EWS are going to run this code. If you want to see what happens to the bots, you just have to land it. Safe to say that if it introduces a huge number of failures in your local environment that were not there before, there is a problem. 1546874 10 cturner 2019-06-21 07:58:33 -0700 (In reply to Michael Catanzaro from comment #9) > None of the test EWS are going to run this code. If you want to see what > happens to the bots, you just have to land it. > > Safe to say that if it introduces a huge number of failures in your local > environment that were not there before, there is a problem. Obviously I would be aware of that, the issue is that I have so many failures for other reasons with my environment that I can't see the signal for the noise at the moment. 1546876 11 mcatanzaro 2019-06-21 08:24:25 -0700 OK, just making sure you understand: neither the EWS nor the commit queue test GTK/WPE. 1546958 12 cturner 2019-06-21 13:15:24 -0700 I see. This patch doesn't regress any tests in media/ or imported/w3c/web-platform-tests/media-source for me. Let's land it on Monday when I can monitor the bots closely. 1547287 13 372627 commit-queue 2019-06-24 04:33:37 -0700 Comment on attachment 372627 Patch Clearing flags on attachment: 372627 Committed r246731: <https://trac.webkit.org/changeset/246731> 1547288 14 commit-queue 2019-06-24 04:33:39 -0700 All reviewed patches have been landed. Closing bug. 1547289 15 webkit-bug-importer 2019-06-24 04:34:32 -0700 <rdar://problem/52049850> 372562 2019-06-20 09:50:43 -0700 2019-06-21 06:44:31 -0700 Patch bug-197355-20190620175042.patch text/plain 1965 cturner U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ2NTM2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjgxY2NiNjIxNWY4NTJi ZmIyNjY5Yjg2NjdlMTkwYTI5ODk1MTEzYy4uMjcxZTljNTQwMmI2NDE2MTY2NjQ0YmI2MWY2YjUy OTY1NjViMmU0MiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIwIEBACisyMDE5LTA2LTIwICBDaGFy bGllIFR1cm5lciAgPGN0dXJuZXJAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1N0cmVhbWVyXVtN U0VdIFBhdXNpbmcgdmlkZW8gc29tZXRpbWVzIGNhdXNlcyBza2lwIHRvIGZpbmlzaAorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk3MzU1CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgV29yayBpbiBwcm9ncmVz cz8KKworICAgICAgICBObyBuZXcgdGVzdHMgKE9PUFMhKS4KKworICAgICAgICAqIHBsYXRmb3Jt L2dyYXBoaWNzL2dzdHJlYW1lci9tc2UvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyTVNFLmNw cDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lck1TRTo6Y3Vy cmVudE1lZGlhVGltZSBjb25zdCk6CisgICAgICAgIEFzc3VtaW5nIHRoYXQgd2hlbiBtX2Vvc1Bl bmRpbmcgaXMgb24gYW5kIHdlJ3JlIHBhdXNlZCgpIHRoYXQgdGhlCisgICAgICAgIG5ldHdvcmsg cmVzb3VyY2UgaXMgZnVsbHkgbG9hZGVkIGFuZCB0aGUgZW5kIGlzIHJlYWNoZWQgaXMgY2xlYXJs eQorICAgICAgICB3cm9uZy4gV2hldGhlciB0aGlzIGlzIG5vdyBjb3JyZWN0IGlzIHVuY2xlYXIu Li4KKwogMjAxOS0wNi0xOCAgWmFuIERvYmVyc2VrICA8emRvYmVyc2VrQGlnYWxpYS5jb20+CiAK ICAgICAgICAgW1dlYkdMXSBFeHRlbnNpb25zM0RPcGVuR0xFUzo6YmluZFZlcnRleEFycmF5T0VT KCkgc2hvdWxkIGFsbG93IHplcm8gYXJyYXkgb2JqZWN0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvbXNlL01lZGlhUGxheWVyUHJpdmF0ZUdT dHJlYW1lck1TRS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFt ZXIvbXNlL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lck1TRS5jcHAKaW5kZXggYTg4ZmM4YjI1 MmE2MjBiMzYxZjU2OWJlM2E4ZWYzMTNlYjMyYmE3NC4uNGU5ZDE5ZDg1MjdkNjk2MjBjZjVhYTk0 MTA3MzYyODM4MWIzYzlmMCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3Jh cGhpY3MvZ3N0cmVhbWVyL21zZS9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXJNU0UuY3BwCisr KyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9tc2UvTWVkaWFQ bGF5ZXJQcml2YXRlR1N0cmVhbWVyTVNFLmNwcApAQCAtNzU4LDcgKzc1OCw3IEBAIE1lZGlhVGlt ZSBNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXJNU0U6OmN1cnJlbnRNZWRpYVRpbWUoKSBjb25z dAogewogICAgIE1lZGlhVGltZSBwb3NpdGlvbiA9IE1lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1l cjo6Y3VycmVudE1lZGlhVGltZSgpOwogCi0gICAgaWYgKG1fZW9zUGVuZGluZyAmJiAocGF1c2Vk KCkgfHwgKHBvc2l0aW9uID49IGR1cmF0aW9uTWVkaWFUaW1lKCkpKSkgeworICAgIGlmIChtX2Vv c1BlbmRpbmcgJiYgcG9zaXRpb24gPj0gZHVyYXRpb25NZWRpYVRpbWUoKSkgewogICAgICAgICBp ZiAobV9uZXR3b3JrU3RhdGUgIT0gTWVkaWFQbGF5ZXI6OkxvYWRlZCkgewogICAgICAgICAgICAg bV9uZXR3b3JrU3RhdGUgPSBNZWRpYVBsYXllcjo6TG9hZGVkOwogICAgICAgICAgICAgbV9wbGF5 ZXItPm5ldHdvcmtTdGF0ZUNoYW5nZWQoKTsK 372627 2019-06-21 06:44:35 -0700 2019-06-24 04:33:37 -0700 Patch bug-197355-20190621144434.patch text/plain 1941 cturner U3VidmVyc2lvbiBSZXZpc2lvbjogMjQ2NTM2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjgxY2NiNjIxNWY4NTJi ZmIyNjY5Yjg2NjdlMTkwYTI5ODk1MTEzYy4uYTIxMDBlZDc4ZThlYjdmMGFhYjc4NzI1M2UzMTRl OTE5ZTZkYzYzMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE5LTA2LTIwICBDaGFy bGllIFR1cm5lciAgPGN0dXJuZXJAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1N0cmVhbWVyXVtN U0VdIFBhdXNpbmcgdmlkZW8gc29tZXRpbWVzIGNhdXNlcyBza2lwIHRvIGZpbmlzaAorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk3MzU1CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQ292ZXJlZCBieSBleGlz dGluZyB0ZXN0cy4KKworICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9tc2Uv TWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyTVNFLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1l ZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lck1TRTo6Y3VycmVudE1lZGlhVGltZSBjb25zdCk6Cisg ICAgICAgIEFzc3VtaW5nIHRoYXQgd2hlbiBtX2Vvc1BlbmRpbmcgaXMgb24gYW5kIHdlJ3JlIHBh dXNlZCgpIHRoYXQgdGhlCisgICAgICAgIG5ldHdvcmsgcmVzb3VyY2UgaXMgZnVsbHkgbG9hZGVk IGFuZCB0aGUgZW5kIGlzIHJlYWNoZWQgaXMgY2xlYXJseQorICAgICAgICB3cm9uZy4gV2hldGhl ciB0aGlzIGlzIG5vdyBjb3JyZWN0IGlzIHVuY2xlYXIuLi4KKwogMjAxOS0wNi0xOCAgWmFuIERv YmVyc2VrICA8emRvYmVyc2VrQGlnYWxpYS5jb20+CiAKICAgICAgICAgW1dlYkdMXSBFeHRlbnNp b25zM0RPcGVuR0xFUzo6YmluZFZlcnRleEFycmF5T0VTKCkgc2hvdWxkIGFsbG93IHplcm8gYXJy YXkgb2JqZWN0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9n c3RyZWFtZXIvbXNlL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lck1TRS5jcHAgYi9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvbXNlL01lZGlhUGxheWVyUHJpdmF0 ZUdTdHJlYW1lck1TRS5jcHAKaW5kZXggYTg4ZmM4YjI1MmE2MjBiMzYxZjU2OWJlM2E4ZWYzMTNl YjMyYmE3NC4uNGU5ZDE5ZDg1MjdkNjk2MjBjZjVhYTk0MTA3MzYyODM4MWIzYzlmMCAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL21zZS9NZWRp YVBsYXllclByaXZhdGVHU3RyZWFtZXJNU0UuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRm b3JtL2dyYXBoaWNzL2dzdHJlYW1lci9tc2UvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyTVNF LmNwcApAQCAtNzU4LDcgKzc1OCw3IEBAIE1lZGlhVGltZSBNZWRpYVBsYXllclByaXZhdGVHU3Ry ZWFtZXJNU0U6OmN1cnJlbnRNZWRpYVRpbWUoKSBjb25zdAogewogICAgIE1lZGlhVGltZSBwb3Np dGlvbiA9IE1lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lcjo6Y3VycmVudE1lZGlhVGltZSgpOwog Ci0gICAgaWYgKG1fZW9zUGVuZGluZyAmJiAocGF1c2VkKCkgfHwgKHBvc2l0aW9uID49IGR1cmF0 aW9uTWVkaWFUaW1lKCkpKSkgeworICAgIGlmIChtX2Vvc1BlbmRpbmcgJiYgcG9zaXRpb24gPj0g ZHVyYXRpb25NZWRpYVRpbWUoKSkgewogICAgICAgICBpZiAobV9uZXR3b3JrU3RhdGUgIT0gTWVk aWFQbGF5ZXI6OkxvYWRlZCkgewogICAgICAgICAgICAgbV9uZXR3b3JrU3RhdGUgPSBNZWRpYVBs YXllcjo6TG9hZGVkOwogICAgICAgICAgICAgbV9wbGF5ZXItPm5ldHdvcmtTdGF0ZUNoYW5nZWQo KTsK