[Bug 739010] [PLUGIN MOVE] Move GstAggregator to gstreamer (core)
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Sun Jan 4 04:52:29 PST 2015
https://bugzilla.gnome.org/show_bug.cgi?id=739010
GStreamer | gstreamer (core) | git
--- Comment #38 from Tim-Philipp Müller <t.i.m at zen.co.uk> 2015-01-04 12:52:25 UTC ---
oggmux port to GstAggregator can be found here:
http://cgit.freedesktop.org/~tpm/gst-plugins-base/log/?h=oggmux-aggregator
Couple of issues:
- we need a replacement/equivalent for
gst_collectpads_set_waiting() to handle
sparse streams. I suspect this is not too
difficult.
- there seem to be some issues with passing
the flow returns upstream, probably because
there has so far been an assumption that every
call to the ::aggregate() vfunc generates at least
one output buffer. If that's not the case, and the
aggregate vfunc returns FLOW_OK when there's
nothing to do, then that might/will overwrite a
"bad" flow return from a previous _finish_buffer()
that had been stored in priv->flow_return.
Possible solutions:
1) add custom flow return to distinguish NO_CHANGE from OK
2) just require sub-classes to track last_flow themselves in this
case and return the latest and greatest from ::aggregate()
3) never overwrite (fatal) non-FLOW_OK with FLOW_OK in GstAggregator
- relatedly, shouldn't pads stuck in _pad_chain () be
woken up in order to return upstream any non-OK
flow return immediately? (The assumption seems
to be that most if not all pads will see buffers popped
off during aggregation, but that might not be true for
a muxer or subtitle overlay, and not even for a mixer)
- aggregator->priv->flow_return access is not thread-safe
in general (e.g. in gst_aggregator_pad_chain)
- do we need/want a GstAggregatorPad::reset() vmethod?
(to reset pad state when shutting down, to make element re-usable)
Or do we assume that flush == reset?
- GST_EVENT_TAG handling looks bogus, stream tags
are merged into one global tag list?! I'm not even sure
if this makes sense for mixers, but muxers probably
want access to per-stream tags. non-global tags should
probably just be dropped and not merged.
- Not sure I like the naming of
gst_aggregator_pad_get_buffer() / _steal_buffer().
This kind of naming is usually about ownership transfer
and thus doesn't reflect the actually more important part
of the calls, which is whether the buffer stays on the pad
or is popped off. I would suggest:
- gst_aggregator_pad_peek_buffer()
- gst_aggregator_pad_pop_buffer() or _dequeue_buffer()
and it's a bit counter-intuitive and inconvenient that get()/peek()
returns a ref as well (though collectpads does that too). I think
any caller to get/peek() holds a ref to the pad, so one could be
sure that any buffer the pad owns is not going to go away while
we'd be peeking at it.
- it would be nice if we could incorporate queuing
functionality on the input side. (Can be added later)
- gst_aggregator_request_new_pad() should not insist
on sink pad template name of "sink_%u" (could be
audio_%u or somesuch too, etc.)
- I think we should rather add the pad GType to
GstPadTemplate, which would also be useful for
gst-inspect.
- I wonder if the "latency" property should not rather
be called "extra-latency" ? Or perhaps I misunderstand
it. The our_latency > data.max comparison in
gst_aggregator_query_latency() doesn't look right if
it's extra latency though.
--
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.
More information about the gstreamer-bugs
mailing list