90733 2012-07-07 18:03:57 -0700 [GStreamer] cache video dimensions 2012-07-09 19:40:55 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 pnormand webkit-unassigned eric.carlson feature-media-reviews gustavo menard mrobinson pnormand webkit.review.bot oldest_to_newest 664436 0 pnormand 2012-07-07 18:03:57 -0700 There's no need to query the video-sink sink-pad caps every time in ::naturalSize(). We can simply cache this size every time the pad caps change and reuse the value by returning early from ::naturalSize(). 664437 1 151151 pnormand 2012-07-07 18:11:39 -0700 Created attachment 151151 Patch 664438 2 151151 mrobinson 2012-07-07 18:50:16 -0700 Comment on attachment 151151 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=151151&action=review The logic is a bit hard to follow here. Perhaps you could do this: 1. naturalSize() would be responsible for caching. m_videoSize. 2. In notifyPlayerOfVideo when you want to invalidate the cached video size simply do: m_videoSize = IntSize(); and then let naturalSize() initialize m_videoSize lazily. 664442 3 pnormand 2012-07-07 19:54:52 -0700 (In reply to comment #2) > (From update of attachment 151151 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=151151&action=review > > The logic is a bit hard to follow here. Perhaps you could do this: > > 1. naturalSize() would be responsible for caching. m_videoSize. That's not possible unfortunately because naturalSize() is const :/ > 2. In notifyPlayerOfVideo when you want to invalidate the cached video size simply do: > > m_videoSize = IntSize(); > That I can do yes! :) 664446 4 mrobinson 2012-07-07 20:10:34 -0700 (In reply to comment #3) > That's not possible unfortunately because naturalSize() is const :/ In these situations, I typically just make the cache member mutable. 664450 5 151153 pnormand 2012-07-07 20:34:32 -0700 Created attachment 151153 Patch 665434 6 pnormand 2012-07-09 19:40:55 -0700 Committed r122187: <http://trac.webkit.org/changeset/122187> 151151 2012-07-07 18:11:39 -0700 2012-07-07 20:34:32 -0700 Patch Patch.patch text/plain 3022 pnormand RnJvbSBhNTU4ZTRlYzJjNjJmZmM0OWNlY2IyMWQ5ZWYwNDc4MTU4YTdkMWEwIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQaGlsaXBwZSBOb3JtYW5kIDxwbm9ybWFuZEBpZ2FsaWEuY29t PgpEYXRlOiBTYXQsIDcgSnVsIDIwMTIgMTg6MTE6MjIgLTA3MDAKU3ViamVjdDogW1BBVENIXSBQ YXRjaAoKLS0tCiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgfCAgIDE2ICsrKysrKysrKysrKysrKysKIC4uLi9ncmFwaGljcy9nc3RyZWFtZXIv TWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcCB8ICAgIDggKysrKysrKysKIC4uLi9ncmFw aGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmggICB8ICAgIDEgKwog MyBmaWxlcyBjaGFuZ2VkLCAyNSBpbnNlcnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvU291cmNlL1dl YkNvcmUvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCmluZGV4IDk5NDUxMDYu LmU3NDIxN2IgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxOSBAQAorMjAxMi0wNy0wNyAgUGhpbGlw cGUgTm9ybWFuZCAgPHBub3JtYW5kQGlnYWxpYS5jb20+CisKKyAgICAgICAgW0dTdHJlYW1lcl0g Y2FjaGUgdmlkZW8gZGltZW5zaW9ucworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9OTA3MzMKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBDYWNoZSB0aGUgdmlkZW8gZGltZW5zaW9ucyB3aGVuZXZlciB0aGUgdmlk ZW8tc2luayBzaW5rIHBhZCBjYXBzCisgICAgICAgIGNoYW5nZS4KKworICAgICAgICAqIHBsYXRm b3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6TWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyOjpuYXR1cmFsU2l6 ZSk6CisgICAgICAgIChXZWJDb3JlOjpNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXI6Om5vdGlm eVBsYXllck9mVmlkZW8pOgorICAgICAgICAqIHBsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9N ZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuaDoKKyAgICAgICAgKE1lZGlhUGxheWVyUHJpdmF0 ZUdTdHJlYW1lcik6CisKIDIwMTItMDctMDYgIExhdXJvIE5ldG8gIDxsYXVyby5uZXRvQG9wZW5i b3NzYS5vcmc+CiAKICAgICAgICAgW1F0XSBBdm9pZCBkZWVwIGNvcHkgb2YgUUltYWdlIGluIEdy YXBoaWNzQ29udGV4dDNEOjpnZXRJbWFnZURhdGEoKQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1l ci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQ bGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcAppbmRleCAxMzJkYjBkLi5hMTQ5ZGQ5IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5 ZXJQcml2YXRlR1N0cmVhbWVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFw aGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcApAQCAtNDk1LDYg KzQ5NSw5IEBAIEludFNpemUgTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyOjpuYXR1cmFsU2l6 ZSgpIGNvbnN0CiAgICAgaWYgKCFoYXNWaWRlbygpKQogICAgICAgICByZXR1cm4gSW50U2l6ZSgp OwogCisgICAgaWYgKCFtX3ZpZGVvU2l6ZS5pc0VtcHR5KCkpCisgICAgICAgIHJldHVybiBtX3Zp ZGVvU2l6ZTsKKwogICAgIEdzdENhcHMqIGNhcHMgPSB3ZWJraXRHc3RHZXRQYWRDYXBzKG1fdmlk ZW9TaW5rUGFkLmdldCgpKTsKICAgICBpZiAoIWNhcHMpCiAgICAgICAgIHJldHVybiBJbnRTaXpl KCk7CkBAIC01NjEsNiArNTY0LDExIEBAIHZvaWQgTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVy Ojpub3RpZnlQbGF5ZXJPZlZpZGVvKCkKICAgICAgICAgZ19vYmplY3RfZ2V0KG1fcGxheUJpbiwg Im4tdmlkZW8iLCAmdmlkZW9UcmFja3MsIE5VTEwpOwogCiAgICAgbV9oYXNWaWRlbyA9IHZpZGVv VHJhY2tzID4gMDsKKworICAgIC8vIENhY2hlIHZpZGVvIGRpbWVuc2lvbnMuCisgICAgbV92aWRl b1NpemUuc2V0V2lkdGgoMCk7CisgICAgbV92aWRlb1NpemUuc2V0SGVpZ2h0KDApOworICAgIG1f dmlkZW9TaXplID0gbmF0dXJhbFNpemUoKTsKICAgICBtX3BsYXllci0+bWVkaWFQbGF5ZXJDbGll bnQoKS0+bWVkaWFQbGF5ZXJFbmdpbmVVcGRhdGVkKG1fcGxheWVyKTsKIH0KIApkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxheWVy UHJpdmF0ZUdTdHJlYW1lci5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0 cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5oCmluZGV4IDUwNjk0MmMuLjAxMTJl MzMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1l ci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0 Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmgKQEAg LTE5NCw2ICsxOTQsNyBAQCBjbGFzcyBNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIgOiBwdWJs aWMgTWVkaWFQbGF5ZXJQcml2YXRlSW50ZXJmYWNlIHsKICAgICAgICAgICAgIGd1aW50IG1fdmlk ZW9UaW1lckhhbmRsZXI7CiAgICAgICAgICAgICBHUmVmUHRyPEdzdEVsZW1lbnQ+IG1fd2Via2l0 QXVkaW9TaW5rOwogICAgICAgICAgICAgR1JlZlB0cjxHc3RQYWQ+IG1fdmlkZW9TaW5rUGFkOwor ICAgICAgICAgICAgSW50U2l6ZSBtX3ZpZGVvU2l6ZTsKICAgICB9OwogfQogCi0tIAoxLjcuMTAu NA== 151153 2012-07-07 20:34:32 -0700 2012-07-09 07:58:39 -0700 Patch Patch.patch text/plain 3425 pnormand RnJvbSA5YjQ1N2IyYWQ5MzNkNGNhOWFjM2VmZjYzZTk1OWEyOGMxMTNjMzlkIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQaGlsaXBwZSBOb3JtYW5kIDxwbm9ybWFuZEBpZ2FsaWEuY29t PgpEYXRlOiBTYXQsIDcgSnVsIDIwMTIgMjA6MzM6MzMgLTA3MDAKU3ViamVjdDogW1BBVENIXSBQ YXRjaAoKLS0tCiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgfCAgIDE2ICsrKysrKysrKysrKysrKysKIC4uLi9ncmFwaGljcy9nc3RyZWFtZXIv TWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmNwcCB8ICAgIDkgKysrKysrKystCiAuLi4vZ3Jh cGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5oICAgfCAgICAxICsK IDMgZmlsZXMgY2hhbmdlZCwgMjUgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQoKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxv ZwppbmRleCA5OTQ1MTA2Li4yYjMwMDdjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFu Z2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIw MTItMDctMDcgIFBoaWxpcHBlIE5vcm1hbmQgIDxwbm9ybWFuZEBpZ2FsaWEuY29tPgorCisgICAg ICAgIFtHU3RyZWFtZXJdIGNhY2hlIHZpZGVvIGRpbWVuc2lvbnMKKyAgICAgICAgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTkwNzMzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSW52YWxpZGF0ZSB0aGUgY2FjaGVkIHZpZGVv IGRpbWVuc2lvbnMgd2hlbmV2ZXIgdGhlIHZpZGVvLXNpbmsgc2luayBwYWQgY2FwcworICAgICAg ICBjaGFuZ2UgYW5kIGxldCA6Om5hdHVyYWxTaXplKCkgbGF6aWx5IHJlY2FsY3VsYXRlIHRoZW0u CisKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2 YXRlR1N0cmVhbWVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhUGxheWVyUHJpdmF0ZUdT dHJlYW1lcjo6bmF0dXJhbFNpemUpOgorICAgICAgICAoV2ViQ29yZTo6TWVkaWFQbGF5ZXJQcml2 YXRlR1N0cmVhbWVyOjpub3RpZnlQbGF5ZXJPZlZpZGVvKToKKyAgICAgICAgKiBwbGF0Zm9ybS9n cmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmg6CisgICAgICAg IChNZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIpOgorCiAyMDEyLTA3LTA2ICBMYXVybyBOZXRv ICA8bGF1cm8ubmV0b0BvcGVuYm9zc2Eub3JnPgogCiAgICAgICAgIFtRdF0gQXZvaWQgZGVlcCBj b3B5IG9mIFFJbWFnZSBpbiBHcmFwaGljc0NvbnRleHQzRDo6Z2V0SW1hZ2VEYXRhKCkKZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBs YXllclByaXZhdGVHU3RyZWFtZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvZ3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5jcHAKaW5kZXggMTMyZGIw ZC4uZjJiODZiMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mv Z3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5jcHAKKysrIGIvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJl YW1lci5jcHAKQEAgLTQ5NSw2ICs0OTUsOSBAQCBJbnRTaXplIE1lZGlhUGxheWVyUHJpdmF0ZUdT dHJlYW1lcjo6bmF0dXJhbFNpemUoKSBjb25zdAogICAgIGlmICghaGFzVmlkZW8oKSkKICAgICAg ICAgcmV0dXJuIEludFNpemUoKTsKIAorICAgIGlmICghbV92aWRlb1NpemUuaXNFbXB0eSgpKQor ICAgICAgICByZXR1cm4gbV92aWRlb1NpemU7CisKICAgICBHc3RDYXBzKiBjYXBzID0gd2Via2l0 R3N0R2V0UGFkQ2FwcyhtX3ZpZGVvU2lua1BhZC5nZXQoKSk7CiAgICAgaWYgKCFjYXBzKQogICAg ICAgICByZXR1cm4gSW50U2l6ZSgpOwpAQCAtNTQyLDcgKzU0NSw4IEBAIEludFNpemUgTWVkaWFQ bGF5ZXJQcml2YXRlR1N0cmVhbWVyOjpuYXR1cmFsU2l6ZSgpIGNvbnN0CiAgICAgfQogCiAgICAg TE9HX1ZFUkJPU0UoTWVkaWEsICJOYXR1cmFsIHNpemU6ICUiIEdfR1VJTlQ2NF9GT1JNQVQgIngl IiBHX0dVSU5UNjRfRk9STUFULCB3aWR0aCwgaGVpZ2h0KTsKLSAgICByZXR1cm4gSW50U2l6ZShz dGF0aWNfY2FzdDxpbnQ+KHdpZHRoKSwgc3RhdGljX2Nhc3Q8aW50PihoZWlnaHQpKTsKKyAgICBt X3ZpZGVvU2l6ZSA9IEludFNpemUoc3RhdGljX2Nhc3Q8aW50Pih3aWR0aCksIHN0YXRpY19jYXN0 PGludD4oaGVpZ2h0KSk7CisgICAgcmV0dXJuIG1fdmlkZW9TaXplOwogfQogCiB2b2lkIE1lZGlh UGxheWVyUHJpdmF0ZUdTdHJlYW1lcjo6dmlkZW9DaGFuZ2VkKCkKQEAgLTU2MSw2ICs1NjUsOSBA QCB2b2lkIE1lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lcjo6bm90aWZ5UGxheWVyT2ZWaWRlbygp CiAgICAgICAgIGdfb2JqZWN0X2dldChtX3BsYXlCaW4sICJuLXZpZGVvIiwgJnZpZGVvVHJhY2tz LCBOVUxMKTsKIAogICAgIG1faGFzVmlkZW8gPSB2aWRlb1RyYWNrcyA+IDA7CisKKyAgICBtX3Zp ZGVvU2l6ZSA9IEludFNpemUoKTsKKwogICAgIG1fcGxheWVyLT5tZWRpYVBsYXllckNsaWVudCgp LT5tZWRpYVBsYXllckVuZ2luZVVwZGF0ZWQobV9wbGF5ZXIpOwogfQogCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2 YXRlR1N0cmVhbWVyLmggYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFt ZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyLmgKaW5kZXggNTA2OTQyYy4uOWUyNzdjYyAx MDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01l ZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuaApAQCAtMTk0 LDYgKzE5NCw3IEBAIGNsYXNzIE1lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lciA6IHB1YmxpYyBN ZWRpYVBsYXllclByaXZhdGVJbnRlcmZhY2UgewogICAgICAgICAgICAgZ3VpbnQgbV92aWRlb1Rp bWVySGFuZGxlcjsKICAgICAgICAgICAgIEdSZWZQdHI8R3N0RWxlbWVudD4gbV93ZWJraXRBdWRp b1Npbms7CiAgICAgICAgICAgICBHUmVmUHRyPEdzdFBhZD4gbV92aWRlb1NpbmtQYWQ7CisgICAg ICAgICAgICBtdXRhYmxlIEludFNpemUgbV92aWRlb1NpemU7CiAgICAgfTsKIH0KIAotLSAKMS43 LjEwLjQ=