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 701996 - context: Still inconvenient to use and racy
context: Still inconvenient to use and racy
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Urgent blocker
: 1.1.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
playback
Depends on:
Blocks:
 
 
Reported: 2013-06-11 10:14 UTC by Sebastian Dröge (slomo)
Modified: 2013-09-18 21:09 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastian Dröge (slomo) 2013-06-11 10:14:12 UTC
Right now GstContext depends on a combination of queries, events, messages, propagation of information in bins and application interaction to work.

This is unfortunately a bit racy still, as e.g. if two elements request a context at the same time, it could happen that nothing gets them the context and they both create their own. To mitigate this, it is required by the application to handle the have-context messages in a sync bus handler (but can't set the context there on the bin because that can lead to deadlocks). So also need to do that from the async bus handler. But this is not solving the race at all.


Also the requirement that the application does stuff is not ideal. Many applications just don't care, the elements should figure out automatically what to do. So ideally the bins would already catch the have-context and need-context messages and propagate things upwards and downwards as required. However this is easily deadlock-prone again. And automatic handling of these messages inside the bin doesn't allow the application to have complete control over the contexts (if it wants to do so), e.g. having a different device context for two parts of a pipeline.


So to summarize:

- context handling is still racy if two elements want/create a context at the same time
- applications often don't care about contexts, but have to handle them in a generic way... deferring that to the bins removes control from the application (if it wants to), so would need a way to override and intercept this (from the application, inside bins you have GstBin::handle_message and GstElement::post_message).
Comment 1 Nicolas Dufresne (ndufresne) 2013-08-08 14:34:25 UTC
A proposition to get the application out of the way most of the time would be to:

a) make the event sticky. This will let playbing element to dispatch their newly created context and expect it to be distributed whenever element are linked.

b) Add handler for the have-context message and redispatch that context to any existing element. Element htat already have a type of context set, would simply ignore the newly dispatched context. This is to enable having multiple context within a single pipeline. As this is a special case, it will require special action by the application.

This is just the big lines, I'll be digging into the code soon to fine more details that might be required to get this running.
Comment 2 Sebastian Dröge (slomo) 2013-08-12 13:28:27 UTC
c) remove the context storing inside GstElement and gst_element_get_context() and let elements just worry about that

d) let bins automatically distribute contexts from have-context of one of their childs to all other childs and accumulate the contexts internally


Nicolas and I talked about this at GUADEC and this all seems like it would improve the situation a lot, and especially not require any application code to handle contexts for the normal playbin use case.
Comment 3 Sebastian Dröge (slomo) 2013-08-22 10:47:54 UTC
How do we make sure that a bin is not storing and distributing an obsolete context?
Comment 4 Nicolas Dufresne (ndufresne) 2013-08-26 23:01:44 UTC
I thought bin accumulation of context is just like what a seek (for seek on ready) would be. Which means it would keep it around as long as the bin state did not reach paused state, after that it would drop it, since the pipeline can exchange the context by itself at this point.

Does that make sense ? I might be missing something.
Comment 5 Sebastian Dröge (slomo) 2013-08-27 08:32:38 UTC
(In reply to comment #4)
> I thought bin accumulation of context is just like what a seek (for seek on
> ready) would be. Which means it would keep it around as long as the bin state
> did not reach paused state, after that it would drop it, since the pipeline can
> exchange the context by itself at this point.
> 
> Does that make sense ? I might be missing something.

This wouldn't propagate the contexts to newly added elements for example, unless they accidentally happen to get it via the sticky event. We would need to keep it around in the bins forever until it goes back to NULL or something like that. Maybe we need a context-lost message similar to clock-lost?
Comment 6 Sebastian Dröge (slomo) 2013-09-13 09:28:41 UTC
Also not sure about the sticky event, we could also make the query upstream/downstream instead. That way inside the pipeline everything could be pull-based by queries instead of sometimes pull (query) and sometimes push (event).

Instead of always setting all contexts on child elements of a bin, we could also make this pull-based. An element asks for a context and the highest-level bin (or app) that has such a context will then set it on that element only.

And bins would cache contexts that are posted on the bus by child elements, or that were set on them by the application. The ones set by the application should always be preferred and kept separate, the ones from the pipeline are fallback and would be cleared when going to NULL. It is possible by the bins to keep these separate because a need-context message will only cause the context to be set on the sender of the message, not on any higher-level bin.

Only open problems with that are:
- setting application contexts on specific elements, how to distinguish between these and ones set by a bin? Thread local storage usage while the need-context message is sent?
- bin subclasses that want contexts set on them, how to distinguish there between application and pipeline internal contexts? Basically the same problem.


Maybe setting a context as a result to the message should be done different than setting an application context? Unfortunately we can't just add them to the message as it will likely not be writable.
Comment 7 Sebastian Dröge (slomo) 2013-09-17 15:42:09 UTC
Working on it btw, I have a good solution now...
Comment 8 Sebastian Dröge (slomo) 2013-09-17 19:38:09 UTC
http://cgit.freedesktop.org/~slomo/gstreamer/log/?h=context
Comment 9 Nicolas Dufresne (ndufresne) 2013-09-18 15:09:33 UTC
I like this one, simplier and more intuitive. It also cover all the use case we had with the implementation we had in 0.10 with the addition of caching at the bin level.
Comment 10 Sebastian Dröge (slomo) 2013-09-18 15:28:29 UTC
I'll merge it later then after fixing eglglessink and extending the unit test, and then release 1.1.90. Thanks for the review Nicolas :)
Comment 11 Sebastian Dröge (slomo) 2013-09-18 21:09:36 UTC
All pushed now.