[Bug 741146] aggregator: deadline based aggregation

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=741146
  GStreamer | gst-plugins-bad | git

--- Comment #3 from Matthew Waters <ystreet00 at gmail.com> 2014-12-06 13:31:04 UTC ---
(In reply to comment #2)
> Review of attachment 292171 [details]:
> 
> ::: gst-libs/gst/base/gstaggregator.c
> @@ +479,3 @@
> +{
> +  if (buffer && GST_BUFFER_TIMESTAMP_IS_VALID (buffer)) {
> +    *start = GST_BUFFER_TIMESTAMP (buffer);
> 
> This never happens as this is always called with buffer==NULL?

Currently, yes.

> For the "default case" of subclasses that don't give us times, we should just
> disable the deadline system entirely?

The default case in basesink is to retrieve that information from the buffer.

> @@ +508,3 @@
> +/* called with the src STREAM lock */
> +static gboolean
> +_wait_and_check (GstAggregator * self, GstBuffer * buffer)
> 
> Where does the buffer come from? It seems to always be called with NULL?

Nowhere yet but when aggregator does the allocation query it will need to deal
with buffer pools and allocating buffers like basetransform and basesrc so that
would be the buffer that is used.

> ::: gst-libs/gst/base/gstaggregator.h
> @@ +240,3 @@
> +  void              (*get_times)      (GstAggregator    *  aggregator,
> +                                       GstBuffer        *  buffer,
> +                                       GstClockTime     *  start,
> 
> My gut feeling is that it should be get_next_time() that returns the start,
> because this is called before the buffer is created. And probably allow
> returning GST_CLOCK_TIME_NONE to mean "ASAP".

This is modelled after basesink's get_times() and its usage.  Another use case
for it is calculating buffer durations for qos like what is currently done in
videoaggregator.

-- 
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.