77923 2012-02-06 18:36:20 -0800 [chromium] Allow retaining texture across frames in composited video playback and correctly handle lost context 2012-02-07 17:28:26 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 jamesr jamesr cc-bugs fischman vangelis webkit.review.bot oldest_to_newest 550513 0 jamesr 2012-02-06 18:36:20 -0800 [chromium] Allow retaining texture across frames in composited video playback and correctly handle lost context 550515 1 125748 jamesr 2012-02-06 18:38:07 -0800 Created attachment 125748 Patch 550517 2 jamesr 2012-02-06 18:40:06 -0800 This improves the composited video playback a good bit. It depends on http://trac.webkit.org/changeset/106840, in case we want to merge this. Ami - we avoid regressing lost context case for other types of content by adding layout tests like LayoutTests/platform/chromium/compositing/lost-compositor-context*. I'd like to do the same for html5 video but am not sure how to make a reliable layout test involving video playback, do you have any hints or suggestions or pointers to similar tests that I could copy? I'd like to make sure we draw at least one frame, lost the compositor context, then draw at least one more frame from the video. 550518 3 jamesr 2012-02-06 18:41:19 -0800 Actually, depends on http://trac.webkit.org/changeset/106891 as well since I'm using ManagedTexture::setTextureManager() 550533 4 fischman 2012-02-06 19:00:04 -0800 (In reply to comment #2) > This improves the composited video playback a good bit. OOC, how do you observe this? > Ami - we avoid regressing lost context case for other types of content by adding layout tests like LayoutTests/platform/chromium/compositing/lost-compositor-context*. Oooh, nice. > I'd like to do the same for html5 video but am not sure how to make a reliable layout test involving video playback, do you have any hints or suggestions or pointers to similar tests that I could copy? I'd like to make sure we draw at least one frame, lost the compositor context, then draw at least one more frame from the video. http://trac.webkit.org/browser/trunk/LayoutTests/media/ is where most video-related layouttests live. I think you could have a reliable test by seeking a video that hasn't been play()'d yet to a known spot, losing the context, and seeking to another spot, asserting the expected currentTime when the latter 'seeked' event fires. 550534 5 fischman 2012-02-06 19:00:39 -0800 Also, the patch lgtm, fwiw. Thanks! 550631 6 125748 vangelis 2012-02-06 22:18:46 -0800 Comment on attachment 125748 Patch Looks great! 551386 7 125748 kbr 2012-02-07 14:18:25 -0800 Comment on attachment 125748 Patch rs=me based on Vangelis's review. 551391 8 jamesr 2012-02-07 14:20:45 -0800 Committed r106992: <http://trac.webkit.org/changeset/106992> 551595 9 jamesr 2012-02-07 17:28:26 -0800 (In reply to comment #4) > (In reply to comment #2) > > This improves the composited video playback a good bit. > > OOC, how do you observe this? I should clarify - it improves the behavior I see in tracing (there aren't any new textures created) but I haven't measured any framerate improvement or anything like that. > > > Ami - we avoid regressing lost context case for other types of content by adding layout tests like LayoutTests/platform/chromium/compositing/lost-compositor-context*. > > Oooh, nice. > > > I'd like to do the same for html5 video but am not sure how to make a reliable layout test involving video playback, do you have any hints or suggestions or pointers to similar tests that I could copy? I'd like to make sure we draw at least one frame, lost the compositor context, then draw at least one more frame from the video. > > http://trac.webkit.org/browser/trunk/LayoutTests/media/ is where most video-related layouttests live. > I think you could have a reliable test by seeking a video that hasn't been play()'d yet to a known spot, losing the context, and seeking to another spot, asserting the expected currentTime when the latter 'seeked' event fires. Added a layout test here: https://bugs.webkit.org/show_bug.cgi?id=78060 that I believe would have caught this regression 125748 2012-02-06 18:38:07 -0800 2012-02-07 14:18:25 -0800 Patch bug-77923-20120206183806.patch text/plain 3765 jamesr U3VidmVyc2lvbiBSZXZpc2lvbjogMTA2ODkxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNTdiM2M3OThiNTM2NzYw NDQ0NzNjYTAzNTcyYTM4YzZmZjIyOTJjMC4uY2E4MTFjNGU1MzUyOWFjYjRiNzA1NDU3ODM3Nzdh NWQ4YjE2YmIyMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDIzIEBACiAyMDEyLTAyLTA2ICBKYW1l cyBSb2JpbnNvbiAgPGphbWVzckBjaHJvbWl1bS5vcmc+CiAKKyAgICAgICAgW2Nocm9taXVtXSBB bGxvdyByZXRhaW5pbmcgdGV4dHVyZSBhY3Jvc3MgZnJhbWVzIGluIGNvbXBvc2l0ZWQgdmlkZW8g cGxheWJhY2sgYW5kIGNvcnJlY3RseSBoYW5kbGUgbG9zdCBjb250ZXh0CisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD03NzkyMworCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoYW5rcyB0byByMTA2ODQwLCB3ZSBj YW4gaW1wcm92ZSB0aGUgdmlkZW8gcGxheWJhY2sgbW9kZSBhIGJpdC4gSW5zdGVhZCBvZiBjcmVh dGluZyBhIG5ldyB0ZXh0dXJlIG9uIGV2ZXJ5CisgICAgICAgIGZyYW1lLCB0aGlzIGF0dGVtcHRz IHRvIHJldXNlIHRoZSB0ZXh0dXJlIGZyb20gdGhlIHByZXZpb3VzIGZyYW1lIHVubGVzcyB0aGUg Y29udGV4dCBpcyBsb3N0LiBBbHNvIGltcHJvdmVzCisgICAgICAgIGVycm9yIGNoZWNraW5nIGlu IGNhc2UgdGhlIFRleHR1cmVNYW5hZ2VyIGNhbm5vdCBzdWNjZXNzZnVsbHkgcmVzZXJ2ZSBtZW1v cnkgZm9yIHRoZSB0ZXh0dXJlLgorCisgICAgICAgIFRlc3RlZCBtYW51YWxseSBieSBraWxsaW5n IHRoZSBHUFUgcHJvY2VzcyB3aXRoIGFuIGh0bWw1IHZpZGVvIHBsYXlpbmcgYW5kIHZlcmlmeWlu ZyB0aGF0IHRoZSB2aWRlbyBwbGF5YmFjaworICAgICAgICBjb250aW51ZXMgYW5kIHRoYXQgd2Ug ZG9uJ3QgY3JlYXRlIGEgbmV3IHNldCBvZiB0ZXh0dXJlcyBmb3IgZWFjaCBwbGFuZSBvbiBlYWNo IGZyYW1lLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvY2hyb21pdW0vY2MvQ0NWaWRl b0xheWVySW1wbC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDQ1ZpZGVvTGF5ZXJJbXBsOjpkcmF3 KToKKyAgICAgICAgKFdlYkNvcmU6OkNDVmlkZW9MYXllckltcGw6OnJlc2VydmVUZXh0dXJlcyk6 CisKKzIwMTItMDItMDYgIEphbWVzIFJvYmluc29uICA8amFtZXNyQGNocm9taXVtLm9yZz4KKwog ICAgICAgICBbY2hyb21pdW1dIGNhbnZhcyBkZW1vIGlzIHNsb3cgZHVlIHRvIHVubmVjZXNzYXJ5 IHJlc291cmNlIGNsZWFudXBzCiAgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD03NzEzNQogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9n cmFwaGljcy9jaHJvbWl1bS9jYy9DQ1ZpZGVvTGF5ZXJJbXBsLmNwcCBiL1NvdXJjZS9XZWJDb3Jl L3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDVmlkZW9MYXllckltcGwuY3BwCmluZGV4 IGZmODFhY2RhODI3MDcxN2RkYTZhM2MwYmU4MmU2NmE0N2UwZDgzNmIuLmVlZjBjYTNlOTQyNWI0 NmVhM2FhNTFjODdhYTI0ODlmYjNlMzQzNjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3Bs YXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDVmlkZW9MYXllckltcGwuY3BwCisrKyBiL1Nv dXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2Nocm9taXVtL2NjL0NDVmlkZW9MYXllcklt cGwuY3BwCkBAIC0xNTAsMTcgKzE1MCw4IEBAIHZvaWQgQ0NWaWRlb0xheWVySW1wbDo6ZHJhdyhM YXllclJlbmRlcmVyQ2hyb21pdW0qIGxheWVyUmVuZGVyZXIpCiAgICAgICAgIENSQVNIKCk7IC8v IFNvbWVvbmUgdXBkYXRlZCBjb252ZXJ0VkZDRm9ybWF0VG9HQzNERm9ybWF0IGFib3ZlIGJ1dCB1 cGRhdGUgdGhpcyEKICAgICB9CiAKLSAgICBmb3IgKHVuc2lnbmVkIHBsYW5lID0gMDsgcGxhbmUg PCBmcmFtZS0+cGxhbmVzKCk7ICsrcGxhbmUpIHsKKyAgICBmb3IgKHVuc2lnbmVkIHBsYW5lID0g MDsgcGxhbmUgPCBmcmFtZS0+cGxhbmVzKCk7ICsrcGxhbmUpCiAgICAgICAgIG1fdGV4dHVyZXNb cGxhbmVdLm1fdGV4dHVyZS0+dW5yZXNlcnZlKCk7Ci0gICAgICAgIC8vIEZJWE1FOiBNYW5hZ2Vk VGV4dHVyZSdzIHN0b3JlIGEgcmF3IHBvaW50ZXIgdG8gdGhlaXIgVGV4dHVyZU1hbmFnZXIsCi0g ICAgICAgIC8vIGFuZCB0aGUgdGV4dHVyZXMgd2UgY3JlYXRlIHVzZSBsYXllclJlbmRlcmVyLT5y ZW5kZXJTdXJmYWNlVGV4dHVyZU1hbmFnZXIoKS4KLSAgICAgICAgLy8gU2luY2UgdGhlcmUgaXMg bm8gZ3VhcmFudGVlIGxheWVyUmVuZGVyZXIgd2lsbCBzdGlsbCBiZSBhbGl2ZSB0aGUKLSAgICAg ICAgLy8gbmV4dCB0aW1lIHdlIGFyZSBjYWxsZWQsIHdlIGNsZWFyIHRoZSB0ZXh0dXJlIHJlZmVy ZW5jZS4gSXQgd291bGQKLSAgICAgICAgLy8gYmUgbmljZSBpZiBpbnN0ZWFkIHdlIGNvdWxkIHJl bHkgb24gdGV4dHVyZXMgYmVpbmcgaW52YWxpZGF0ZWQgd2hlbgotICAgICAgICAvLyB0aGVpciBt YW5hZ2VyIHdhcyBkZWxldGVkIHNvIHRoYXQgbmV3IHRleHR1cmVzIGRpZG4ndCBhbHdheXMgaGF2 ZSB0bwotICAgICAgICAvLyBiZSByZWNyZWF0ZWQgZm9yIGVhY2ggZnJhbWUuCi0gICAgICAgIG1f dGV4dHVyZXNbcGxhbmVdLm1fdGV4dHVyZS5jbGVhcigpOwotICAgIH0KICAgICBtX3Byb3ZpZGVy LT5wdXRDdXJyZW50RnJhbWUoZnJhbWUpOwogfQogCkBAIC0yNTUsMTAgKzI0NiwxNSBAQCBib29s IENDVmlkZW9MYXllckltcGw6OnJlc2VydmVUZXh0dXJlcyhjb25zdCBWaWRlb0ZyYW1lQ2hyb21p dW0qIGZyYW1lLCBHQzNEZW51bQogICAgICAgICAgICAgaWYgKCFtX3RleHR1cmVzW3BsYW5lXS5t X3RleHR1cmUpCiAgICAgICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICAgICAgbV90 ZXh0dXJlc1twbGFuZV0ubV92aXNpYmxlU2l6ZSA9IEludFNpemUoKTsKKyAgICAgICAgfSBlbHNl IHsKKyAgICAgICAgICAgIC8vIFRoZSByZW5kZXJTdXJmYWNlVGV4dHVyZU1hbmFnZXIgbWF5IGhh dmUgYmVlbiBkZXN0cm95ZWQgYW5kIHJlY3JlYXRlZCBzaW5jZSB0aGUgbGFzdCBmcmFtZSwgc28g cGFzcyB0aGUgbmV3IG9uZS4KKyAgICAgICAgICAgIC8vIFRoaXMgaXMgYSBuby1vcCBpZiB0aGUg VGV4dHVyZU1hbmFnZXIgaXMgc3RpbGwgYXJvdW5kLgorICAgICAgICAgICAgbV90ZXh0dXJlc1tw bGFuZV0ubV90ZXh0dXJlLT5zZXRUZXh0dXJlTWFuYWdlcihsYXllclJlbmRlcmVyLT5yZW5kZXJT dXJmYWNlVGV4dHVyZU1hbmFnZXIoKSk7CiAgICAgICAgIH0KICAgICAgICAgaWYgKG1fdGV4dHVy ZXNbcGxhbmVdLm1fdGV4dHVyZS0+c2l6ZSgpICE9IHJlcXVpcmVkVGV4dHVyZVNpemUpCiAgICAg ICAgICAgICBtX3RleHR1cmVzW3BsYW5lXS5tX3Zpc2libGVTaXplID0gY29tcHV0ZVZpc2libGVT aXplKGZyYW1lLCBwbGFuZSk7Ci0gICAgICAgIG1fdGV4dHVyZXNbcGxhbmVdLm1fdGV4dHVyZS0+ cmVzZXJ2ZShyZXF1aXJlZFRleHR1cmVTaXplLCBmb3JtYXQpOworICAgICAgICBpZiAoIW1fdGV4 dHVyZXNbcGxhbmVdLm1fdGV4dHVyZS0+cmVzZXJ2ZShyZXF1aXJlZFRleHR1cmVTaXplLCBmb3Jt YXQpKQorICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgIH0KICAgICByZXR1cm4gdHJ1ZTsK IH0K