[Bug 739010] [PLUGIN MOVE] Move GstAggregator to gstreamer (core)

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=739010
  GStreamer | gstreamer (core) | git

--- Comment #16 from Olivier Crete (Tester) <olivier.crete at ocrete.ca> 2014-11-14 16:47:31 UTC ---
(In reply to comment #13)
> (In reply to comment #9)
> > The unresponsive functionality should inform the subclass at which time it is
> > now, according to the pipeline clock, that way, the subclass can fill in the
> > right amount of "gap data", but not more. I guess this could also be used to
> > process incoming GAP events and give them to the subclasses.
> 
> That's handled elsewhere by the subclasses atm.  GstAggregator currently
> doesn't deal at all with how the data is matched against stream times.

But knowing the deadline would be useful to the subclasses, I'm currently
trying to implement a raw audio subclass based on the audiomixer code and
porting interleave to us it as I need a synchronized interleave.

> (In reply to comment #10)
> > Because we want the timeouts to follow the speed at which it is displayed,
> > which is based on the pipeline clock. We really have no guarantees that this
> > clock has anything to do with the system clock. Also, you add it to the latency
> > query, and you can only do that if you use the pipeline clock, you're comparing
> > apples and oranges.
> 
> The speed of what? display? Why would you dynamically adjust the timeout based
> on some speed?

The speed of the clock. If the pipeline clock is a bit faster than the system
clock, then every packet will arrive late at the sink.

> I think that some small duration in two separate clocks will amount to a close
> enough elapsing of time that it does not make a difference.

Most clocks go about the same speed, why do we even bother with having a
pipeline clock then? If we want it in core, let's do it right.

> > > (In reply to comment #7)
> > I think it should work along the same lines as rtpjitterbuffer. So the variable
> > we want to control is how long can useful data stay in the aggregator before it
> > is pushed out. And that should be based on the oldest valid unit of data stuck
> > in the aggregator.
> 
> Which is exactly the same as 'if a buffer does not occur on a pad after the
> timeout, ignore it and generate a frame anyway'.
> 
> ASCII diagram :)
> 
> b1, b2, b3 = buffer
> f = generated frame
> | = timeout occurences
> 
> ----- running time ------>
> . timeout |(b1)  timeout |(b2)|(b3)
> b1- - - - - - - - - b3- - - - - -  -> to sink1
> - - - - - - - b2- - - - - - - - -  -> to sink2
> __________f__________f________f
> 
> As is visible from the great diagram, a frame is generated at most after the
> timeout has occured.

I think that the mistake you're making is assuming that the time where the
buffer is entering the aggregator is a useful point to start from even though
we have no idea if it's early or it's late. I think all of this should be based
on the time when it should reach the sink (ie, the buffer timestamp).

> (In reply to comment #11)
> > Basically, to fulfill the latency promise made in the response to the latency
> > query, a buffer has to exit the element when the pipeline clock reaches
> > buffer_in_running_time + element_latency + upstream_latency at the latest.
> 
> Which is upheld in the current code.

It's not, you're waiting on a completely different clock which for all we know
could go measurably slower.

> (In reply to comment #12)
> > Another thing, it's missing an API for subclasses to set the amount of latency
> > they are adding.
> 
> Subclasses can override the latency query where they call the base class and
> then add their own if they need.

They could, but it would be nice to a gst_aggregator_set_latency() like the
encoder & decoder base classes, it's quite handy.


(In reply to comment #14)
> Another consideration that was brought to my attention is that if you take a
> random pipeline clock it might only advance while the pipeline is playing and
> we do want to timeout a pad in paused so that we are not forever waiting to
> generate a buffer because of a buffer that's never going to arrive.

The whole thing is really only for live sources. On a non-live sources, you can
read more data or get a EOS, so this timeout doesn't apply. On a live stream,
you won't get any buffers unless you're in playing anyway.

(In reply to comment #15)
> Also rtpjitterbuffer was mentioned here... but last time I checked it was using
> the system clock unconditionally.

That's just factually not true. rtpjitterbuffer proposes the system clock in
case there is no other clock in the pipeline. You're probably confused with the
optional usage of the system clock to produce the NTP times in rtpsession, but
that's always compared to the pipeline clock.

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