99852 2012-10-19 10:16:17 -0700 Gstreamer 1.0 not working 2012-10-22 09:44:42 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 nicolas webkit-unassigned eric.carlson feature-media-reviews gustavo menard mrobinson pnormand webkit.review.bot oldest_to_newest 746411 0 nicolas 2012-10-19 10:16:17 -0700 Gstreamer 1.0 not working 746416 1 169644 nicolas 2012-10-19 10:22:40 -0700 Created attachment 169644 Patch 746422 2 169644 menard 2012-10-19 10:27:54 -0700 Comment on attachment 169644 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169644&action=review > Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:-91 > - ASSERT(!ptr || !gstObjectIsFloating(GST_OBJECT(ptr))); You should not get rid of the entire assert no? Testing pointer nullity is still valid to me. 746435 3 169644 pnormand 2012-10-19 10:42:15 -0700 Comment on attachment 169644 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=169644&action=review >> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:-91 >> - ASSERT(!ptr || !gstObjectIsFloating(GST_OBJECT(ptr))); > > You should not get rid of the entire assert no? Testing pointer nullity is still valid to me. +1 > Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.cpp:-46 > - GstCaps* caps = gst_pad_get_current_caps(pad); > - if (!caps) > - caps = gst_pad_query_caps(pad, 0); > - return adoptGRef(caps); // gst_pad_query_caps and gst_pad_get_current_caps return a new reference. gst_pad_get_current_caps() can return NULL AFAIK. What should we do in that case instead of querying the caps to the pad? 746461 4 nicolas 2012-10-19 11:12:33 -0700 (In reply to comment #3) > (From update of attachment 169644 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=169644&action=review > > >> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:-91 > >> - ASSERT(!ptr || !gstObjectIsFloating(GST_OBJECT(ptr))); > > > > You should not get rid of the entire assert no? Testing pointer nullity is still valid to me. > > +1 ok. > > > Source/WebCore/platform/graphics/gstreamer/GStreamerVersioning.cpp:-46 > > - GstCaps* caps = gst_pad_get_current_caps(pad); > > - if (!caps) > > - caps = gst_pad_query_caps(pad, 0); > > - return adoptGRef(caps); // gst_pad_query_caps and gst_pad_get_current_caps return a new reference. > > gst_pad_get_current_caps() can return NULL AFAIK. What should we do in that case instead of querying the caps to the pad? Nothing, 0.10 GST_PAD_CAPS() macro you can also return NULL and this is handled gracefully by callers (by not painting). 746467 5 nicolas 2012-10-19 11:16:10 -0700 (In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 169644 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=169644&action=review > > > > >> Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp:-91 > > >> - ASSERT(!ptr || !gstObjectIsFloating(GST_OBJECT(ptr))); > > > > > > You should not get rid of the entire assert no? Testing pointer nullity is still valid to me. > > > > +1 > > ok. > Sorry I answered too fast. No the !ptr check is not valid, this is a OR. It is there only to prevent calling GST_OBJECT() with a NULL pointer. It's totally fine to store NULL in the GRefPtr. 746520 6 pnormand 2012-10-19 12:16:07 -0700 Thank you Nicolas! Hopefully we'll turn gst-1.0 build support soon in the build-webkit. 747239 7 pnormand 2012-10-21 23:08:14 -0700 If you're not committer yet please set the cq? flag so the commit-queue takes care of it. 747599 8 nicolas 2012-10-22 09:19:26 -0700 (In reply to comment #7) > If you're not committer yet please set the cq? flag so the commit-queue takes care of it. Sorry I forgot. Done. 747601 9 169644 webkit.review.bot 2012-10-22 09:22:27 -0700 Comment on attachment 169644 Patch Rejecting attachment 169644 from commit-queue. nicolas.dufresne@collabora.co.uk does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. 747607 10 nicolas 2012-10-22 09:28:30 -0700 (In reply to comment #9) > (From update of attachment 169644 [details]) > Rejecting attachment 169644 [details] from commit-queue. > > nicolas.dufresne@collabora.co.uk does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. > > - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. > > - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. Sorry again, selected the wrong option, hopefully setting it to ? now is enough. 747608 11 169644 pnormand 2012-10-22 09:28:45 -0700 Comment on attachment 169644 Patch I did set it to cq+ ... 747628 12 169644 webkit.review.bot 2012-10-22 09:44:38 -0700 Comment on attachment 169644 Patch Clearing flags on attachment: 169644 Committed r132081: <http://trac.webkit.org/changeset/132081> 747629 13 webkit.review.bot 2012-10-22 09:44:42 -0700 All reviewed patches have been landed. Closing bug. 169644 2012-10-19 10:22:40 -0700 2012-10-22 09:44:38 -0700 Patch bug-99852-20121019132123.patch text/plain 3569 nicolas U3VidmVyc2lvbiBSZXZpc2lvbjogMTMxNjA5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggOGYzZDZiZWVlYWE3ODAw OGNhYjNmNDQwN2IwOGZiY2MxMGUyYzJjMy4uOGMzYTliM2Y5NTMzMTEzZjMxOWQ2YmNmMWNlY2Yx NDQxN2E1NjJmMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI0IEBACisyMDEyLTEwLTE5ICBOaWNv bGFzIER1ZnJlc25lIDxuaWNvbGFzLmR1ZnJlc25lQGNvbGxhYm9yYS5jb20+CisKKyAgICAgICAg R3N0cmVhbWVyIDEuMCBub3Qgd29ya2luZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9OTk4NTIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBUaGVyZSB3YXMgYSBzZXJpZXMgb2YgdHJpdmlhbCBpc3N1ZSwgZ19v YmplY3RfaXNfZmxvYXRpbmcoKSB3YXMgY2FsbGVkCisgICAgICAgIG9uIHR5cGUgR3N0Q2FwcyAo d2hpY2ggaXMgbm90IGEgR09iamVjdCksIHdlYmtpdEdzdEdldFBhZENhcHMoKSB3YXMKKyAgICAg ICAgcmV0dXJuaW5nIG5vbi1maXhlZCBjYXBzIGFuZCBHU1RfTUVTU0FHRV9EVVJBVElPTiBoYXMg YmVlbiByZW5hbWVkCisgICAgICAgIHRvIEdTVF9NRVNTQUdFX0RVUkFUSU9OX0NIQU5HRUQuCisK KyAgICAgICAgTW9zdCBmYWlsaW5nIHRlc3QgcGFzc2VzIG5vdy4KKworICAgICAgICAqIHBsYXRm b3JtL2dyYXBoaWNzL2dzdHJlYW1lci9HUmVmUHRyR1N0cmVhbWVyLmNwcDoKKyAgICAgICAgKFdU Rjo6YWRvcHRHUmVmKToKKyAgICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvR1N0 cmVhbWVyVmVyc2lvbmluZy5jcHA6CisgICAgICAgICh3ZWJraXRHc3RHZXRQYWRDYXBzKToKKyAg ICAgICAgKiBwbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvTWVkaWFQbGF5ZXJQcml2YXRlR1N0 cmVhbWVyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Ok1lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1l cjo6aGFuZGxlTWVzc2FnZSk6CisKIDIwMTItMTAtMTcgIFZzZXZvbG9kIFZsYXNvdiAgPHZzZXZp a0BjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogW1JlZ3Jlc3Npb25dIFVu ZG9pbmcgc2NyaXB0IGNoYW5nZXMgZG9lcyBub3QgcmVzdG9yZSBicmVha3BvaW50cy4KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9HUmVmUHRy R1N0cmVhbWVyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1l ci9HUmVmUHRyR1N0cmVhbWVyLmNwcAppbmRleCBkZWViMzM5ZDJmYmVkMmI0NTRlZTJkNjgyYzNm ZDFhYjBlOTExZTM0Li5mZmIwZTE2NzNkM2ZmMjRhM2MxYjQ3YjYwOTliY2RhNDYyNmJlNzJkIDEw MDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIvR1Jl ZlB0ckdTdHJlYW1lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mv Z3N0cmVhbWVyL0dSZWZQdHJHU3RyZWFtZXIuY3BwCkBAIC04OCw3ICs4OCw2IEBAIHRlbXBsYXRl IDw+IHZvaWQgZGVyZWZHUHRyPEdzdFBhZFRlbXBsYXRlPihHc3RQYWRUZW1wbGF0ZSogcHRyKQog CiB0ZW1wbGF0ZSA8PiBHUmVmUHRyPEdzdENhcHM+IGFkb3B0R1JlZihHc3RDYXBzKiBwdHIpCiB7 Ci0gICAgQVNTRVJUKCFwdHIgfHwgIWdzdE9iamVjdElzRmxvYXRpbmcoR1NUX09CSkVDVChwdHIp KSk7CiAgICAgcmV0dXJuIEdSZWZQdHI8R3N0Q2Fwcz4ocHRyLCBHUmVmUHRyQWRvcHQpOwogfQog CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFtZXIv R1N0cmVhbWVyVmVyc2lvbmluZy5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGlj cy9nc3RyZWFtZXIvR1N0cmVhbWVyVmVyc2lvbmluZy5jcHAKaW5kZXggNjM4ZjQ4MGZmZWE5YTAy MmZlN2NlZTVmMWRkNzJhYWYwZDcwOWU1Yi4uNzE1OGNhZjYyMTU3NTMwYmE3ZTc3Y2Y5OTI3NjQ0 M2E0YWE1YmMwYyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3Mv Z3N0cmVhbWVyL0dTdHJlYW1lclZlcnNpb25pbmcuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3Bs YXRmb3JtL2dyYXBoaWNzL2dzdHJlYW1lci9HU3RyZWFtZXJWZXJzaW9uaW5nLmNwcApAQCAtNDAs MTAgKzQwLDcgQEAgR1JlZlB0cjxHc3RDYXBzPiB3ZWJraXRHc3RHZXRQYWRDYXBzKEdzdFBhZCog cGFkKQogICAgICAgICByZXR1cm4gMDsKIAogI2lmZGVmIEdTVF9BUElfVkVSU0lPTl8xCi0gICAg R3N0Q2FwcyogY2FwcyA9IGdzdF9wYWRfZ2V0X2N1cnJlbnRfY2FwcyhwYWQpOwotICAgIGlmICgh Y2FwcykKLSAgICAgICAgY2FwcyA9IGdzdF9wYWRfcXVlcnlfY2FwcyhwYWQsIDApOwotICAgIHJl dHVybiBhZG9wdEdSZWYoY2Fwcyk7IC8vIGdzdF9wYWRfcXVlcnlfY2FwcyBhbmQgZ3N0X3BhZF9n ZXRfY3VycmVudF9jYXBzIHJldHVybiBhIG5ldyByZWZlcmVuY2UuCisgICAgcmV0dXJuIGFkb3B0 R1JlZihnc3RfcGFkX2dldF9jdXJyZW50X2NhcHMocGFkKSk7IC8vIGdzdF9wYWRfZ2V0X2N1cnJl bnRfY2FwcyByZXR1cm4gYSBuZXcgcmVmZXJlbmNlLgogI2Vsc2UKICAgICByZXR1cm4gR1NUX1BB RF9DQVBTKHBhZCk7CiAjZW5kaWYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2dyYXBoaWNzL2dzdHJlYW1lci9NZWRpYVBsYXllclByaXZhdGVHU3RyZWFtZXIuY3BwIGIvU291 cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0 ZUdTdHJlYW1lci5jcHAKaW5kZXggYTRkNDc0NTFkOWZhYmQ4MTFlYzc0NGZlZGM3M2Y5MWNhOWVm NmE0OC4uZjJiZDI5MjU5MDJiZDFkZjgzZGNkNmEzNjVjYmI3MDRhMTQwNzMyNSAxMDA2NDQKLS0t IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL01lZGlhUGxheWVy UHJpdmF0ZUdTdHJlYW1lci5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhp Y3MvZ3N0cmVhbWVyL01lZGlhUGxheWVyUHJpdmF0ZUdTdHJlYW1lci5jcHAKQEAgLTg1Nyw3ICs4 NTcsMTEgQEAgZ2Jvb2xlYW4gTWVkaWFQbGF5ZXJQcml2YXRlR1N0cmVhbWVyOjpoYW5kbGVNZXNz YWdlKEdzdE1lc3NhZ2UqIG1lc3NhZ2UpCiAgICAgY2FzZSBHU1RfTUVTU0FHRV9CVUZGRVJJTkc6 CiAgICAgICAgIHByb2Nlc3NCdWZmZXJpbmdTdGF0cyhtZXNzYWdlKTsKICAgICAgICAgYnJlYWs7 CisjaWZkZWYgR1NUX0FQSV9WRVJTSU9OXzEKKyAgICBjYXNlIEdTVF9NRVNTQUdFX0RVUkFUSU9O X0NIQU5HRUQ6CisjZWxzZQogICAgIGNhc2UgR1NUX01FU1NBR0VfRFVSQVRJT046CisjZW5kaWYK ICAgICAgICAgTE9HX01FRElBX01FU1NBR0UoIkR1cmF0aW9uIGNoYW5nZWQiKTsKICAgICAgICAg ZHVyYXRpb25DaGFuZ2VkKCk7CiAgICAgICAgIGJyZWFrOwo=