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 754042 - v4l2src: Asserts on renegotiation with USERPTR
v4l2src: Asserts on renegotiation with USERPTR
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.8.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-24 20:17 UTC by Nicolas Dufresne (ndufresne)
Modified: 2016-06-07 21:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v4l2src: Avoid decide allocation on active pool (2.11 KB, patch)
2016-06-07 16:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
v4l2-util: Introduce GstV4l2Error (3.23 KB, patch)
2016-06-07 20:35 UTC, Nicolas Dufresne (ndufresne)
rejected Details | Review
v4l2: Add an error return to _try/_set_format (12.16 KB, patch)
2016-06-07 20:35 UTC, Nicolas Dufresne (ndufresne)
rejected Details | Review

Description Nicolas Dufresne (ndufresne) 2015-08-24 20:17:25 UTC
With memory model USERPTR, renegotiation lead to assertion as the code expect the downstream pool to be deactivated after the pipeline get drained using the allocation query. Arguably, this code should be more robust.

gst-launch-1.0 v4l2src io-mode=userptr ! glimagesink

Result is:
** (gst-launch-1.0:8898): CRITICAL **: gst_v4l2_buffer_pool_set_other_pool: assertion '!gst_buffer_pool_is_active (GST_BUFFER_POOL (pool))' failed


This only occurs with glimagesink and gtkglsink, which sends reconfigure when the display size changes.
Comment 1 Nicolas Dufresne (ndufresne) 2015-08-24 21:24:24 UTC
Ok, basically, we deactivate the internal pool only if the caps have change, that is, to avoid reallocating buffers. But we still execute the decide allocation query, which in userptr and dmabuf-import expects the internal pool to be stopped.

What the assertion endup doing (keeping the old downstream pool) works well of course, and avoid re-allocation. I'm tempted to simply change that into a check/return.

The side effect though, is that if the renegotiation was triggered because the element offering the pool was replaced, we'll keep using this pool and the pipeline might not be optimal. Also, if the buffer requirement have change, it won't be updated either.

Replacing the other_pool is difficult, as the new pool is not yet configured, or activated. Allocating buffer from it will fail (flusing), which fails _qbuf() which unfortunately lead to ->release_buffer(), which finally endup causing an infinit recursive loop.
Comment 2 Nicolas Dufresne (ndufresne) 2016-06-07 16:45:51 UTC
Created attachment 329312 [details] [review]
v4l2src: Avoid decide allocation on active pool

v4l2src will renegotiate only if the format have changed. As of now,
it's not possible to change the allocationw without resetting the
camera. To avoid unwanted side effect, simply keep the old allocation
if no renegotiation is taking place. This fixes assertion and possible
failures in USERPTR or DMABUF import mode (when using downstream pools).
Comment 3 Nicolas Dufresne (ndufresne) 2016-06-07 16:48:01 UTC
Attachment 329312 [details] pushed as ec169a1 - v4l2src: Avoid decide allocation on active pool
Comment 4 Nicolas Dufresne (ndufresne) 2016-06-07 20:35:11 UTC
Created attachment 329335 [details] [review]
v4l2-util: Introduce GstV4l2Error

This is to allow returning an error that can easily be sent as
message to the application if the element needs it. Using this
also allow ignoring errors.

https://bugzilla.gnome.org/show_bug.cgi?id=766172
Comment 5 Nicolas Dufresne (ndufresne) 2016-06-07 20:35:15 UTC
Created attachment 329336 [details] [review]
v4l2: Add an error return to _try/_set_format

This way one can easily ignore errors. Previously, error were always
posted ont he bus.

https://bugzilla.gnome.org/show_bug.cgi?id=766172
Comment 6 Nicolas Dufresne (ndufresne) 2016-06-07 21:21:53 UTC
Branch: 1.8
Commit: 16b0c43ea032ed6cf17337ca69073d9745d341aa