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 642373 - [basetransform] Avoid too may pad allocs
[basetransform] Avoid too may pad allocs
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal enhancement
: 0.10.33
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-02-15 13:34 UTC by Thiago Sousa Santos
Modified: 2011-02-22 12:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test application (4.59 KB, text/x-csrc)
2011-02-15 13:36 UTC, Thiago Sousa Santos
  Details
basetransform: Be smarter with pad allocs (5.35 KB, patch)
2011-02-15 13:42 UTC, Thiago Sousa Santos
committed Details | Review

Description Thiago Sousa Santos 2011-02-15 13:34:51 UTC
Everytime basetransform gets a buffer it does a pad-alloc downstream to check for caps renegotiation feedback, this happens even on passthrough mode.

Now, take the following pipeline:

videotestsrc ! ffmpegcolorspace ! videocrop ! videoscale ! ffmpegcolorspace ! fakesink

What happens:
1) Videotestsrc pad-allocs its buffer, and pushes.
2) The first ffmpegcolorspace gets the buffer on its chain, and does a pad-alloc, discards the alloc'ed buffer as it is on passthrough and pushes the original buffer.
3) videocrop does the same
4) videoscale does the same
5) The second ffmpegcolorspace does the same
6) The sink renders the buffer

As you can see, we have too many pad allocs hapenning here. I'm attaching a test application to show this problem.

We can't simply stop basetransform from pad-alloc'ing (even on passthrough) because we probably rely on it for renegotiation scenarios, but we can try to make it smarter and get the "pad-allocs per buffer received" lower on the sink.
Comment 1 Thiago Sousa Santos 2011-02-15 13:36:20 UTC
Created attachment 180891 [details]
test application

Test application that uses a custom sink to count the number of pad-allocs and buffers rendered.
Comment 2 Thiago Sousa Santos 2011-02-15 13:42:19 UTC
Created attachment 180892 [details] [review]
basetransform: Be smarter with pad allocs

Avoid doing unnecessary pad-allocs when on passthrough mode.
If multiple basetransform elements are on a pipeline, they
would do a pad-alloc for each received buffer, each element
would do this, so we would have lots of pad allocs on the
pipeline for a single buffer being pushed through it.

This patch attempts to reduce this amount by avoiding
doing pad-allocs if the element has already done it
after the last pushed buffer. So it will only be allowed
to do a new pad-alloc after it has pushed a buffer, so we get
1x1 pad-alloc and buffer ratio
Comment 3 Sebastian Dröge (slomo) 2011-02-15 14:13:21 UTC
I don't think that behaviour is correct, just consider a case where you have to allocate multiple buffers before pushing the first one
Comment 4 Thiago Sousa Santos 2011-02-15 14:16:06 UTC
How? Can basetransform receive a buffer and don't push one downstream?

If upstream does a pad-alloc basetransform will pass it downstream just as before the patch.


The patch avoids pad-alloc'ing when basetransform gets a buffer on its sinkpad.
Comment 5 Sebastian Dröge (slomo) 2011-02-15 14:21:48 UTC
Right, ignore my comment, sorry :)
Comment 6 Stefan Sauer (gstreamer, gtkdoc dev) 2011-02-17 21:28:25 UTC
Without looking too closely at the patch yet, could be related to bug #584465
Comment 7 Thiago Sousa Santos 2011-02-18 11:37:52 UTC
Yes, it looks related. Though the solutions are different.
Comment 8 Stefan Sauer (gstreamer, gtkdoc dev) 2011-02-18 14:55:12 UTC
Comment on attachment 180892 [details] [review]
basetransform: Be smarter with pad allocs

Patch looks good to me. I don't see any breakage from it (tests and applications).
Comment 9 Stefan Sauer (gstreamer, gtkdoc dev) 2011-02-18 14:57:47 UTC
*** Bug 584465 has been marked as a duplicate of this bug. ***
Comment 10 Thiago Sousa Santos 2011-02-22 12:48:29 UTC
Pushed.

commit 83597767b169dd6c39a07b6144a650c1f098825a
Author: Thiago Santos <thiago.sousa.santos@collabora.co.uk>
Date:   Mon Feb 14 18:05:09 2011 -0300

    basetransform: Be smarter with pad allocs
    
    Avoid doing unnecessary pad-allocs when on passthrough mode.
    If multiple basetransform elements are on a pipeline, they
    would do a pad-alloc for each received buffer, each element
    would do this, so we would have lots of pad allocs on the
    pipeline for a single buffer being pushed through it.
    
    This patch attempts to reduce this amount by avoiding
    doing pad-allocs if the element has already done it
    after the last pushed buffer. So it will only be allowed
    to do a new pad-alloc after it has pushed a buffer, so we get
    1x1 pad-alloc and buffer ratio
    
    https://bugzilla.gnome.org/show_bug.cgi?id=642373