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 728268 - Need a way to detect that pool haven't changed, is active, and does not need to be renegotiated
Need a way to detect that pool haven't changed, is active, and does not need ...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other Linux
: Normal normal
: 1.3.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 727765 (view as bug list)
Depends on:
Blocks: 727916
 
 
Reported: 2014-04-15 14:39 UTC by Nicolas Dufresne (ndufresne)
Modified: 2014-07-25 17:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] value: Add support for GObject comparising in structures (1.51 KB, patch)
2014-05-06 23:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
[PATCH] test: Comparision of GObject (by pointer) (1.16 KB, patch)
2014-05-06 23:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
[PATCH] value: Add support for GstAllocationParams comparision (1.73 KB, patch)
2014-05-06 23:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
[PATCH] test: Comparision of GAllocationParams (1.45 KB, patch)
2014-05-06 23:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
[PATCH] bufferpool: Add support for reconfiguring a pool (2.52 KB, patch)
2014-05-06 23:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
[PATCH] test: Test buffer pool activation and configuration (3.25 KB, patch)
2014-05-06 23:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2014-04-15 14:39:11 UTC
Currently base classes blindly call decide_allocation() whenever a reconfigure event is triggered. It assumed that pools found in the query and selected by the element are not active. In most cases, the second query endup with the same pool (which is active, hence configuration cannot be changed) and settings are still compatible. 

This issue can be triggered with pipelines like this one (VideoDecoder):
GST_DEBUG=3 gst-launch-1.0 videotestsrc ! jpegenc ! decodebin ! xvimagesink

Trace:
bufferpool gstbufferpool.c:632:gst_buffer_pool_set_config:<xvimagebufferpool0> can't change config, we are active

And this one (BaseSrc):
bufferpool gstbufferpool.c:632:gst_buffer_pool_set_config:<xvimagebufferpool0> can't change config, we are active

I'm not fully certain what is right and what is wrong in current implementation. It seems that most active pool detection is required, that some helper to detect "compatible" config would be nice.

I also have the impression that setting the config should be done once, most likely by the base class, but that I'm not so clear, since there is no way to tell the base class we want to enabled certain options.

Finally, for decoders, it's not always possible to switch pool right away. Buffers from the pool could be used as the decoder observation, so releasing them at any moment could break the image. Instead, the renegotiation should be handled asynchronously. Waiting for a sync point, draining, stopping the pool, and then operating the allocation query. We need a way to tell the base class about that, and implement an asynchronous mechanism for it.

This not exactly a plan, so input would be appreciated.
Comment 1 Nicolas Dufresne (ndufresne) 2014-04-15 15:31:55 UTC
*** Bug 727765 has been marked as a duplicate of this bug. ***
Comment 2 Nicolas Dufresne (ndufresne) 2014-05-06 23:45:27 UTC
Created attachment 276031 [details] [review]
[PATCH] value: Add support for GObject comparising in structures


This is useful to allow comparing pool configuration where a GstAllocator
is set.

https://bugzilla.gnome.org/show_bug.cgi?id=728268
---
 gst/gstvalue.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
Comment 3 Nicolas Dufresne (ndufresne) 2014-05-06 23:45:32 UTC
Created attachment 276032 [details] [review]
[PATCH] test: Comparision of GObject (by pointer)


https://bugzilla.gnome.org/show_bug.cgi?id=728268
---
 tests/check/gst/gstvalue.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
Comment 4 Nicolas Dufresne (ndufresne) 2014-05-06 23:45:37 UTC
Created attachment 276033 [details] [review]
[PATCH] value: Add support for GstAllocationParams comparision


This is useful to compare buffer pool configuaration.

https://bugzilla.gnome.org/show_bug.cgi?id=728268
---
 gst/gstvalue.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
Comment 5 Nicolas Dufresne (ndufresne) 2014-05-06 23:45:47 UTC
Created attachment 276034 [details] [review]
[PATCH] test: Comparision of GAllocationParams


https://bugzilla.gnome.org/show_bug.cgi?id=728268
---
 tests/check/gst/gstvalue.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
Comment 6 Nicolas Dufresne (ndufresne) 2014-05-06 23:45:52 UTC
Created attachment 276035 [details] [review]
[PATCH] bufferpool: Add support for reconfiguring a pool


If a pool config is being configured again, check if the configuration have changed.
If not, skip that step. Finally, if the pool is active, try deactivating it.

https://bugzilla.gnome.org/show_bug.cgi?id=728268
---
 gst/gstbufferpool.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)
Comment 7 Nicolas Dufresne (ndufresne) 2014-05-06 23:45:56 UTC
Created attachment 276036 [details] [review]
[PATCH] test: Test buffer pool activation and configuration


https://bugzilla.gnome.org/show_bug.cgi?id=728268
---
 tests/check/gst/gstbufferpool.c | 51 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)
Comment 8 Nicolas Dufresne (ndufresne) 2014-05-15 23:18:50 UTC
I've sqashed the code and their unit tests.

commit 6079666a71ab098151207a0564946e433fa7838b
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Tue May 6 15:35:14 2014 -0400

    bufferpool: Add support for reconfiguring a pool
    
    If a pool config is being configured again, check if the configuration have changed.
    If not, skip that step. Finally, if the pool is active, try deactivating it.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728268

commit 64aa64cb80a404c49bb5d123c06610b9509d6ead
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Tue May 6 16:59:34 2014 -0400

    value: Add support for GstAllocationParams comparision
    
    This is useful to compare buffer pool configuaration.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728268

commit 00614e2c2b3882604d7715908a3b43230a2607aa
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Tue May 6 16:46:55 2014 -0400

    value: Add support for GObject comparising in structures
    
    This is useful to allow comparing pool configuration where a GstAllocator
    is set.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728268
Comment 9 Nicolas Dufresne (ndufresne) 2014-07-25 17:48:45 UTC
Just had one more cleanup left in my branch, related but not major.

commit e196906b993285f3405690f6240f5dcd491ae9ea
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Sat May 24 19:02:59 2014 -0400

    v4l2object: Remove is_active checks
    
    These checks are no longer required with recent change to the bufferpool. This
    should allow changing the configuartion, hence the way forward renegotiation
    support.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728268