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 760590 - query: recursive accept-caps query
query: recursive accept-caps query
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 760477
Blocks:
 
 
Reported: 2016-01-13 18:28 UTC by Thiago Sousa Santos
Modified: 2018-11-03 12:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
query: add new recursive-accept-caps (8.49 KB, patch)
2016-01-15 12:49 UTC, Thiago Sousa Santos
none Details | Review
pad: implement a default recursive-accept-caps handler (6.54 KB, patch)
2016-01-15 12:49 UTC, Thiago Sousa Santos
none Details | Review

Description Thiago Sousa Santos 2016-01-13 18:28:45 UTC
+++ This bug was initially created as a clone of Bug #760477 +++

As we are replacing the current misused accept-caps queries we noticed that there is a certain feature gap.

Before we were believing that using an accept-caps query would cause all the pipeline downstream to verify if it would be able to accept a certain caps but the accept-caps was created to be a pre-check of a caps event and only goes to the next element.

Right now we have replaced accept-caps with a caps query + gst_caps_can_intersect() or gst_caps_is_subset() but for some elements we need to do a is_subset() while for others an intersect is enough.

Perhaps we should have a 'recursive' flag to a accept-caps query?

If we decide to do this in 1.x, we keep the current behavior and enable the flag when needed. Every baseclass and lots of elements out there will need to be updated so I'm not sure if this is a backwards compatible change.

Decided to open a bug so we can discuss and, if we defer it to 2.0, we can mark it as so to discuss it again once the time arrives.
Comment 1 Olivier Crête 2016-01-13 18:38:36 UTC
Any reason is not just do a query caps with the desired caps as a filter and check if the return value is empty or not? That should be the equivalent of a recursive accept-caps? I guess we could add some helper functions to for the intersections vs subset case.
Comment 2 Thiago Sousa Santos 2016-01-13 18:41:47 UTC
(In reply to Olivier Crête from comment #1)
> Any reason is not just do a query caps with the desired caps as a filter and
> check if the return value is empty or not? That should be the equivalent of
> a recursive accept-caps? I guess we could add some helper functions to for
> the intersections vs subset case.

Suppose you have an element that does h264 and has in its template caps:

video/x-h264, stream-format=byte-stream, alignment=nal

If you do a caps query with "video/x-h264, stream-format=byte-stream" as a filter it will return 'video/x-h264, stream-format=byte-stream, alignment=nal' but if you send a caps event with the same caps it will fail because of the subset check.
Comment 3 Sebastian Dröge (slomo) 2016-01-13 20:16:05 UTC
The tricky part here is the INTERSECT flag on pads that makes accept-caps either do a subset or a intersection test.
Comment 4 Olivier Crête 2016-01-13 20:30:07 UTC
Shouldn't getcaps have the same behaviour?
Comment 5 Sebastian Dröge (slomo) 2016-01-13 20:39:56 UTC
(which is also why I still think it was a bad idea to add this behaviour, it seems inconsistent)

getcaps is always giving you all possible caps, but then if you intersect with those caps you will have a different behaviour than if checking for a subset. And at this point you can't know if (and which of the) pads downstream are using that flag and which not and can't really reconstruct that anymore.


I would probably add the recursive accept-caps query as a separate, new query and would let it do what the default accept-caps query handling currently is doing.
Comment 6 Thiago Sousa Santos 2016-01-14 12:26:39 UTC
(In reply to Sebastian Dröge (slomo) from comment #5)
> 
> 
> I would probably add the recursive accept-caps query as a separate, new
> query and would let it do what the default accept-caps query handling
> currently is doing.

That seems like a safer idea
Comment 7 Thiago Sousa Santos 2016-01-15 12:49:26 UTC
Created attachment 319101 [details] [review]
query: add new recursive-accept-caps

Adds a new query type that works like accept-caps but it is recursive.
This means that elements will forward the query to its peers when it
is received.

It allows checking whether a particular caps would be accepted by
the whole pipeline branch and not only the next element as a regular
accept-caps query does. This is different than doing a caps query with
a filter or intersecting with the result because elements might require
the presence of certain fields and would need to use gst_caps_is_subset()
instead of an intersection.

Besides that, it should also be faster than doing a caps query that
would have to generate a full list of supported formats while the
recursive accept-caps would only deal with a fixed caps.

Using a synonym of accept would likely create confusion with the
regular accept-caps, better to explicitly use a name that indicates
that it is a variation so people are aware of it.

API: gst_query_new_recursive_accept_caps
API: gst_query_parse_recursive_accept_caps
API: gst_query_parse_recursive_accept_caps_result
API: gst_query_set_recursive_accept_caps_result
API: GST_QUERY_RECURSIVE_ACCEPT_CAPS
Comment 8 Thiago Sousa Santos 2016-01-15 12:49:43 UTC
Created attachment 319102 [details] [review]
pad: implement a default recursive-accept-caps handler

The handler will act similar to the default accept-caps one.

If it is a proxy pad, will proxy the query and, if accepted, then compare
with the pad template. Otherwise does a caps query and uses
gst_caps_is_subset() to check if the caps would be acceptable.

is_subset() is always used in this case because it is more restrictive
than an intersect, even if a pad is marked with ACCEPT_INTERSECT flag
it might be that one of the downstream pads aren't intersect and would
use is_subset() so always default to subset checks when a downstream
query was made.
Comment 9 Thiago Sousa Santos 2016-01-15 12:54:10 UTC
Used "Since: 1.8" but perhaps that is too close, maybe 1.10 would give us more time to implement it in the elements and see if we find any pitfalls.

One thing that is different from a regular caps event propagation is that the caps event can be accepted by parsers with incomplete caps and then the parser would only propagate its own caps event downstream when it actually parses some data and has more information. With this query this wouldn't be possible.
Comment 10 Sebastian Dröge (slomo) 2016-01-18 09:16:18 UTC
Seems all good to me but let's keep it for early 1.10. Maybe we find a better solution until then :)
Comment 11 GStreamer system administrator 2018-11-03 12:32:23 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/151.