Commit graph

128 commits

Author SHA1 Message Date
Sebastian Dröge 92a80e6490 aggregator: Don't forward QOS events to sinkpads that had no buffer yet
Otherwise they will receive a QOS event that has earliest_time=0 (because we
can't have negative timestamps), and consider their buffer as too late

https://bugzilla.gnome.org/show_bug.cgi?id=754356
2017-12-02 15:10:26 +00:00
Olivier Crête a7845e80f1 aggregator: Keep at least two buffers in the queue in live mode
When in live mode, the queue needs to hold the currently processed
buffer and one more at least.

https://bugzilla.gnome.org/show_bug.cgi?id=754851
2017-12-02 15:10:26 +00:00
Sebastian Dröge 265ac95553 aggregator: Document that get_next_time() should return running time
https://bugzilla.gnome.org/show_bug.cgi?id=753196
2017-12-02 15:10:26 +00:00
Olivier Crête 2c19653410 aggregator: Also ignore start-time on seek from gst_element_send_event()
https://bugzilla.gnome.org/show_bug.cgi?id=753806
2017-12-02 15:10:26 +00:00
Olivier Crête 22dc57f84e aggregator: Queue "latency" buffers at each sink pad.
In the case where you have a source giving the GstAggregator smaller
buffers than it uses, when it reaches a timeout, it will consume the
first buffer, then try to read another buffer for the pad. If the
previous element is not fast enough, it may get the next buffer even
though it may be queued just before. To prevent that race, the easiest
solution is to move the queue inside the GstAggregatorPad itself. It
also means that there is no need for strange code cause by increasing
the min latency without increasing the max latency proportionally.

This also means queuing the synchronized events and possibly acting
on them on the src task.

https://bugzilla.gnome.org/show_bug.cgi?id=745768
2017-12-02 15:10:26 +00:00
Olivier Crête 5db97caef7 aggregator: Default to "zero" start time selection mode as documented 2017-12-02 15:10:26 +00:00
Olivier Crête 1b3f1b1a4e aggregator: Ignore the "first" mode if the segment not a time segment 2017-12-02 15:10:26 +00:00
Sebastian Dröge 3030d1b501 aggregator: Add property to select how to decide on a start time
Before aggregator based elements always started at running time 0,
now it's possible to select the first input buffer running time or
explicitly set a start-time value.

https://bugzilla.gnome.org/show_bug.cgi?id=749966
2017-12-02 15:10:26 +00:00
Sebastian Dröge 9ab98e9b22 aggregator: Query the peer latency again on the next opportunity after a pad was added or removed
Adding a pad will add a new upstream that might have a bigger minimum latency,
so we might have to wait longer. Or it might be the first live upstream, in
which case we will have to start deadline based aggregation.

Removing a pad will remove a new upstream that might have had the biggest
latency, so we can now stop waiting a bit earlier. Or it might be the last
live upstream, in which case we can stop deadline based aggregation.
2017-12-02 15:10:26 +00:00
Thiago Santos 07247f01ca aggregator: add a convenience macro to get the source pad
Easier than casting or acessing the parent everywhere
2017-12-02 15:10:26 +00:00
Olivier Crête 0a35f550b2 aggregator: Document that the latency is in ns 2017-12-02 15:10:26 +00:00
Jan Schmidt b16697d286 aggregator: Push EOS on error return.
Before shutting down the srcpad task due to a
downstream error, push an EOS to give downstream
a chance to shut down somewhat cleanly.
2017-12-02 15:10:26 +00:00
Reynaldo H. Verdejo Pinochet 656b7a0594 aggregator: document gap handling behavior
https://bugzilla.gnome.org/show_bug.cgi?id=746249
2017-12-02 15:10:26 +00:00
Reynaldo H. Verdejo Pinochet 3f56fb7189 aggregator: drop stale white space at warning 2017-12-02 15:10:26 +00:00
Reynaldo H. Verdejo Pinochet c955254758 aggregator: implement gap handling
https://bugzilla.gnome.org/show_bug.cgi?id=746249
2017-12-02 15:10:26 +00:00
Olivier Crête e73a173224 aggregator: Unify downstream flow return and flushing
Also means that having a non-OK downstream flow return
wakes up the chain functions.

https://bugzilla.gnome.org/show_bug.cgi?id=747220
2017-12-02 15:10:26 +00:00
Olivier Crête 30eb13a16d aggregator: Flushing is always in pad lock, no need to atomics
The usage of atomics was always doubtful as it was used to release a
GCond

https://bugzilla.gnome.org/show_bug.cgi?id=747220
2017-12-02 15:10:26 +00:00
Olivier Crête c55b6be463 aggregator: Reset pending_eos on pad flush
https://bugzilla.gnome.org/show_bug.cgi?id=747220
2017-12-02 15:10:26 +00:00
Olivier Crête d88e064019 aggregator: Unify code to set a pad flushing
https://bugzilla.gnome.org/show_bug.cgi?id=747220
2017-12-02 15:10:26 +00:00
Olivier Crête 5a0e2c4c47 aggregator: Query latency on first incoming buffer.
And keep on querying upstream until we get a reply.

Also, the _get_latency_unlocked() method required being calld
with a private lock, so removed the _unlocked() variant from the API.
And it now returns GST_CLOCK_TIME_NONE when the element is not live as
we think that 0 upstream latency is possible.

https://bugzilla.gnome.org/show_bug.cgi?id=745768
2017-12-02 15:10:26 +00:00
Olivier Crête 46e33e84fd aggregator: Be more aggressive with invalid replies to our latency query
https://bugzilla.gnome.org/show_bug.cgi?id=745768
2017-12-02 15:10:26 +00:00
Matthew Waters 87b25fb7a5 aggregatory: don't redefine GST_FLOW_CUSTOM_SUCCESS 2017-12-02 15:10:26 +00:00
Arun Raghavan b7155695f8 aggregator: Use standard upstream latency querying logic
The same functionality is duplicated in the default latency querying
now.
2017-12-02 15:10:26 +00:00
Olivier Crete 5ac166a5d9 aggregator: Use src_lock to protect latency related members
One has to use the src_lock anyway to protect the min/max/live so they
can be notified atomically to the src thread to wake it up on changes,
such as property changes. So no point in having a second lock.

Also, the object lock was being held across a call to
GST_ELEMENT_WARNING, guaranteeing a deadlock.
2017-12-02 15:10:26 +00:00
Olivier Crête 77698267db aggregator: Remove untrue comment 2017-12-02 15:10:26 +00:00
Olivier Crête 8f8430ee9e aggregator: Don't try to push tags while flush seeking
The downstream segment could have been flushed already, so
need to re-send the segment event before re-sending the tags.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Sebastian Dröge 83203e9dc4 aggregator: Use the sinkpads iterator directly to query upstream latencies
While gst_aggregator_iterate_sinkpads() makes sure that every pad is only
visited once, even when the iterator has to resync, this is not all we have
to do for querying the latency. When the iterator resyncs we actually have
to query all pads for the latency again and forget our previous results. It
might have happened that a pad was removed, which influenced the result of
the latency query.
2017-12-02 15:10:26 +00:00
Sebastian Dröge 075cfd8d7b aggregator: Move gst_aggregator_get_latency_unlocked() a bit
It was between another function and its helper function before, which was
confusing when reading the code as it had nothing to do with the other
functions.
2017-12-02 15:10:26 +00:00
Sebastian Dröge 2b932dc551 aggregator: Fail the latency query if one of the upstream queries fails 2017-12-02 15:10:26 +00:00
Olivier Crête a42c4740ec aggregator: Document locking order
https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête ad2cd52bae aggregator: Rename confusinly named SRC_STREAM_LOCK macros to SRC_LOCK
This will match the name of the lock itself. It is also not a stream
lock as it not recursive and not held while pushing.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête 3c17c777ee aggregator: Rename confusingly named stream lock to flush lock
This lock is not what is commonly known as a "stream lock" in GStremer,
it's not recursive and it's taken from the non-serialized FLUSH_START event.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête aa78cf96f4 aggregator: Fix macro indendation
Changes no code

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller e1aba92514 aggregator: drop GAP events until we handle them properly 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller 1fe5f9db71 aggregator: use new gst_aggregator_pad_drop_buffer() 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller 950b772a0d aggregator: add gst_aggregator_pad_drop_buffer()
steal_buffer() + unref seems to be a wide-spread idiom
(which perhaps indicates that something is not quite
right with the way aggregator pad works currently).
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller 4e97321898 aggregator: only post latency message if anything changed
Perhaps we should check for element state as well and
only post it if in PLAYING state.
2017-12-02 15:10:26 +00:00
Sebastian Dröge d97d363b1f Improve and fix LATENCY query handling
This now follows the design docs everywhere, especially the maximum latency
handling.

https://bugzilla.gnome.org/show_bug.cgi?id=744106
2017-12-02 15:10:26 +00:00
Sebastian Dröge 1609e1d696 aggregator: Pause srcpad task on flow errors
Otherwise we will call the task function over and over again until
upstream finally handled the flow return and shuts us down.
2017-12-02 15:10:26 +00:00
Sebastian Dröge 9213f24716 aggregator: Streamline latency calculations
Min latency can never be invalid, latency property can never be invalid
either. So no need to check for all these things in various places.
2017-12-02 15:10:26 +00:00
Sebastian Dröge 07d9bf9cab aggregator: If upstream has no max latency but the subclass has, take the subclass max latency 2017-12-02 15:10:26 +00:00
Sebastian Dröge e57b62be72 aggregator: Fix min>max latency error check
We have to include the upstream latency, our own latency and the subclass
latency in the calculations.

FIXME: This is still not entirely correct
2017-12-02 15:10:26 +00:00
Sebastian Dröge 87a2fbab60 aggregator: Don't add the latency property to the max latency
It has no meaning for the max latency and is only used to increase the min
latency.
2017-12-02 15:10:26 +00:00
Thibault Saunier 24abac9fce aggregator: Cleanup locking around AggregatorPad flush related fields
And document the locking

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Mathieu Duponchelle 14f35e8a78 aggregator: keep chain functions as dumb as possible.
+ A pad chain function has no business checking other pads,
  that's what the aggregate thread is for.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Thibault Saunier 97eab82ddf aggregator: More fixes around locking when accessing protected private fields
In some more places we were accessing GstAggregator->segment
and GstAggregator->seqnum without holding the GST_OBJECT_LOCK

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Thibault Saunier ae12041f0c aggregator: Make the PAD_LOCK private
Instead of using the GST_OBJECT_LOCK we should have
a dedicated mutex for the pad as it is also associated
with the mutex on the EVENT_MUTEX on which we wait
in the _chain function of the pad.

The GstAggregatorPad.segment is still protected with the
GST_OBJECT_LOCK.

Remove the gst_aggregator_pad_peak_unlocked method as it does not make
sense anymore with a private lock.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Thibault Saunier 5f176b724b aggregator: Hide GstAggregatorPad buffer and EOS fileds
And add a getter for the EOS.

The user should always use the various getters to access
those fields

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête 0588222925 aggregator: Document locking of GstAggregatorPrivate members
Most of them are protected by the object lock, specify
which ones use a different lock.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête fd326639ea aggregator: Document how the segment is protected
Document that it can only be accessed with the object lock.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00