After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 702632 - acceptcaps query can be very very slow
acceptcaps query can be very very slow
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 1.1.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-06-19 10:21 UTC by Sjoerd Simons
Modified: 2013-07-10 06:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (1.35 KB, patch)
2013-06-19 10:21 UTC, Sjoerd Simons
reviewed Details | Review
proposed patch (1.16 KB, patch)
2013-06-19 10:32 UTC, Sjoerd Simons
reviewed Details | Review

Description Sjoerd Simons 2013-06-19 10:21:59 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
Comment 1 Sebastian Dröge (slomo) 2013-06-19 10:28:43 UTC
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 :)
Comment 2 Sjoerd Simons 2013-06-19 10:32:36 UTC
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
Comment 3 Sebastian Dröge (slomo) 2013-06-19 10:38:32 UTC
Also looks good to me, if nobody has objections please push :)
Comment 4 Sjoerd Simons 2013-06-19 13:13:17 UTC
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
Comment 5 Tim-Philipp Müller 2013-06-23 12:21:36 UTC
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
Comment 6 Sebastian Dröge (slomo) 2013-07-08 12:12:49 UTC
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
Comment 7 Sebastian Dröge (slomo) 2013-07-08 12:52:33 UTC
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.
Comment 8 Olivier Crête 2013-07-09 21:45:41 UTC
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
Comment 9 Sebastian Dröge (slomo) 2013-07-10 06:49:30 UTC
Ok, so everything should be happy again :)