GNOME Bugzilla – Bug 702632
acceptcaps query can be very very slow
Last modified: 2013-07-10 06:49:30 UTC
Created attachment 247248 [details] [review] Proposed patch I've got a logitech C910 camera, which supports quite a few different resolutions. Apparently this is a great way of trigger caps explosion corner cases :) On my system a simple test program which just creates a camerabin, using wrappercamerabinsrc takes more then 50 seconds before it starts sreally treaming frames: $ GST_DEBUG='*v4l2src*:4' gst-uninstalled ./test 2>&1 | grep gst_v4l2src_fill 0:00:23.129465038 10324 0xaae990 INFO v4l2src gstv4l2src.c:861:gst_v4l2src_fill:<v4l> sync to 0:00:00.500000000 out ts 0:00:20.160806192 0:00:52.687093166 10324 0xaae990 INFO v4l2src gstv4l2src.c:861:gst_v4l2src_fill:<v4l> sync to 0:00:01.000000000 out ts 0:00:50.355114373 0:00:52.691789600 10324 0xaae990 INFO v4l2src gstv4l2src.c:861:gst_v4l2src_fill:<v4l> sync to 0:00:01.500000000 out ts 0:00:50.728432813 Almost all the time is taking up by intersections triggers by caps queries in acceptcaps of basetransform.. With the attached patch the time before the second frame starts is reduces to a bit more then 15 seconds: 0:00:14.980203785 13227 0x26ca990 INFO v4l2src gstv4l2src.c:861:gst_v4l2src_fill:<v4l> sync to 0:00:00.500000000 out ts 0:00:12.103921631 0:00:15.980073337 13227 0x26ca990 INFO v4l2src gstv4l2src.c:861:gst_v4l2src_fill:<v4l> sync to 0:00:01.000000000 out ts 0:00:13.104369148
I think this is fine, but we should add the same to gstpad.c for the default acceptcaps handling and also (ideally) everywhere else where custom acceptcaps handling is implemented. Did you profile what the remaining 15 seconds are wasted with? Still sounds a bit too high :)
Created attachment 247249 [details] [review] proposed patch same patch for gstpad.. The only other accept_caps implementation in core is capsfilter, which is optimized already
Also looks good to me, if nobody has objections please push :)
commit 1815e6067a1077ddb92330b25f172da82f38a3e8 Author: Sjoerd Simons <sjoerd.simons@collabora.co.uk> Date: Wed Jun 19 12:30:47 2013 +0200 pad: Add a filter to the caps_query done by acceptcaps Use the caps that the pad is asked to accept as filter for the query https://bugzilla.gnome.org/show_bug.cgi?id=702632 commit 12a72d2b086d3052afa41774db62821039bafcad Author: Sjoerd Simons <sjoerd.simons@collabora.co.uk> Date: Wed Jun 19 12:19:02 2013 +0200 basetransform: optimize default acceptcaps implementation Pass the fixed caps we're asked to accept as a filter for the caps query, so we don't get a fully-expanded set of caps back (which we don't need and can take a lot of time for intersection). This reduces the time for camerabin to produce a second frame on a logitech C910 camera from around 52 seconds to a bit less then 16 seconds on my system. https://bugzilla.gnome.org/show_bug.cgi?id=702632
This change: commit 1815e6067a1077ddb92330b25f172da82f38a3e8 Author: Sjoerd Simons <sjoerd.simons@collabora.co.uk> Date: Wed Jun 19 12:30:47 2013 +0200 pad: Add a filter to the caps_query done by acceptcaps appears to break at least these unit tests in gst-plugins-good (reverting it makes them pass again): Running suite(s): rtp_data_test Error from element appsrc1: Internal data flow error. gstbasesrc.c(2812): gst_base_src_loop (): /GstPipeline:rtph263ppay-identity-pipeline/GstAppSrc:appsrc1: streaming task paused, reason not-negotiated (-4) Error from element appsrc2: Internal data flow error. gstbasesrc.c(2812): gst_base_src_loop (): /GstPipeline:rtph263ppay-identity-pipeline/GstAppSrc:appsrc2: streaming task paused, reason not-negotiated (-4) 100%: Checks: 29, Failures: 0, Errors: 0 PASS: elements/rtp-payloading [well, should really say FAIL here] Running suite(s): rtpmux 0%: Checks: 3, Failures: 3, Errors: 0 elements/rtpmux.c:187:F:rtpmux_basic:test_rtpmux_basic:0: Assertion 'gst_pad_push (src1, inbuf) == GST_FLOW_OK' failed elements/rtpmux.c:187:F:rtpdtmfmux_basic:test_rtpdtmfmux_basic:0: Assertion 'gst_pad_push (src1, inbuf) == GST_FLOW_OK' failed elements/rtpmux.c:187:F:rtpdtmfmux_lock:test_rtpdtmfmux_lock:0: Assertion 'gst_pad_push (src1, inbuf) == GST_FLOW_OK' failed FAIL: elements/rtpmux
commit 0cc77d8e305484d310e7f7df8cc4bec753d34a4d Author: Sebastian Dröge <slomo@circular-chaos.org> Date: Mon Jul 8 14:09:37 2013 +0200 rtph263ppay: Don't pass upstream filter caps to downstream Downstream usually can't accept video/x-h263 but only application/x-rtp, so we would always get an empty intersection here. https://bugzilla.gnome.org/show_bug.cgi?id=702632
The rtpmux one looks like a bug in the rtpmux CAPS query handler... this code looks completely wrong. 1) The condition in clear_caps() does not make any sense at all, it should probably be (strcmp (name, "clock-rate") == 0 || (!only_clock_rate && strcmp (name, "ssrc") == 0)) 2) Querying the sinkpad caps, causes a downstream caps query (ok) and an upstream caps query for every sinkpad (not ok) It's not clear to me when and where ssrc and clock-rate should be ignored and where not. However it is necessary to not pass any of these fields as filter caps to any CAPS query if they should be ignored. The one in same_clock_rate_fold() looks like a good candidate where no filter caps (or cleared filter caps) should be passed to the query.
Fixed the rtpmux unit tests commit 1997acc8b2f3536ed1b0006dc9ffcbb1d1b98325 Author: Olivier Crête <olivier.crete@collabora.com> Date: Tue Jul 9 17:42:59 2013 -0400 rtpmux: Keep caps order from the peer or the filter
Ok, so everything should be happy again :)