213888 2020-07-02 09:19:37 -0700 [MSE][GStreamer] Don't skip samples past media duration in AppendPipeline 2020-07-06 00:42:37 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 aboya aboya bugs-noreply calvaris cgarcia eocanha ews-watchlist gustavo menard pnormand vjaquez oldest_to_newest 1668266 0 aboya 2020-07-02 09:19:37 -0700 appsinkNewSample() contained code to skip samples whose presentation time starts after media duration. This is paradoxical, because later sourceBufferPrivateDidReceiveSample() extends media duration whenever the presentation end time of the sample is past the original one. It does not sound reasonable that samples extending the duration are okay on one case but outright rejected in the other. Also, if it was about skipping samples, sourceBufferPrivateDidReceiveSample() could do it itself, and already does in other cases. For all these reasons I was very doubtful of the need for this if() and indeed removing this condition didn't cause any new test failure. 1668267 1 403372 aboya 2020-07-02 09:20:30 -0700 Created attachment 403372 Patch 1668270 2 eocanha 2020-07-02 09:32:34 -0700 After discussing this removal with Alicia, checking where the original code came from before being upstreamed[1] (I can't recall any rationale for discarding samples beyond the duration), and Alicia checking that it doesn't break any YouTube TV test[2] nor LayoutTest, I agree with removing the code block. [1] https://github.com/eocanha/webkit/commit/86764b20df56#diff-1a0340714c802e905fa36d804906e36eR3270 [2] https://ytlr-cert.appspot.com/2019/main.html 1668986 3 ews-feeder 2020-07-06 00:42:35 -0700 Committed r263962: <https://trac.webkit.org/changeset/263962> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403372. 403372 2020-07-02 09:20:30 -0700 2020-07-06 00:42:36 -0700 Patch bug-213888-20200702182028.patch text/plain 2865 aboya U3VidmVyc2lvbiBSZXZpc2lvbjogMjYzODQ0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMmZlYzRmZmVjNjE3N2Vj ODkyZGIxNDc3OWVhZTg2MDE0YWVhOWJlMi4uYmJmZDM3Mjg4NmUzN2NiMjIzMDIyYzJhODdiOWVi Mjk5ODIzNjQ5YyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2IEBACisyMDIwLTA3LTAyICBBbGlj aWEgQm95YSBHYXJjw61hICA8YWJveWFAaWdhbGlhLmNvbT4KKworICAgICAgICBbTVNFXVtHU3Ry ZWFtZXJdIERvbid0IHNraXAgc2FtcGxlcyBwYXN0IG1lZGlhIGR1cmF0aW9uIGluIEFwcGVuZFBp cGVsaW5lCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0y MTM4ODgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBh cHBzaW5rTmV3U2FtcGxlKCkgY29udGFpbmVkIGNvZGUgdG8gc2tpcCBzYW1wbGVzIHdob3NlIHBy ZXNlbnRhdGlvbgorICAgICAgICB0aW1lIHN0YXJ0cyBhZnRlciBtZWRpYSBkdXJhdGlvbi4gVGhp cyBpcyBwYXJhZG94aWNhbCwgYmVjYXVzZSBsYXRlcgorICAgICAgICBzb3VyY2VCdWZmZXJQcml2 YXRlRGlkUmVjZWl2ZVNhbXBsZSgpIGV4dGVuZHMgbWVkaWEgZHVyYXRpb24gd2hlbmV2ZXIKKyAg ICAgICAgdGhlIHByZXNlbnRhdGlvbiBlbmQgdGltZSBvZiB0aGUgc2FtcGxlIGlzIHBhc3QgdGhl IG9yaWdpbmFsIG9uZS4KKworICAgICAgICBJdCBkb2VzIG5vdCBzb3VuZCByZWFzb25hYmxlIHRo YXQgc2FtcGxlcyBleHRlbmRpbmcgdGhlIGR1cmF0aW9uIGFyZQorICAgICAgICBva2F5IG9uIG9u ZSBjYXNlIGJ1dCBvdXRyaWdodCByZWplY3RlZCBpbiB0aGUgb3RoZXIuIEFsc28sIGlmIGl0IHdh cworICAgICAgICBhYm91dCBza2lwcGluZyBzYW1wbGVzLCBzb3VyY2VCdWZmZXJQcml2YXRlRGlk UmVjZWl2ZVNhbXBsZSgpIGNvdWxkIGRvCisgICAgICAgIGl0IGl0c2VsZiwgYW5kIGFscmVhZHkg ZG9lcyBpbiBvdGhlciBjYXNlcy4KKworICAgICAgICBGb3IgYWxsIHRoZXNlIHJlYXNvbnMgSSB3 YXMgdmVyeSBkb3VidGZ1bCBvZiB0aGUgbmVlZCBmb3IgdGhpcyBpZigpCisgICAgICAgIGFuZCBp bmRlZWQgcmVtb3ZpbmcgdGhpcyBjb25kaXRpb24gZGlkbid0IGNhdXNlIGFueSBuZXcgdGVzdCBm YWlsdXJlLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL21zZS9BcHBl bmRQaXBlbGluZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpBcHBlbmRQaXBlbGluZTo6YXBwc2lu a05ld1NhbXBsZSk6CisKIDIwMjAtMDctMDIgIFlvdWVubiBGYWJsZXQgIDx5b3Vlbm5AYXBwbGUu Y29tPgogCiAgICAgICAgIGdldFVzZXJNZWRpYSByZXR1cm5zIE92ZXJDb25zdHJhaW5lZCBvbiBK aXRzaQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVh bWVyL21zZS9BcHBlbmRQaXBlbGluZS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9nc3RyZWFtZXIvbXNlL0FwcGVuZFBpcGVsaW5lLmNwcAppbmRleCBlMjAyMjdhMWI1Zjcx Zjk0M2I1ZWYwNmM4OGVmNjc4MGE4ZDdkNzQ1Li5kYzAwOTQ5YzcxZjA1ODZhM2UwODdjOTkxYTdj ODE0YTdiOGYzNmNhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGlj cy9nc3RyZWFtZXIvbXNlL0FwcGVuZFBpcGVsaW5lLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9w bGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvbXNlL0FwcGVuZFBpcGVsaW5lLmNwcApAQCAtNDcw LDEzICs0NzAsNiBAQCB2b2lkIEFwcGVuZFBpcGVsaW5lOjphcHBzaW5rTmV3U2FtcGxlKEdSZWZQ dHI8R3N0U2FtcGxlPiYmIHNhbXBsZSkKICAgICAgICAgbWVkaWFTYW1wbGUtPmR1cmF0aW9uKCku dG9TdHJpbmcoKS51dGY4KCkuZGF0YSgpLAogICAgICAgICBtZWRpYVNhbXBsZS0+cHJlc2VudGF0 aW9uU2l6ZSgpLndpZHRoKCksIG1lZGlhU2FtcGxlLT5wcmVzZW50YXRpb25TaXplKCkuaGVpZ2h0 KCkpOwogCi0gICAgLy8gSWYgd2UncmUgYmV5b25kIHRoZSBkdXJhdGlvbiwgaWdub3JlIHRoaXMg c2FtcGxlLgotICAgIE1lZGlhVGltZSBkdXJhdGlvbiA9IG1fbWVkaWFTb3VyY2VDbGllbnQtPmR1 cmF0aW9uKCk7Ci0gICAgaWYgKGR1cmF0aW9uLmlzVmFsaWQoKSAmJiAhZHVyYXRpb24uaW5kZWZp bml0ZVRpbWUoKSAmJiBtZWRpYVNhbXBsZS0+cHJlc2VudGF0aW9uVGltZSgpID4gZHVyYXRpb24p IHsKLSAgICAgICAgR1NUX0RFQlVHX09CSkVDVChtX3BpcGVsaW5lLmdldCgpLCAiRGV0ZWN0ZWQg c2FtcGxlICglcykgYmV5b25kIHRoZSBkdXJhdGlvbiAoJXMpLCBkaXNjYXJkaW5nIiwgbWVkaWFT YW1wbGUtPnByZXNlbnRhdGlvblRpbWUoKS50b1N0cmluZygpLnV0ZjgoKS5kYXRhKCksIGR1cmF0 aW9uLnRvU3RyaW5nKCkudXRmOCgpLmRhdGEoKSk7Ci0gICAgICAgIHJldHVybjsKLSAgICB9Ci0K ICAgICAvLyBIYWNrLCByZXdvcmsgd2hlbiBHU3RyZWFtZXIgPj0gMS4xNiBiZWNvbWVzIGEgcmVx dWlyZW1lbnQ6CiAgICAgLy8gV2UncmUgbm90IGFwcGx5aW5nIGVkaXQgbGlzdHMuIEdTdHJlYW1l ciA8IDEuMTYgZG9lc24ndCBlbWl0IHRoZSBjb3JyZWN0IHNlZ21lbnRzIHRvIGRvIHNvLgogICAg IC8vIEdTdHJlYW1lciBmaXggaW4gaHR0cHM6Ly9naXRsYWIuZnJlZWRlc2t0b3Aub3JnL2dzdHJl YW1lci9nc3QtcGx1Z2lucy1nb29kLy0vY29tbWl0L2MyYTBkYTgwOTYwMDlmMGY5OTk0M2Y3OGRj MTgwNjY5NjViZTYwZjkK