GNOME Bugzilla – Bug 638749
(Duration) queries on a source-only bin are currently not supported
Last modified: 2013-02-12 09:34:41 UTC
GstBin's gst_bin_query() handler currently only supports forwarding queries to sink children. In WebKit's GStreamer back-end we have a source-only bin, on which we would like to do a duration query. It would be nice if the gst_bin_query() handler would support this, for example perhaps by falling back to what the gst_element_default_query() function does.
This affects WebKitGtk upstream at https://bugs.webkit.org/show_bug.cgi?id=51926
Created attachment 177591 [details] [review] bin: query sink elements and source pads of the bin gst_bin_query() now forwards the query to the source pads as well if none of the sinks of the bin satisfied the query. This helps in the case of DURATION queries done a bin containing a source element. Fixes bug 638749
Created attachment 177594 [details] [review] bin: query sink elements and source pads of the bin gst_bin_query() now forwards the query to the source pads as well if none of the sinks of the bin satisfied the query. This helps in the case of DURATION queries done a bin containing a source element. Fixes bug 638749
This second version uses GST_IS_PAD instead of the slower GST_IS_ELEMENT macro and always forwards the query to src pads, as advised by Sebastian.
This patch needs some more work, I think the fold data might be reset unexpectedly during the second iteration if a RESYNC is emitted by the gst_iterator.
I talked to Wim and he said that this change makes sense too for the DURATION queries. And that GstElement's query handler should also iterate over all srcpads and take the maximum instead of just taking a random element like it does now. You might also want to pass the pad-or-element information via the user data of the fold function btw. And really just do two folds with completely reinitialized state and everything and then just take the maximum of both results
(In reply to comment #6) > I talked to Wim and he said that this change makes sense too for the DURATION > queries. And that GstElement's query handler should also iterate over all > srcpads and take the maximum instead of just taking a random element like it > does now. > Ok, but only for DURATION queries, right? > You might also want to pass the pad-or-element information via the user data of > the fold function btw. And really just do two folds with completely > reinitialized state and everything and then just take the maximum of both > results I made those changes in my branch, thanks :)
Well, for all query types that are currently handled in a special way, e.g. LATENCY. While you're at it you could also add this to GstElement's default query function btw ;) Also, where's your branch?
I moved the "query iterate folding" code out of gstbin so it can be used in gstelement default query handler. The branch is there: http://git.igalia.com/cgi-bin/gitweb.cgi?p=user/pnormand/gstreamer.git;a=shortlog;h=refs/heads/bug/638749
Is your branch ready for review and committing?
I was thinking of fixing position queries wrt negative playback rate too but maybe it can be done later on? Apart from that the branch would need a rebase, I'll do that tomorrow and check the patches again before pinging you.
Hi Sebastian, I updated the branch with some changes: - rebased against master - renamed the test element to mocksrc - fixed 2 leaks in the test - and some minor other changes The position query fixing should be part of another patch I believe now. Can you review the branch when you have some time please?
* You're adding the query iterate stuff as public API. Not a good idea, please put the header in noinst_HEADERS (or put the content in gstprivate.h) * Add some (non-gtkdoc) documentation to the query iteration API * You should probably do the srcpad iteration only when necessary, i.e. for duration queries so far * In the GstElement default query handler, maybe do the iteration stuff for the sinkpads too instead of taking a random pad? * Well... and the position query stuff. Just add another commit to your branch, no need for a new bug. It's easy enough :) * I'm not 100% sure but I think most of this can be in the GstElement default query handler and the GstBin query handler can just chain up to it in most cases. Could you check this? Other than that this looks good
(In reply to comment #13) > * You're adding the query iterate stuff as public API. Not a good idea, please > put the header in noinst_HEADERS (or put the content in gstprivate.h) Hum it's in noinst_HEADERS already :) > * Add some (non-gtkdoc) documentation to the query iteration API OK > * You should probably do the srcpad iteration only when necessary, i.e. for > duration queries so far This contradicts your comment 8 or am I misunderstanding something? > * In the GstElement default query handler, maybe do the iteration stuff for the > sinkpads too instead of taking a random pad? > * Well... and the position query stuff. Just add another commit to your branch, > no need for a new bug. It's easy enough :) > OK! > * I'm not 100% sure but I think most of this can be in the GstElement default > query handler and the GstBin query handler can just chain up to it in most > cases. Could you check this? > OK I'll check.
(In reply to comment #14) > (In reply to comment #13) > > * You're adding the query iterate stuff as public API. Not a good idea, please > > put the header in noinst_HEADERS (or put the content in gstprivate.h) > > Hum it's in noinst_HEADERS already :) Sorry, you're right :) > > * You should probably do the srcpad iteration only when necessary, i.e. for > > duration queries so far > > This contradicts your comment 8 or am I misunderstanding something? Well, for non-duration queries you're currently iterating the srcpads but you don't use the result. That's what I mean, if you don't use the result anyway you can skip the iterating too. In comment #8 I meant that you should do the srcpad iterations for other queries too but of course you have to do something with the result then. And it was about doing it in GstElement too
(In reply to comment #13) > * In the GstElement default query handler, maybe do the iteration stuff for the > sinkpads too instead of taking a random pad? > > * I'm not 100% sure but I think most of this can be in the GstElement default > query handler and the GstBin query handler can just chain up to it in most > cases. Could you check this? > Indeed it seems that most of the GstQuery management of GstBin could move to GstElement. But maybe we can do that move as a follow-up of this patch? I'm going to upload a new version now.
Created attachment 235589 [details] [review] bin: query sink elements and source pads of the bin gst_bin_query() now forwards the query to the source pads as well if none of the sinks of the bin satisfied the query. This helps in the case of DURATION queries done a bin containing a source element. Fixes bug 638749
commit f3d268de7f7fb1161778a9a95e0d54d8c89ef626 Author: Philippe Normand <philn@igalia.com> Date: Sat Feb 9 18:14:09 2013 +0100 bin: query sink elements and source pads of the bin gst_bin_query() now forwards the query to the source pads as well if none of the sinks of the bin satisfied the query. This helps in the case of DURATION queries done a bin containing a source element. Fixes bug 638749