81856 2012-03-21 18:07:42 -0700 HTMLMediaElement::updateActiveTextTrackCues can do unnecessary work 2013-05-01 11:54:41 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 43668 1 eric.carlson eric.carlson commit-queue esprehn+autocc feature-media-reviews glenn jer.noble vcarbune webkit-bug-importer webkit.review.bot oldest_to_newest 584846 0 eric.carlson 2012-03-21 18:07:42 -0700 HTMLMediaElement::updateActiveTextTrackCues is called several times a second to see if text track cues need to be rendered. Unfortunately, it does a fair amount of work even when there are no text tracks at all. 704977 1 160640 edaena 2012-08-26 22:51:16 -0700 Created attachment 160640 Proposed fix. 705013 2 160640 vcarbune 2012-08-27 00:17:10 -0700 Comment on attachment 160640 Proposed fix. View in context: https://bugs.webkit.org/attachment.cgi?id=160640&action=review It's a good check, but I don't think it's enough to close this bug - it's about unnecessary work when there actually are tracks associated with the video. > Source/WebCore/html/HTMLMediaElement.cpp:715 > + if (RuntimeEnabledFeatures::webkitVideoTrackEnabled() && m_textTracks) I think checking for m_textTracks should be in updateActiveTextTrackCues. Even better, HTMLMediaElement::ignoreTrackDisplayUpdateRequests() could become { return !m_textTracks || m_ignoreTrackDisplayUpdate > 0; } 705226 3 eric.carlson 2012-08-27 06:43:42 -0700 (In reply to comment #2) > (From update of attachment 160640 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=160640&action=review > > It's a good check, but I don't think it's enough to close this bug - it's about unnecessary work when there actually are tracks associated with the video. > > > Source/WebCore/html/HTMLMediaElement.cpp:715 > > + if (RuntimeEnabledFeatures::webkitVideoTrackEnabled() && m_textTracks) > > I think checking for m_textTracks should be in updateActiveTextTrackCues. Even better, HTMLMediaElement::ignoreTrackDisplayUpdateRequests() could become { return !m_textTracks || m_ignoreTrackDisplayUpdate > 0; } I agree with Victor. Additionally, the "LOG(...)" in HTMLMediaElement::updateActiveTextTrackCues should be after the ignoreTrackDisplayUpdateRequests check. 883472 4 webkit-bug-importer 2013-05-01 11:24:41 -0700 <rdar://problem/13783424> 883473 5 200229 eric.carlson 2013-05-01 11:25:53 -0700 Created attachment 200229 Proposed patch 883482 6 200229 commit-queue 2013-05-01 11:54:39 -0700 Comment on attachment 200229 Proposed patch Clearing flags on attachment: 200229 Committed r149443: <http://trac.webkit.org/changeset/149443> 883483 7 commit-queue 2013-05-01 11:54:41 -0700 All reviewed patches have been landed. Closing bug. 160640 2012-08-26 22:51:16 -0700 2013-05-01 11:25:53 -0700 Proposed fix. HTMLMediaElement_Patch.txt text/plain 3090 edaena SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEyNjcyMykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIxIEBACisyMDEyLTA4LTI2ICBFZGFlbmEg U2FsaW5hcyAgPGVkYWVuYUBnbWFpbC5jb20+CisKKyAgICAgICAgSFRNTE1lZGlhRWxlbWVudDo6 dXBkYXRlQWN0aXZlVGV4dFRyYWNrQ3VlcyBjYW4gZG8gdW5uZWNlc3Nhcnkgd29yay4KKyAgICAg ICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTgxODU2CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgQmVmb3JlIGNhbGxpbmcg dXBkYXRlQWN0aXZlVGV4dFRyYWNrQ3VlcyBpdCBjaGVja3MgaWYgYW4gaHRtbCB2aWRlbyBlbGVt ZW50IGhhcyB0cmFjayBlbGVtZW50cy4KKworICAgICAgICBObyBuZXcgdGVzdHMgKE9PUFMhKS4K KworICAgICAgICAqIGh0bWwvSFRNTE1lZGlhRWxlbWVudC5jcHA6CisgICAgICAgIChXZWJDb3Jl OjpIVE1MTWVkaWFFbGVtZW50OjpwcmVwYXJlRm9yTG9hZCk6IENoZWNrcyBpZiB0aGUgdmlkZW8g ZWxlbWVudCBoYXMgdHJhY2sgZWxlbWVudHMuCisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFF bGVtZW50OjpzZXRSZWFkeVN0YXRlKTogIENoZWNrcyBpZiB0aGUgdmlkZW8gaGFzIHRyYWNrIGVs ZW1lbnRzLgorICAgICAgICAoV2ViQ29yZTo6SFRNTE1lZGlhRWxlbWVudDo6cGxheWJhY2tQcm9n cmVzc1RpbWVyRmlyZWQpOiBDaGVja3MgaWYgdGhlIHZpZGVvIGhhcyB0cmFjayBlbGVtZW50cy4K KyAgICAgICAgKFdlYkNvcmU6OkhUTUxNZWRpYUVsZW1lbnQ6Om1lZGlhUGxheWVyVGltZUNoYW5n ZWQpOiBDaGVja3MgaWYgdGhlIHZpZGVvIGhhcyB0cmFjayBlbGVtZW50cy4KKyAgICAgICAgKFdl YkNvcmU6OkhUTUxNZWRpYUVsZW1lbnQ6OnVzZXJDYW5jZWxsZWRMb2FkKTogQ2hlY2tzIGlmIHRo ZSB2aWRlbyBoYXMgdHJhY2sgZWxlbWVudHMuCisKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2h0bWwv SFRNTE1lZGlhRWxlbWVudC5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvaHRtbC9I VE1MTWVkaWFFbGVtZW50LmNwcAkocmV2aXNpb24gMTI2Njk0KQorKysgU291cmNlL1dlYkNvcmUv aHRtbC9IVE1MTWVkaWFFbGVtZW50LmNwcAkod29ya2luZyBjb3B5KQpAQCAtNzEyLDcgKzcxMiw3 IEBAIHZvaWQgSFRNTE1lZGlhRWxlbWVudDo6cHJlcGFyZUZvckxvYWQoKQogICAgICAgICBzY2hl ZHVsZUV2ZW50KGV2ZW50TmFtZXMoKS5lbXB0aWVkRXZlbnQpOwogICAgICAgICB1cGRhdGVNZWRp YUNvbnRyb2xsZXIoKTsKICNpZiBFTkFCTEUoVklERU9fVFJBQ0spCi0gICAgICAgIGlmIChSdW50 aW1lRW5hYmxlZEZlYXR1cmVzOjp3ZWJraXRWaWRlb1RyYWNrRW5hYmxlZCgpKQorICAgICAgICBp ZiAoUnVudGltZUVuYWJsZWRGZWF0dXJlczo6d2Via2l0VmlkZW9UcmFja0VuYWJsZWQoKSAmJiBt X3RleHRUcmFja3MpCiAgICAgICAgICAgICB1cGRhdGVBY3RpdmVUZXh0VHJhY2tDdWVzKDApOwog I2VuZGlmCiAgICAgfQpAQCAtMTc0MSw3ICsxNzQxLDcgQEAgdm9pZCBIVE1MTWVkaWFFbGVtZW50 OjpzZXRSZWFkeVN0YXRlKE1lZAogICAgIHVwZGF0ZVBsYXlTdGF0ZSgpOwogICAgIHVwZGF0ZU1l ZGlhQ29udHJvbGxlcigpOwogI2lmIEVOQUJMRShWSURFT19UUkFDSykKLSAgICBpZiAoUnVudGlt ZUVuYWJsZWRGZWF0dXJlczo6d2Via2l0VmlkZW9UcmFja0VuYWJsZWQoKSkKKyAgICBpZiAoUnVu dGltZUVuYWJsZWRGZWF0dXJlczo6d2Via2l0VmlkZW9UcmFja0VuYWJsZWQoKSAmJiBtX3RleHRU cmFja3MpCiAgICAgICAgIHVwZGF0ZUFjdGl2ZVRleHRUcmFja0N1ZXMoY3VycmVudFRpbWUoKSk7 CiAjZW5kaWYKIH0KQEAgLTI2MDYsNyArMjYwNiw3IEBAIHZvaWQgSFRNTE1lZGlhRWxlbWVudDo6 cGxheWJhY2tQcm9ncmVzc1QKICAgICAgICAgbWVkaWFDb250cm9scygpLT5wbGF5YmFja1Byb2dy ZXNzZWQoKTsKICAgICAKICNpZiBFTkFCTEUoVklERU9fVFJBQ0spCi0gICAgaWYgKFJ1bnRpbWVF bmFibGVkRmVhdHVyZXM6OndlYmtpdFZpZGVvVHJhY2tFbmFibGVkKCkpCisgICAgaWYgKFJ1bnRp bWVFbmFibGVkRmVhdHVyZXM6OndlYmtpdFZpZGVvVHJhY2tFbmFibGVkKCkgJiYgbV90ZXh0VHJh Y2tzKQogICAgICAgICB1cGRhdGVBY3RpdmVUZXh0VHJhY2tDdWVzKGN1cnJlbnRUaW1lKCkpOwog I2VuZGlmCiB9CkBAIC0zMTY0LDcgKzMxNjQsNyBAQCB2b2lkIEhUTUxNZWRpYUVsZW1lbnQ6Om1l ZGlhUGxheWVyVGltZUNoCiAgICAgTE9HKE1lZGlhLCAiSFRNTE1lZGlhRWxlbWVudDo6bWVkaWFQ bGF5ZXJUaW1lQ2hhbmdlZCIpOwogCiAjaWYgRU5BQkxFKFZJREVPX1RSQUNLKQotICAgIGlmIChS dW50aW1lRW5hYmxlZEZlYXR1cmVzOjp3ZWJraXRWaWRlb1RyYWNrRW5hYmxlZCgpKQorICAgIGlm IChSdW50aW1lRW5hYmxlZEZlYXR1cmVzOjp3ZWJraXRWaWRlb1RyYWNrRW5hYmxlZCgpICYmIG1f dGV4dFRyYWNrcykKICAgICAgICAgdXBkYXRlQWN0aXZlVGV4dFRyYWNrQ3VlcyhjdXJyZW50VGlt ZSgpKTsKICNlbmRpZgogCkBAIC0zNjM5LDcgKzM2MzksNyBAQCB2b2lkIEhUTUxNZWRpYUVsZW1l bnQ6OnVzZXJDYW5jZWxsZWRMb2FkCiAgICAgbV9yZWFkeVN0YXRlID0gSEFWRV9OT1RISU5HOwog ICAgIHVwZGF0ZU1lZGlhQ29udHJvbGxlcigpOwogI2lmIEVOQUJMRShWSURFT19UUkFDSykKLSAg ICBpZiAoUnVudGltZUVuYWJsZWRGZWF0dXJlczo6d2Via2l0VmlkZW9UcmFja0VuYWJsZWQoKSkK KyAgICBpZiAoUnVudGltZUVuYWJsZWRGZWF0dXJlczo6d2Via2l0VmlkZW9UcmFja0VuYWJsZWQo KSAmJiBtX3RleHRUcmFja3MpCiAgICAgICAgIHVwZGF0ZUFjdGl2ZVRleHRUcmFja0N1ZXMoMCk7 CiAjZW5kaWYKIH0K 200229 2013-05-01 11:25:53 -0700 2013-05-01 11:54:39 -0700 Proposed patch patch_2.txt text/plain 1561 eric.carlson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE0OTQzNSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBACisyMDEzLTA1LTAxICBFcmljIENh cmxzb24gIDxlcmljLmNhcmxzb25AYXBwbGUuY29tPgorCisgICAgICAgIEhUTUxNZWRpYUVsZW1l bnQ6OnVwZGF0ZUFjdGl2ZVRleHRUcmFja0N1ZXMgY2FuIGRvIHVubmVjZXNzYXJ5IHdvcmsKKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTgxODU2CisKKyAg ICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3Rz LCBjb3ZlcmVkIGJ5IGV4aXN0aW5nIHRlc3RzLgorCisgICAgICAgICogaHRtbC9IVE1MTWVkaWFF bGVtZW50Lmg6CisgICAgICAgIChXZWJDb3JlOjpIVE1MTWVkaWFFbGVtZW50OjppZ25vcmVUcmFj a0Rpc3BsYXlVcGRhdGVSZXF1ZXN0cyk6IEFkZCBhbiBlYXJseSByZXR1cm4gd2hlbiB0aGVyZQor ICAgICAgICAgICAgaXMgb2J2aW91c2x5IG5vIHdvcmsgdG8gYmUgZG9uZS4KKwogMjAxMy0wNS0w MSAgQW5kcmVhcyBLbGluZyAgPGFrbGluZ0BhcHBsZS5jb20+CiAKICAgICAgICAgUkVHUkVTU0lP TihyMTQ5Mjg3KTogQXNzZXJ0aW9uIGZhaWx1cmUgaW4gZmFzdC9mcmFtZXMvZmxhdHRlbmluZy9p ZnJhbWUtZmxhdHRlbmluZy1jcmFzaC5odG1sCkluZGV4OiBTb3VyY2UvV2ViQ29yZS9odG1sL0hU TUxNZWRpYUVsZW1lbnQuaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9odG1sL0hUTUxN ZWRpYUVsZW1lbnQuaAkocmV2aXNpb24gMTQ5Mzc3KQorKysgU291cmNlL1dlYkNvcmUvaHRtbC9I VE1MTWVkaWFFbGVtZW50LmgJKHdvcmtpbmcgY29weSkKQEAgLTQxNSw3ICs0MTUsNyBAQCBwcm90 ZWN0ZWQ6CiAgICAgdm9pZCByZW1vdmVCZWhhdmlvclJlc3RyaWN0aW9uKEJlaGF2aW9yUmVzdHJp Y3Rpb25zIHJlc3RyaWN0aW9uKSB7IG1fcmVzdHJpY3Rpb25zICY9IH5yZXN0cmljdGlvbjsgfQog CiAjaWYgRU5BQkxFKFZJREVPX1RSQUNLKQotICAgIGJvb2wgaWdub3JlVHJhY2tEaXNwbGF5VXBk YXRlUmVxdWVzdHMoKSBjb25zdCB7IHJldHVybiBtX2lnbm9yZVRyYWNrRGlzcGxheVVwZGF0ZSA+ IDA7IH0KKyAgICBib29sIGlnbm9yZVRyYWNrRGlzcGxheVVwZGF0ZVJlcXVlc3RzKCkgY29uc3Qg eyByZXR1cm4gbV9pZ25vcmVUcmFja0Rpc3BsYXlVcGRhdGUgPiAwIHx8ICFtX3RleHRUcmFja3Mg fHwgIW1fY3VlVHJlZS5zaXplKCk7IH0KICAgICB2b2lkIGJlZ2luSWdub3JpbmdUcmFja0Rpc3Bs YXlVcGRhdGVSZXF1ZXN0cygpOwogICAgIHZvaWQgZW5kSWdub3JpbmdUcmFja0Rpc3BsYXlVcGRh dGVSZXF1ZXN0cygpOwogI2VuZGlmCg==