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 784044 - basesrc: Should not re-allocate buffers on EOS
basesrc: Should not re-allocate buffers on EOS
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.8.2
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-21 15:41 UTC by Nicolas Dufresne (ndufresne)
Modified: 2017-06-29 15:08 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Nicolas Dufresne (ndufresne) 2017-06-21 15:41:00 UTC
Currently, when the application send an EOS, we reallocate the buffers by disabling and re-enabling the buffer pool. This is not needed. It also triggers a driver/firmware race for some cameras using v4l2src as the camera endup being turned off and on quickly (see bug #783945 for more details).
Comment 1 Tim-Philipp Müller 2017-06-21 15:50:57 UTC
Did you mean to title this "basesrc:" ?
Comment 2 Nicolas Dufresne (ndufresne) 2017-06-21 17:10:39 UTC
Yes.
Comment 3 Rafael Sierra 2017-06-25 08:02:13 UTC
I'm the reporter of the original bug 783945 

Seeing this new bug has been marked as "Normal" rather than "Enhancement", and it's already assigned, I'd like to ask for a development schedule or road map concerning this bug, if there is any. The fact is I'm under pressure to fix this issue as soon as possible.

Fixing the issue myself is at the moment beyond my abilities/knowledge of GStreamer, but if there is any patch available to test or any specific code modifications to do and verify, I'd be more than pleased to help.

Thank you.
Comment 4 Nicolas Dufresne (ndufresne) 2017-06-25 18:47:56 UTC
I think the classification could be low, but it's a little bit more the an enhancement for sure, I just left the default. As of my experience, it often takes few iterations when fixing basesrc, though to reassure you, I'll hope to propose a first patch in the next two weeks. First iteration is just to replace  gst_buffer_pool_set_active() with gst_buffer_pool_set_flushing(). That you could give try already, it's just about porting to the new API.

Then, I suspect it might cause regression in renegotion code, though I believe we'll be able to fix as soon as we find out why.
Comment 5 Nicolas Dufresne (ndufresne) 2017-06-26 17:21:26 UTC
Did a quick implementation, it speed up a little bit the EOS handling with UVC driver, but it does not workaround the driver race described in bug #783945. Flushing the pool still trigger a flush on the driver side, which is a quick trigger of STREAMOFF/STREAMON, which seems to be triggering the race. This is a terrible bug in the driver/hw imho. That basically means, if you want to work around this, you need to detach streaming state from the pool, that's a lot more work, but likely a good idea.

I'll post of my fix later after some more testing, it's does not matter if another bug is fixed or not by this one.
Comment 6 Nicolas Dufresne (ndufresne) 2017-06-29 15:08:31 UTC
Does not fix the race with cameras, but this specific bug is now fixed.

commit 2be51ba60ce718b6febf5c1bd40ca761c17bfb80
Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Date:   Mon Jun 26 14:09:20 2017 -0400

    basesrc: Don't reallocate buffers when flushing
    
    Instead of using gst_buffer_pool_set_active() when flushing, use
    gst_buffer_pool_set_flushing(), this avoids uneeded reallocation of the
    buffers.

I'll file a bug explaining the race, and how we could solve it.