134949 2014-07-15 16:03:32 -0700 [MSE] http/tests/media/media-source/mediasource-buffered.html is flakey 2014-07-21 09:52:37 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 135118 1 jer.noble jer.noble calvaris commit-queue eric.carlson glenn ltilve philipj sergio oldest_to_newest 1022588 0 jer.noble 2014-07-15 16:03:32 -0700 [MSE] http/tests/media/media-source/mediasource-buffered.html is flakey 1022783 1 235010 jer.noble 2014-07-16 11:28:14 -0700 Created attachment 235010 Patch 1022819 2 235010 commit-queue 2014-07-16 13:51:40 -0700 Comment on attachment 235010 Patch Clearing flags on attachment: 235010 Committed r171151: <http://trac.webkit.org/changeset/171151> 1022820 3 commit-queue 2014-07-16 13:51:43 -0700 All reviewed patches have been landed. Closing bug. 1023500 4 235010 darin 2014-07-19 22:56:04 -0700 Comment on attachment 235010 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235010&action=review > Source/WebCore/Modules/mediasource/MediaSource.cpp:747 > +void MediaSource::sourceBufferDidChangeAcitveState(SourceBuffer*, bool) Noticed that this function name misspells Active. > Source/WebCore/Modules/mediasource/MediaSource.cpp:874 > + for (auto& sourceBuffer : *m_sourceBuffers) { > + if (sourceBuffer->active()) > + newList.append(sourceBuffer); > + } Regenerating the list every time seems like it will make things O(n^2) when they needn’t be. Is there a way to be lazy about this? Maybe O(n^2) is not a problem? > Source/WebCore/Modules/mediasource/MediaSource.cpp:875 > + m_activeSourceBuffers->swap(newList); Why swap? Can’t we just set it instead? Swapping is extra work. And with move semantics it should be easy to make a set function. > Source/WebCore/Modules/mediasource/SourceBufferList.cpp:78 > + if (!m_list.contains(sourceBuffer)) This O(n^2) algorithm seems like a bad idea, but maybe it’s OK. Worse, if this item is called every time a source buffer changes active state, activating all the source buffers makes this O(n^3). I worry that even with some relatively small numbers this could possibly bite us. > Source/WebCore/Modules/mediasource/SourceBufferList.h:61 > + void swap(Vector<RefPtr<SourceBuffer>>&); As I mentioned above, this should be: void set(Vector<RefPtr<SourceBuffer>>&&); 1023687 5 235010 jer.noble 2014-07-21 09:50:22 -0700 Comment on attachment 235010 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235010&action=review >> Source/WebCore/Modules/mediasource/MediaSource.cpp:874 >> + } > > Regenerating the list every time seems like it will make things O(n^2) when they needn’t be. Is there a way to be lazy about this? Maybe O(n^2) is not a problem? Generally speaking, these entries only have 2 items in them, a video and audio track, and definitely no more than 10. And the list only needs to be regenerated when they become active or inactive (like when a user choses a different audio track). So regardless of the O(), this will be a fast operation. >> Source/WebCore/Modules/mediasource/MediaSource.cpp:875 >> + m_activeSourceBuffers->swap(newList); > > Why swap? Can’t we just set it instead? Swapping is extra work. And with move semantics it should be easy to make a set function. Ah yes! I haven't internalized move semantics yet. >> Source/WebCore/Modules/mediasource/SourceBufferList.cpp:78 >> + if (!m_list.contains(sourceBuffer)) > > This O(n^2) algorithm seems like a bad idea, but maybe it’s OK. > > Worse, if this item is called every time a source buffer changes active state, activating all the source buffers makes this O(n^3). I worry that even with some relatively small numbers this could possibly bite us. The other option here would be to have a SourceBufferList expose two interfaces: the master SourceBufferList, and a list of only the "active" SourceBuffers. This would move all the complexity to the accessing side, rather than the modification side. I'll open up a new bug to look into that alternative. (Which would remove the need for the set() function above.) 235010 2014-07-16 11:28:14 -0700 2014-07-21 09:50:22 -0700 Patch bug-134949-20140716112758.patch text/plain 5882 jer.noble U3VidmVyc2lvbiBSZXZpc2lvbjogMTcxMTE3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggODIxN2ZkZTRiZDBhN2Zi MWI4N2JkMDU4ZThmNjQxOTNiOThlZjk0ZS4uNmU3NDg1YWU0MmQ2YTFhOWI5MDdlYmNjNWRlOTY0 MjQwMWRlZDhkMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDE0LTA3LTE1ICBKZXIg Tm9ibGUgIDxqZXIubm9ibGVAYXBwbGUuY29tPgorCisgICAgICAgIFtNU0VdIGh0dHAvdGVzdHMv bWVkaWEvbWVkaWEtc291cmNlL21lZGlhc291cmNlLWJ1ZmZlcmVkLmh0bWwgaXMgZmxha2V5Cisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzQ5NDkKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBEZXBlbmRpbmcg b24gd2hpY2ggU291cmNlQnVmZmVyIGlzIHN1Y2Nlc3NmdWxseSBwYXJzZWQgZmlyc3QsIHRoZSBv cmRlciBvZiBhY3RpdmVTb3VyY2VCdWZmZXJzCisgICAgICAgIG1heSBjaGFuZ2UgZnJvbSBydW4g dG8gcnVuLCBicmVha2luZyB0aGUgaHR0cC90ZXN0cy9tZWRpYS9tZWRpYS1zb3VyY2UvbWVkaWFz b3VyY2UtYnVmZmVyZWQuaHRtbAorICAgICAgICB0ZXN0LiBNYWtlIHRoZSBvcmRlciBvZiBhY3Rp dmVTb3VyY2VCdWZmZXJzIHRoZSBzYW1lIGFzIChhIHN1YnNldCBvZikgc291cmNlQnVmZmVycywg YnkgcmVwbGFjaW5nCisgICAgICAgIGNhbGxzIHRvIGFjdGl2ZVNvdXJjZUJ1ZmZlcnMtPmFkZCgp IGFuZCB3aXRoIGEgbmV3IGNhbGwgdG8gcmVnZW5lcmF0ZUFjdGl2ZVNvdXJjZUJ1ZmZlcnMoKSwg d2hpY2gKKyAgICAgICAgc3dhcHMgdGhlIGNvbnRlbnRzIG9mIHRoZSBTb3VyY2VCdWZmZXJMaXN0 IHdpdGggYSBuZXcsIG9yZGVyZWQgVmVjdG9yLgorCisgICAgICAgICogTW9kdWxlcy9tZWRpYXNv dXJjZS9NZWRpYVNvdXJjZS5jcHA6CisgICAgICAgIChXZWJDb3JlOjpNZWRpYVNvdXJjZTo6YWRk U291cmNlQnVmZmVyKToKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhU291cmNlOjpzb3VyY2VCdWZm ZXJEaWRDaGFuZ2VBY2l0dmVTdGF0ZSk6CisgICAgICAgIChXZWJDb3JlOjpNZWRpYVNvdXJjZTo6 cmVnZW5lcmF0ZUFjdGl2ZVNvdXJjZUJ1ZmZlcnMpOgorICAgICAgICAqIE1vZHVsZXMvbWVkaWFz b3VyY2UvTWVkaWFTb3VyY2UuaDoKKyAgICAgICAgKiBNb2R1bGVzL21lZGlhc291cmNlL1NvdXJj ZUJ1ZmZlckxpc3QuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U291cmNlQnVmZmVyTGlzdDo6c3dh cCk6CisgICAgICAgICogTW9kdWxlcy9tZWRpYXNvdXJjZS9Tb3VyY2VCdWZmZXJMaXN0Lmg6CisK IDIwMTQtMDctMTYgIEplciBOb2JsZSAgPGplci5ub2JsZUBhcHBsZS5jb20+CiAKICAgICAgICAg UkVHUkVTU0lPTihyMTcxMDY5KSA3NSUgcmVwcm8gY3Jhc2ggaW4gV2ViQ29yZTo6QXVkaW9IYXJk d2FyZUxpc3RlbmVyTWFjOjpwcm9jZXNzSXNSdW5uaW5nQ2hhbmdlZCgpCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc291cmNlL01lZGlhU291cmNlLmNwcCBiL1NvdXJj ZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzb3VyY2UvTWVkaWFTb3VyY2UuY3BwCmluZGV4IDZhYWU2 ZTc3YTNiYzVjOTEyYjRhOTY2N2EwMzNmYWI4YTE4NDVkNWYuLmQ4OWJkMGM2YjU1ZjQwYzU2OTlk Y2U2ZTdiZTA1YTAzNjdlNWZiNzAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMv bWVkaWFzb3VyY2UvTWVkaWFTb3VyY2UuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMv bWVkaWFzb3VyY2UvTWVkaWFTb3VyY2UuY3BwCkBAIC01NDAsOSArNTQwLDggQEAgU291cmNlQnVm ZmVyKiBNZWRpYVNvdXJjZTo6YWRkU291cmNlQnVmZmVyKGNvbnN0IFN0cmluZyYgdHlwZSwgRXhj ZXB0aW9uQ29kZSYgZWMKICAgICBSZWZQdHI8U291cmNlQnVmZmVyPiBidWZmZXIgPSBTb3VyY2VC dWZmZXI6OmNyZWF0ZShzb3VyY2VCdWZmZXJQcml2YXRlLnJlbGVhc2VOb25OdWxsKCksIHRoaXMp OwogICAgIC8vIDYuIEFkZCB0aGUgbmV3IG9iamVjdCB0byBzb3VyY2VCdWZmZXJzIGFuZCBmaXJl IGEgYWRkc291cmNlYnVmZmVyIG9uIHRoYXQgb2JqZWN0LgogICAgIG1fc291cmNlQnVmZmVycy0+ YWRkKGJ1ZmZlcik7CisgICAgcmVnZW5lcmF0ZUFjdGl2ZVNvdXJjZUJ1ZmZlcnMoKTsKIAotICAg IGlmIChidWZmZXItPmFjdGl2ZSgpKQotICAgICAgICBtX2FjdGl2ZVNvdXJjZUJ1ZmZlcnMtPmFk ZChidWZmZXIpOwogICAgIC8vIDcuIFJldHVybiB0aGUgbmV3IG9iamVjdCB0byB0aGUgY2FsbGVy LgogICAgIHJldHVybiBidWZmZXIuZ2V0KCk7CiB9CkBAIC03NDUsMTIgKzc0NCw5IEBAIHZvaWQg TWVkaWFTb3VyY2U6OmNsb3NlKCkKICAgICBzZXRSZWFkeVN0YXRlKGNsb3NlZEtleXdvcmQoKSk7 CiB9CiAKLXZvaWQgTWVkaWFTb3VyY2U6OnNvdXJjZUJ1ZmZlckRpZENoYW5nZUFjaXR2ZVN0YXRl KFNvdXJjZUJ1ZmZlciogc291cmNlQnVmZmVyLCBib29sIGFjdGl2ZSkKK3ZvaWQgTWVkaWFTb3Vy Y2U6OnNvdXJjZUJ1ZmZlckRpZENoYW5nZUFjaXR2ZVN0YXRlKFNvdXJjZUJ1ZmZlciosIGJvb2wp CiB7Ci0gICAgaWYgKGFjdGl2ZSAmJiAhbV9hY3RpdmVTb3VyY2VCdWZmZXJzLT5jb250YWlucyhz b3VyY2VCdWZmZXIpKQotICAgICAgICBtX2FjdGl2ZVNvdXJjZUJ1ZmZlcnMtPmFkZChzb3VyY2VC dWZmZXIpOwotICAgIGVsc2UgaWYgKCFhY3RpdmUgJiYgbV9hY3RpdmVTb3VyY2VCdWZmZXJzLT5j b250YWlucyhzb3VyY2VCdWZmZXIpKQotICAgICAgICBtX2FjdGl2ZVNvdXJjZUJ1ZmZlcnMtPnJl bW92ZShzb3VyY2VCdWZmZXIpOworICAgIHJlZ2VuZXJhdGVBY3RpdmVTb3VyY2VCdWZmZXJzKCk7 CiB9CiAKIGJvb2wgTWVkaWFTb3VyY2U6OmF0dGFjaFRvRWxlbWVudChIVE1MTWVkaWFFbGVtZW50 KiBlbGVtZW50KQpAQCAtODY5LDYgKzg2NSwxNiBAQCBVUkxSZWdpc3RyeSYgTWVkaWFTb3VyY2U6 OnJlZ2lzdHJ5KCkgY29uc3QKICAgICByZXR1cm4gTWVkaWFTb3VyY2VSZWdpc3RyeTo6cmVnaXN0 cnkoKTsKIH0KIAordm9pZCBNZWRpYVNvdXJjZTo6cmVnZW5lcmF0ZUFjdGl2ZVNvdXJjZUJ1ZmZl cnMoKQoreworICAgIFZlY3RvcjxSZWZQdHI8U291cmNlQnVmZmVyPj4gbmV3TGlzdDsKKyAgICBm b3IgKGF1dG8mIHNvdXJjZUJ1ZmZlciA6ICptX3NvdXJjZUJ1ZmZlcnMpIHsKKyAgICAgICAgaWYg KHNvdXJjZUJ1ZmZlci0+YWN0aXZlKCkpCisgICAgICAgICAgICBuZXdMaXN0LmFwcGVuZChzb3Vy Y2VCdWZmZXIpOworICAgIH0KKyAgICBtX2FjdGl2ZVNvdXJjZUJ1ZmZlcnMtPnN3YXAobmV3TGlz dCk7Cit9CisKIH0KIAogI2VuZGlmCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVz L21lZGlhc291cmNlL01lZGlhU291cmNlLmggYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlh c291cmNlL01lZGlhU291cmNlLmgKaW5kZXggNzM4OWRiMTQzNDUzNjkzOTBlYmEwNWI4OWQ0YWQ4 MmE1NzA2NmExOS4uOTNlNGRkZTM3MDExZTQ4NDY0ZDBmZjQ5YWU3ZjY1N2IyOGJiZmEyYiAxMDA2 NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXNvdXJjZS9NZWRpYVNvdXJjZS5o CisrKyBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzb3VyY2UvTWVkaWFTb3VyY2UuaApA QCAtMTIzLDYgKzEyMyw4IEBAIHByb3RlY3RlZDoKICAgICB2b2lkIHNjaGVkdWxlRXZlbnQoY29u c3QgQXRvbWljU3RyaW5nJiBldmVudE5hbWUpOwogICAgIEdlbmVyaWNFdmVudFF1ZXVlJiBhc3lu Y0V2ZW50UXVldWUoKSB7IHJldHVybiBtX2FzeW5jRXZlbnRRdWV1ZTsgfQogCisgICAgdm9pZCBy ZWdlbmVyYXRlQWN0aXZlU291cmNlQnVmZmVycygpOworCiAgICAgc3RhdGljIFVSTFJlZ2lzdHJ5 KiBzX3JlZ2lzdHJ5OwogCiAgICAgUmVmUHRyPE1lZGlhU291cmNlUHJpdmF0ZT4gbV9wcml2YXRl OwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXNvdXJjZS9Tb3VyY2VC dWZmZXJMaXN0LmNwcCBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzb3VyY2UvU291cmNl QnVmZmVyTGlzdC5jcHAKaW5kZXggMmRkN2I4MWYwYjI4YmYzOWRjMzYxMDBlMjcxZjY3MmFlOGMy ZDhjZC4uMDJmZTc4MjhkN2FlMWM3MmM5OTc3MmIyOGQ2NWM2ZGRhNTU1ODViMyAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXNvdXJjZS9Tb3VyY2VCdWZmZXJMaXN0LmNw cAorKysgYi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc291cmNlL1NvdXJjZUJ1ZmZlckxp c3QuY3BwCkBAIC03MCw2ICs3MCwyNCBAQCB2b2lkIFNvdXJjZUJ1ZmZlckxpc3Q6OmNsZWFyKCkK ICAgICBzY2hlZHVsZUV2ZW50KGV2ZW50TmFtZXMoKS5yZW1vdmVzb3VyY2VidWZmZXJFdmVudCk7 CiB9CiAKK3ZvaWQgU291cmNlQnVmZmVyTGlzdDo6c3dhcChWZWN0b3I8UmVmUHRyPFNvdXJjZUJ1 ZmZlcj4+JiBvdGhlcikKK3sKKyAgICBpbnQgY2hhbmdlSW5TaXplID0gb3RoZXIuc2l6ZSgpIC0g bV9saXN0LnNpemUoKTsKKyAgICBpbnQgYWRkZWRFbnRyaWVzID0gMDsKKyAgICBmb3IgKGF1dG8m IHNvdXJjZUJ1ZmZlciA6IG90aGVyKSB7CisgICAgICAgIGlmICghbV9saXN0LmNvbnRhaW5zKHNv dXJjZUJ1ZmZlcikpCisgICAgICAgICAgICArK2FkZGVkRW50cmllczsKKyAgICB9CisgICAgaW50 IHJlbW92ZWRFbnRyaWVzID0gYWRkZWRFbnRyaWVzIC0gY2hhbmdlSW5TaXplOworCisgICAgbV9s aXN0LnN3YXAob3RoZXIpOworCisgICAgaWYgKGFkZGVkRW50cmllcykKKyAgICAgICAgc2NoZWR1 bGVFdmVudChldmVudE5hbWVzKCkuYWRkc291cmNlYnVmZmVyRXZlbnQpOworICAgIGlmIChyZW1v dmVkRW50cmllcykKKyAgICAgICAgc2NoZWR1bGVFdmVudChldmVudE5hbWVzKCkucmVtb3Zlc291 cmNlYnVmZmVyRXZlbnQpOworfQorCiB2b2lkIFNvdXJjZUJ1ZmZlckxpc3Q6OnNjaGVkdWxlRXZl bnQoY29uc3QgQXRvbWljU3RyaW5nJiBldmVudE5hbWUpCiB7CiAgICAgUmVmUHRyPEV2ZW50PiBl dmVudCA9IEV2ZW50OjpjcmVhdGUoZXZlbnROYW1lLCBmYWxzZSwgZmFsc2UpOwpkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9tZWRpYXNvdXJjZS9Tb3VyY2VCdWZmZXJMaXN0Lmgg Yi9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc291cmNlL1NvdXJjZUJ1ZmZlckxpc3QuaApp bmRleCBmYTIzN2IzODUzZjM1ZDQ0ZGY3OGUwZTc1NjBkMjBkYzA1ZTI2NDYxLi42YmNiOTM1NzJl MDQzY2Q0Mzg1NmM2NjgyNzQ4MmQ5MjM0NzZlYWU2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9Nb2R1bGVzL21lZGlhc291cmNlL1NvdXJjZUJ1ZmZlckxpc3QuaAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9Nb2R1bGVzL21lZGlhc291cmNlL1NvdXJjZUJ1ZmZlckxpc3QuaApAQCAtNTgsNiArNTgs NyBAQCBwdWJsaWM6CiAgICAgdm9pZCByZW1vdmUoU291cmNlQnVmZmVyKik7CiAgICAgYm9vbCBj b250YWlucyhTb3VyY2VCdWZmZXIqIGJ1ZmZlcikgeyByZXR1cm4gbV9saXN0LmZpbmQoYnVmZmVy KSAhPSBub3RGb3VuZDsgfQogICAgIHZvaWQgY2xlYXIoKTsKKyAgICB2b2lkIHN3YXAoVmVjdG9y PFJlZlB0cjxTb3VyY2VCdWZmZXI+PiYpOwogCiAgICAgVmVjdG9yPFJlZlB0cjxTb3VyY2VCdWZm ZXI+Pjo6aXRlcmF0b3IgYmVnaW4oKSB7IHJldHVybiBtX2xpc3QuYmVnaW4oKTsgfQogICAgIFZl Y3RvcjxSZWZQdHI8U291cmNlQnVmZmVyPj46Oml0ZXJhdG9yIGVuZCgpIHsgcmV0dXJuIG1fbGlz dC5lbmQoKTsgfQo=