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 745377 - v4l2src: Camera restarts when used with decodebin
v4l2src: Camera restarts when used with decodebin
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-03-01 18:09 UTC by Nicolas Dufresne (ndufresne)
Modified: 2015-03-14 13:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] bufferpool: Don't stop the pool in set_config() (1.59 KB, patch)
2015-03-01 18:17 UTC, Nicolas Dufresne (ndufresne)
reviewed Details | Review

Description Nicolas Dufresne (ndufresne) 2015-03-01 18:09:43 UTC
When using decodebin, v4l2src receives a reconfigure-event. Even though it won't renegotiate, a new allocation query is run. In the decide allocation, v4l2 sets the negotiated configuration on the pool but the pool is active. This configuation is different because the pool do small adjustments.

Currently the pool check for equality of the structure, and try to stop the pool in order to set the new configuration. I think the pool should not try to stop the configuration. Stopping the pool, cause the streaming to stop and start (which takes a bit of time with UVC cameras). In the end, the configuration didn't change, so there was no reason to stop the pool.

I think start/stop the pool should be left to the user of the pool and not implicit. Instead, what the pool could do (and this is according to the doc) is simply return FALSE on the set_config. The caller will then read back the pool configuration (the active one) and check if it's usable.

This way moves back the task of start/stop of the pool the controlling element instead of being implicit.

Pipeline to reproduce:
v4l2src ! image/jpeg ! decodebin ! autovideosink
Comment 1 Nicolas Dufresne (ndufresne) 2015-03-01 18:17:48 UTC
Created attachment 298221 [details] [review]
[PATCH] bufferpool: Don't stop the pool in set_config()


Don't stop the pool in set_config(). Instead, let the controlling
element manage it. Most of the time, when an active pool is being
configured is because the caps didn't change.

https://bugzilla.gnome.org/show_bug.cgi?id=745377
---
 gst/gstbufferpool.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)
Comment 2 Nicolas Dufresne (ndufresne) 2015-03-03 14:05:22 UTC
Comment ?
Comment 3 Sebastian Dröge (slomo) 2015-03-04 10:13:25 UTC
Comment on attachment 298221 [details] [review]
[PATCH] bufferpool: Don't stop the pool in set_config()

This might break assumptions in existing code... but otherwise it makes sense
Comment 4 Nicolas Dufresne (ndufresne) 2015-03-04 13:56:48 UTC
Fortunately, I might have been the only one aware of that new pool behaviour. I really think I over-engineered a way to try and accept a config. What I found later is that even the code I wrote for that may fail, as it didn't expect the pool to stop suddenly (v4l2videodec).
Comment 5 Nicolas Dufresne (ndufresne) 2015-03-14 12:01:39 UTC
Ok, I also update the unit test.

commit c740bad1a003e43c732183f18ed242b602e085a2
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Sun Mar 1 13:15:40 2015 -0500

    bufferpool: Don't stop the pool in set_config()
    
    Don't stop the pool in set_config(). Instead, let the controlling
    element manage it. Most of the time, when an active pool is being
    configured is because the caps didn't change.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=745377