113880 2013-04-03 08:58:05 -0700 [Gstreamer] Use gst_buffer_extract() in copyGstreamerBuffersToAudioChannel() 2013-04-03 11:11:11 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 cdumez cdumez eric.carlson feature-media-reviews jer.noble laszlo.gombos mrobinson pnormand webkit.review.bot oldest_to_newest 868747 0 cdumez 2013-04-03 08:58:05 -0700 copyGstreamerBuffersToAudioChannel() currently maps the GstBuffer content to memcpy it the the AudioChannel buffer. We could leverage gst_buffer_extract() API function to simplify the code and avoid issues with error handling. 868751 1 196362 cdumez 2013-04-03 09:08:38 -0700 Created attachment 196362 Patch 868760 2 196362 pnormand 2013-04-03 09:33:33 -0700 Comment on attachment 196362 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196362&action=review Looks cool indeed! > Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:105 > + guint bufferCount = gst_buffer_list_length(buffers); > + for (guint i = 0; i < bufferCount; ++i) { bufferCount is not needed I think. Also please use unsigned instead of guint if possible, we try to give preference to non-glib types when possible. 868787 3 196362 cdumez 2013-04-03 10:29:59 -0700 Comment on attachment 196362 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=196362&action=review >> Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:105 >> + for (guint i = 0; i < bufferCount; ++i) { > > bufferCount is not needed I think. Also please use unsigned instead of guint if possible, we try to give preference to non-glib types when possible. Caching the container size before the loop is commonly done in WebKit and it avoids calling gst_buffer_list_length() for each iteration. It's not a big deal here because gst_buffer_list_length() is inexpensive but I still think it is good practice. I'll replace guint by unsigned, I wasn't sure what the policy was about this. 868789 4 196374 cdumez 2013-04-03 10:34:14 -0700 Created attachment 196374 Patch Using unsigned instead of guint. I kept the container size caching for now. 868802 5 196374 webkit.review.bot 2013-04-03 11:11:07 -0700 Comment on attachment 196374 Patch Clearing flags on attachment: 196374 Committed r147567: <http://trac.webkit.org/changeset/147567> 868803 6 webkit.review.bot 2013-04-03 11:11:11 -0700 All reviewed patches have been landed. Closing bug. 196362 2013-04-03 09:08:38 -0700 2013-04-03 10:34:14 -0700 Patch 113880_gst_buffer_extract.patch text/plain 2720 cdumez ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA5YzZlOGZhLi5hNWMwZDg0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjUg QEAKKzIwMTMtMDQtMDMgIENocmlzdG9waGUgRHVtZXogIDxjaC5kdW1lekBzaXNhLnNhbXN1bmcu Y29tPgorCisgICAgICAgIFtHc3RyZWFtZXJdIFVzZSBnc3RfYnVmZmVyX2V4dHJhY3QoKSBpbiBj b3B5R3N0cmVhbWVyQnVmZmVyc1RvQXVkaW9DaGFubmVsKCkKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMzg4MAorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGNvcHlHc3RyZWFtZXJCdWZmZXJzVG9BdWRpb0No YW5uZWwoKSB3YXMgbWFwcGluZyB0aGUgR3N0QnVmZmVyIGNvbnRlbnQgdG8gbWVtY3B5CisgICAg ICAgIGl0IHRvIHRoZSBBdWRpb0NoYW5uZWwgYnVmZmVyLiBUaGlzIHBhdGNoIGxldmVyYWdlcyBn c3RfYnVmZmVyX2V4dHJhY3QoKSB0bworICAgICAgICBzaW1wbGlmeSB0aGUgY29kZSBhcyBpdCBk b2VzIGV4YWN0bHkgd2hhdCB3ZSBuZWVkOiBnc3RfYnVmZmVyX21hcCwgbWVtY3B5LAorICAgICAg ICBnc3RfYnVmZmVyX3VubWFwIGFuZCBlcnJvciBoYW5kbGluZy4KKworICAgICAgICBBbHNvIHJl cGxhY2UgR3N0QnVmZmVyIE5VTEwgY2hlY2sgYnkgYW4gYXNzZXJ0aW9uIGFzIHdlIGFscmVhZHkg bWFrZSBzdXJlIHRoZXkKKyAgICAgICAgYXJlIG5vdCBOVUxMIGJlZm9yZSBhZGRpbmcgdGhlbSB0 byB0aGUgbGlzdC4gQWRkaXRpb25hbGx5LCB3ZSBub3cgY2FsbAorICAgICAgICBhdWRpb0NoYW5u ZWwtPm11dGFibGVEYXRhKCkgb25seSBvbmNlIGluc3RlYWQgb2Ygb25jZSBwZXIgaXRlcmF0aW9u LiBJdCBpcworICAgICAgICBhIGJpdCBiZXR0ZXIgYXMgdGhlIGdldHRlciBkb2VzIHNvbWUgd29y ayBpbnRlcm5hbGx5LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cywgbm8gYmVoYXZpb3IgY2hhbmdl LgorCisgICAgICAgICogcGxhdGZvcm0vYXVkaW8vZ3N0cmVhbWVyL0F1ZGlvRmlsZVJlYWRlckdT dHJlYW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpjb3B5R3N0cmVhbWVyQnVmZmVyc1RvQXVk aW9DaGFubmVsKToKKwogMjAxMy0wNC0wMyAgQ3NhYmEgT3N6dHJvZ29uw6FjICA8b3NzeUB3ZWJr aXQub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQgMzJiaXQgYnVpbGRmaXggYWZ0ZXIgcjE0NzU0 Mi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2F1ZGlvL2dzdHJlYW1lci9B dWRpb0ZpbGVSZWFkZXJHU3RyZWFtZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vYXVk aW8vZ3N0cmVhbWVyL0F1ZGlvRmlsZVJlYWRlckdTdHJlYW1lci5jcHAKaW5kZXggOGVmYjZmYy4u ZTkwNDc5MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vYXVkaW8vZ3N0cmVh bWVyL0F1ZGlvRmlsZVJlYWRlckdTdHJlYW1lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vYXVkaW8vZ3N0cmVhbWVyL0F1ZGlvRmlsZVJlYWRlckdTdHJlYW1lci5jcHAKQEAgLTEw MCwxNiArMTAwLDE0IEBAIHByaXZhdGU6CiBzdGF0aWMgdm9pZCBjb3B5R3N0cmVhbWVyQnVmZmVy c1RvQXVkaW9DaGFubmVsKEdzdEJ1ZmZlckxpc3QqIGJ1ZmZlcnMsIEF1ZGlvQ2hhbm5lbCogYXVk aW9DaGFubmVsKQogewogI2lmZGVmIEdTVF9BUElfVkVSU0lPTl8xCi0gICAgZ3NpemUgb2Zmc2V0 ID0gMDsKLSAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgZ3N0X2J1ZmZlcl9saXN0X2xlbmd0 aChidWZmZXJzKTsgaSsrKSB7CisgICAgZmxvYXQqIGRlc3RpbmF0aW9uID0gYXVkaW9DaGFubmVs LT5tdXRhYmxlRGF0YSgpOworICAgIGd1aW50IGJ1ZmZlckNvdW50ID0gZ3N0X2J1ZmZlcl9saXN0 X2xlbmd0aChidWZmZXJzKTsKKyAgICBmb3IgKGd1aW50IGkgPSAwOyBpIDwgYnVmZmVyQ291bnQ7 ICsraSkgewogICAgICAgICBHc3RCdWZmZXIqIGJ1ZmZlciA9IGdzdF9idWZmZXJfbGlzdF9nZXQo YnVmZmVycywgaSk7Ci0gICAgICAgIGlmICghYnVmZmVyKQotICAgICAgICAgICAgY29udGludWU7 Ci0gICAgICAgIEdzdE1hcEluZm8gaW5mbzsKLSAgICAgICAgZ3N0X2J1ZmZlcl9tYXAoYnVmZmVy LCAmaW5mbywgR1NUX01BUF9SRUFEKTsKLSAgICAgICAgbWVtY3B5KGF1ZGlvQ2hhbm5lbC0+bXV0 YWJsZURhdGEoKSArIG9mZnNldCwgcmVpbnRlcnByZXRfY2FzdDxmbG9hdCo+KGluZm8uZGF0YSks IGluZm8uc2l6ZSk7Ci0gICAgICAgIG9mZnNldCArPSBpbmZvLnNpemUgLyBzaXplb2YoZmxvYXQp OwotICAgICAgICBnc3RfYnVmZmVyX3VubWFwKGJ1ZmZlciwgJmluZm8pOworICAgICAgICBBU1NF UlQoYnVmZmVyKTsKKyAgICAgICAgZ3NpemUgYnVmZmVyU2l6ZSA9IGdzdF9idWZmZXJfZ2V0X3Np emUoYnVmZmVyKTsKKyAgICAgICAgZ3N0X2J1ZmZlcl9leHRyYWN0KGJ1ZmZlciwgMCwgZGVzdGlu YXRpb24sIGJ1ZmZlclNpemUpOworICAgICAgICBkZXN0aW5hdGlvbiArPSBidWZmZXJTaXplIC8g c2l6ZW9mKGZsb2F0KTsKICAgICB9CiAjZWxzZQogICAgIEdzdEJ1ZmZlckxpc3RJdGVyYXRvciog aXRlciA9IGdzdF9idWZmZXJfbGlzdF9pdGVyYXRlKGJ1ZmZlcnMpOwo= 196374 2013-04-03 10:34:14 -0700 2013-04-03 11:11:07 -0700 Patch 113880_gst_buffer_extract.patch text/plain 2726 cdumez ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA5YzZlOGZhLi5hNWMwZDg0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMjUg QEAKKzIwMTMtMDQtMDMgIENocmlzdG9waGUgRHVtZXogIDxjaC5kdW1lekBzaXNhLnNhbXN1bmcu Y29tPgorCisgICAgICAgIFtHc3RyZWFtZXJdIFVzZSBnc3RfYnVmZmVyX2V4dHJhY3QoKSBpbiBj b3B5R3N0cmVhbWVyQnVmZmVyc1RvQXVkaW9DaGFubmVsKCkKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTExMzg4MAorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIGNvcHlHc3RyZWFtZXJCdWZmZXJzVG9BdWRpb0No YW5uZWwoKSB3YXMgbWFwcGluZyB0aGUgR3N0QnVmZmVyIGNvbnRlbnQgdG8gbWVtY3B5CisgICAg ICAgIGl0IHRvIHRoZSBBdWRpb0NoYW5uZWwgYnVmZmVyLiBUaGlzIHBhdGNoIGxldmVyYWdlcyBn c3RfYnVmZmVyX2V4dHJhY3QoKSB0bworICAgICAgICBzaW1wbGlmeSB0aGUgY29kZSBhcyBpdCBk b2VzIGV4YWN0bHkgd2hhdCB3ZSBuZWVkOiBnc3RfYnVmZmVyX21hcCwgbWVtY3B5LAorICAgICAg ICBnc3RfYnVmZmVyX3VubWFwIGFuZCBlcnJvciBoYW5kbGluZy4KKworICAgICAgICBBbHNvIHJl cGxhY2UgR3N0QnVmZmVyIE5VTEwgY2hlY2sgYnkgYW4gYXNzZXJ0aW9uIGFzIHdlIGFscmVhZHkg bWFrZSBzdXJlIHRoZXkKKyAgICAgICAgYXJlIG5vdCBOVUxMIGJlZm9yZSBhZGRpbmcgdGhlbSB0 byB0aGUgbGlzdC4gQWRkaXRpb25hbGx5LCB3ZSBub3cgY2FsbAorICAgICAgICBhdWRpb0NoYW5u ZWwtPm11dGFibGVEYXRhKCkgb25seSBvbmNlIGluc3RlYWQgb2Ygb25jZSBwZXIgaXRlcmF0aW9u LiBJdCBpcworICAgICAgICBhIGJpdCBiZXR0ZXIgYXMgdGhlIGdldHRlciBkb2VzIHNvbWUgd29y ayBpbnRlcm5hbGx5LgorCisgICAgICAgIE5vIG5ldyB0ZXN0cywgbm8gYmVoYXZpb3IgY2hhbmdl LgorCisgICAgICAgICogcGxhdGZvcm0vYXVkaW8vZ3N0cmVhbWVyL0F1ZGlvRmlsZVJlYWRlckdT dHJlYW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpjb3B5R3N0cmVhbWVyQnVmZmVyc1RvQXVk aW9DaGFubmVsKToKKwogMjAxMy0wNC0wMyAgQ3NhYmEgT3N6dHJvZ29uw6FjICA8b3NzeUB3ZWJr aXQub3JnPgogCiAgICAgICAgIFVucmV2aWV3ZWQgMzJiaXQgYnVpbGRmaXggYWZ0ZXIgcjE0NzU0 Mi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2F1ZGlvL2dzdHJlYW1lci9B dWRpb0ZpbGVSZWFkZXJHU3RyZWFtZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vYXVk aW8vZ3N0cmVhbWVyL0F1ZGlvRmlsZVJlYWRlckdTdHJlYW1lci5jcHAKaW5kZXggOGVmYjZmYy4u NmZiYjFmZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vYXVkaW8vZ3N0cmVh bWVyL0F1ZGlvRmlsZVJlYWRlckdTdHJlYW1lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxh dGZvcm0vYXVkaW8vZ3N0cmVhbWVyL0F1ZGlvRmlsZVJlYWRlckdTdHJlYW1lci5jcHAKQEAgLTEw MCwxNiArMTAwLDE0IEBAIHByaXZhdGU6CiBzdGF0aWMgdm9pZCBjb3B5R3N0cmVhbWVyQnVmZmVy c1RvQXVkaW9DaGFubmVsKEdzdEJ1ZmZlckxpc3QqIGJ1ZmZlcnMsIEF1ZGlvQ2hhbm5lbCogYXVk aW9DaGFubmVsKQogewogI2lmZGVmIEdTVF9BUElfVkVSU0lPTl8xCi0gICAgZ3NpemUgb2Zmc2V0 ID0gMDsKLSAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgZ3N0X2J1ZmZlcl9saXN0X2xlbmd0 aChidWZmZXJzKTsgaSsrKSB7CisgICAgZmxvYXQqIGRlc3RpbmF0aW9uID0gYXVkaW9DaGFubmVs LT5tdXRhYmxlRGF0YSgpOworICAgIHVuc2lnbmVkIGJ1ZmZlckNvdW50ID0gZ3N0X2J1ZmZlcl9s aXN0X2xlbmd0aChidWZmZXJzKTsKKyAgICBmb3IgKHVuc2lnbmVkIGkgPSAwOyBpIDwgYnVmZmVy Q291bnQ7ICsraSkgewogICAgICAgICBHc3RCdWZmZXIqIGJ1ZmZlciA9IGdzdF9idWZmZXJfbGlz dF9nZXQoYnVmZmVycywgaSk7Ci0gICAgICAgIGlmICghYnVmZmVyKQotICAgICAgICAgICAgY29u dGludWU7Ci0gICAgICAgIEdzdE1hcEluZm8gaW5mbzsKLSAgICAgICAgZ3N0X2J1ZmZlcl9tYXAo YnVmZmVyLCAmaW5mbywgR1NUX01BUF9SRUFEKTsKLSAgICAgICAgbWVtY3B5KGF1ZGlvQ2hhbm5l bC0+bXV0YWJsZURhdGEoKSArIG9mZnNldCwgcmVpbnRlcnByZXRfY2FzdDxmbG9hdCo+KGluZm8u ZGF0YSksIGluZm8uc2l6ZSk7Ci0gICAgICAgIG9mZnNldCArPSBpbmZvLnNpemUgLyBzaXplb2Yo ZmxvYXQpOwotICAgICAgICBnc3RfYnVmZmVyX3VubWFwKGJ1ZmZlciwgJmluZm8pOworICAgICAg ICBBU1NFUlQoYnVmZmVyKTsKKyAgICAgICAgZ3NpemUgYnVmZmVyU2l6ZSA9IGdzdF9idWZmZXJf Z2V0X3NpemUoYnVmZmVyKTsKKyAgICAgICAgZ3N0X2J1ZmZlcl9leHRyYWN0KGJ1ZmZlciwgMCwg ZGVzdGluYXRpb24sIGJ1ZmZlclNpemUpOworICAgICAgICBkZXN0aW5hdGlvbiArPSBidWZmZXJT aXplIC8gc2l6ZW9mKGZsb2F0KTsKICAgICB9CiAjZWxzZQogICAgIEdzdEJ1ZmZlckxpc3RJdGVy YXRvciogaXRlciA9IGdzdF9idWZmZXJfbGlzdF9pdGVyYXRlKGJ1ZmZlcnMpOwo=