Commit graph

156 commits

Author SHA1 Message Date
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
Olivier Crête
be58999bff aggregator: Protect all latency related members with the object lock
The locking was not consistent, now consistently use the object lock.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
1d5f3fe084 aggregator: Document locking for gst_aggregator_get_latency_unlocked()
Renamed it to _unlocked() to make it clear.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
142073cc79 aggregator: Protect the srcpad caps negotiation with the stream lock
Instead of adding another lock, use the srcpad stream lock, which is already
taken anyway to push out the new caps if needed.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
9add348012 aggregator: Protect the tags with the object lock
The tags related variables were sometimes protected, sometimes not and
sometimes atomic. Put them all under the object lock.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
25faafaf58 aggregator: Consistenly lock the flow_return state
Use the object's lock to protect it.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
90f3cb4af0 aggregator: Consistently lock some members
Some members sometimes used atomic access, sometimes where not locked at
all. Instead consistently use a mutex to protect them, also document
that.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
2b7d4e2404 aggregator: Protect exported pad members with the pad's object lock
https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
3217e82791 aggregator: Replace event lock with pad's object lock
Reduce the number of locks simplify code, what is protects
is exposed, but the lock was not.

Also means adding an _unlocked version of gst_aggregator_pad_steal_buffer().

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Olivier Crête
6379433a70 aggregator: Protect data with the same mutex as GCond
Whenever a GCond is used, the safest paradigm is to protect
the variable which change is signalled by the GCond with the same
mutex that the GCond depends on.

https://bugzilla.gnome.org/show_bug.cgi?id=742684
2017-12-02 15:10:26 +00:00
Nirbheek Chauhan
d33e6a191e aggregator: Nitpick spacing/punctuation in debug logging 2017-12-02 15:10:26 +00:00
Olivier Crête
ec5ff5f308 aggregator: Remove pointless atomic
It is only modified from the streaming thread
2017-12-02 15:10:26 +00:00
Olivier Crête
53dd0c650f aggregator: Fix query leak 2017-12-02 15:10:26 +00:00
Sebastian Dröge
4534253145 aggregator: Print jitter from clock waiting in the debug logs 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
f4ad5f0cdf aggregator: don't use iterator when setting flush pending on pads 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
0db6fc999b aggregator: check if pads are ready more efficiently
No need to use an iterator for this which creates a temporary
structure every time and also involves taking and releasing the
object lock many times in the course of iterating. Not to mention
all that GList handling in gst_aggregator_iterate_sinkpads().
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
100d860ad2 aggregator: name vfunc arguments consistently 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
50908e60d0 aggregator: add g-i transfer and scope annotations 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
6af6dcb0f7 aggregator: register names of iterate_sinkpads functions with debug system 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
417445c065 aggregator: reduce debug messages for taking/releasing logs to TRACE level
Don't spam debug log with this stuff.
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
c5e3fe807a aggregator: move property member into private structure
Our locking (or lack thereof) while accessing this also
looks generally quite dodgy.
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
b813456233 aggregator: remove empty dispose function 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
71ec8e2879 aggregator: give private functions namespace prefix
Especially the GST_DEBUG_FUNCPTR ones.
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
3793bb8ad9 aggregator: fix up some docs comments in header 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
7302a42ac1 aggregator: remove now-unused system clock member 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
9f417fb08e aggregator: make GstAggregatorPadForeachFunc take an GstAggregatorPad 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
601554df70 aggregator: bring start/stop vfunc semantics in line with other baseclasses
Sub-class should not have to chain up to GstAggregator's start/stop
vfuncs, same as in GstBaseSrc, GstBaseSink, GstBaseTransform etc.
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
a7e2a01098 aggregator: remove pointless GST_DEBUG_FUNCPTR
Not useful for GObject vfuncs.
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
760d3c5611 aggregator: remove duplicate pad parent_class variable
G_DEFINE_TYPE already provides one, just need to use it.
2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
85171e3512 aggregator: add _CAST() variants for cast macros 2017-12-02 15:10:26 +00:00
Tim-Philipp Müller
5350d8c653 aggregator: make padding larger
Esp. the class structures, can't have enough
spare space for virtual functions.
2017-12-02 15:10:26 +00:00
Nirbheek Chauhan
ced334d397 aggregator: Log to the pad instead of the element
More correct way of doing the same thing as before
2017-12-02 15:10:26 +00:00
Sebastian Dröge
8da3d1e596 aggregator: Make sure that the minimum latencies are never GST_CLOCK_TIME_NONE 2017-12-02 15:10:26 +00:00
Sebastian Dröge
4846f06b41 aggregator: Wait for the minimum latency, not the maximum
The minimum latency is the latency we have to wait at least
to guarantee that all upstreams have produced data. The maximum
latency has no meaning like that and shouldn't be used for waiting.
2017-12-02 15:10:26 +00:00
Nirbheek Chauhan
d18b6de2e6 aggregator: Clamp the min latency at the max if it's greater 2017-12-02 15:10:26 +00:00
Nirbheek Chauhan
b49805a840 aggregator: Print the sinkpad name while logging latency queries
Very useful while debugging.
2017-12-02 15:10:26 +00:00
Nirbheek Chauhan
bbde713640 aggregator: Take the stream lock when iterating sink pads
When iterating sink pads to collect some data, we should take the stream lock so
we don't get stale data and possibly deadlock because of that. This fixes
a definitive deadlock in _wait_and_check() that manifests with high max
latencies in a live pipeline, and fixes other possible race conditions.
2017-12-02 15:10:26 +00:00
Sebastian Dröge
9bac89113e aggregator: Don't leak flush-start events 2017-12-02 15:10:26 +00:00
Sebastian Dröge
f2814fa5e3 aggregator: Also change the default latency to 0, not just the minimum 2017-12-02 15:10:26 +00:00
Sebastian Dröge
6697f1521a aggregator: Fix docs and default value of the latency property 2017-12-02 15:10:26 +00:00
Sebastian Dröge
4c477549d2 aggregator: Also include the subclass latency in the result of the latency query 2017-12-02 15:10:26 +00:00
Sebastian Dröge
5caf89da37 aggregator: Post a latency message if the value of the latency property changes 2017-12-02 15:10:26 +00:00
Sebastian Dröge
7240852e55 aggregator: Wake up the src thread after handling a latency query
Due to changed latencies or changed live-ness we might have to
adjust if we wait on a deadline at all and how long.
2017-12-02 15:10:26 +00:00
Sebastian Dröge
a20863a7ec aggregator: Don't count the number of times we need to wake up but instead check all conditions for waiting again
This simplifies the code and also makes sure that we don't forget to check all
conditions for waiting.

Also fix a potential deadlock caused by not checking if we're actually still
running before starting to wait.
2017-12-02 15:10:26 +00:00
Sebastian Dröge
b1dcc7efb0 aggregator: Add function to allow subclasses to set their own latency
For audiomixer this is one blocksize, for videoaggregator this should
be the duration of one output frame.
2017-12-02 15:10:26 +00:00
Sebastian Dröge
435a477f63 aggregator: Add a timeout parameter to ::aggregate()
When this is TRUE, we really have to produce output. This happens
in live mixing mode when we have to output something for the current
time, no matter if we have enough input or not.
2017-12-02 15:10:26 +00:00
Sebastian Dröge
bf5652f84a aggregator: Some minor cleanup 2017-12-02 15:10:26 +00:00
Matthew Waters
f4b86a6d8c aggregator: make the src pad task drive the pipeline for live pipelines
This removes the uses of GAsyncQueue and replaces it with explicit
GMutex, GCond and wakeup count which is used for the non-live case.

For live pipelines, the aggregator waits on the clock until either
data arrives on all sink pads or the expected output buffer time
arrives plus the timeout/latency at which time, the subclass
produces a buffer.

https://bugzilla.gnome.org/show_bug.cgi?id=741146
2017-12-02 15:10:26 +00:00
Sebastian Dröge
47d0efacf2 aggregator: Unblock events/queries immediately if the pad is flushing
https://bugzilla.gnome.org/show_bug.cgi?id=740376
2017-12-02 15:10:26 +00:00
Sebastian Dröge
f3c741e74e aggregator: Drop serialized events/queries if the pad is flushing
https://bugzilla.gnome.org/show_bug.cgi?id=740376
2017-12-02 15:10:26 +00:00
Sebastian Dröge
0ad159dfb4 aggregator: Block serialized events/queries until the pad has consumed all buffers
Otherwise the caps of the pad might change while the subclass still works with
a buffer of the old caps, assuming the the current pad caps apply to that
buffer. Which then leads to crashes and other nice effects.

https://bugzilla.gnome.org/show_bug.cgi?id=740376
2017-12-02 15:10:26 +00:00
Sebastian Dröge
e3b42c8acb aggregator: Fix typo in debug output 2017-12-02 15:10:26 +00:00
Matthew Waters
4bdbb7a98f aggregator: add _get_latency() for subclass usage
API: gst_aggregator_get_latency

https://bugzilla.gnome.org/show_bug.cgi?id=739996
2017-12-02 15:10:26 +00:00
Vineeth T M
376de6cef3 audiomixer: critical error for blocksize, timeout min/max values
Audiomixer blocksize, cant be 0, hence adjusting the minimum value to 1
timeout value of aggregator is defined with MAX of MAXINT64,
but it cannot cross G_MAXLONG * GST_SECOND - 1
Hence changed the max value of the same

https://bugzilla.gnome.org/show_bug.cgi?id=738845
2017-12-02 15:10:26 +00:00
Matthew Waters
3c8198e99f aggregator: add latency query handling 2017-12-02 15:10:26 +00:00
Matthew Waters
efdcc6c8eb aggregator: add a timeout property determining buffer wait time
Determines the amount of time that a pad will wait for a buffer before
being marked unresponsive.

Network sources may fail to produce buffers for an extended period of time,
currently causing the pipeline to stall possibly indefinitely, waiting for
these buffers to appear.

Subclasses should render unresponsive pads with either silence (audio), the
last (video) frame or what makes the most sense in the given context.
2017-12-02 15:10:26 +00:00
Jan Alexander Steffens (heftig)
e8c34afd5f aggregator: Replace GMainContext with GAsyncQueue (v2)
The previous implementation kept accumulating GSources,
slowing down the iteration and leaking memory.

Instead of trying to fix the main context flushing, replace
it with a GAsyncQueue which is simple to flush and has
less overhead.

https://bugzilla.gnome.org/show_bug.cgi?id=736782
2017-12-02 15:10:26 +00:00
Mathieu Duponchelle
aaff6b50f8 aggregator: Set seqnum only when segments are received. 2017-12-02 15:10:26 +00:00
Thibault Saunier
1efd6b2edd aggregator: Add a streaming lock so to secure flush start action
Without a lock that is taken in FLUSH_START we had a rare race where we
end up aggregating a buffer that was before the whole FLUSH_START/STOP
dance. That could lead to very wrong behaviour in subclasses.
2017-12-02 15:10:25 +00:00
Thibault Saunier
d5a3056ef2 aggregator: Query seeking when a seek failed to see if it was expected
And do not worry if seeking failed on a stream that is not seekable
2017-12-02 15:10:25 +00:00
Mathieu Duponchelle
f6adb43916 aggregator: set future seqnum before propagating the seek event.
So the seqnum is properly set for the following events.
2017-12-02 15:10:25 +00:00
Thibault Saunier
eb1d322962 aggregator: Store segment when seeked in READY for later use 2017-12-02 15:10:25 +00:00
Sebastian Dröge
034e74b216 aggregator: Unref the taglist in GstAggregator::stop() 2017-12-02 15:10:25 +00:00
Thibault Saunier
cc14d18e08 aggregator: Take lock to ensure set_caps is not called concurently
Avoiding to be in an inconsistent state where we do not have
actual negotiate caps set as srccaps and leading to point where we
try to unref ->srccaps when they have already been set to NULL.

https://bugzilla.gnome.org/show_bug.cgi?id=735042
2017-12-02 15:10:25 +00:00
Matthew Waters
1a0b4f88c2 aggregator: fix up doc comment for set_src_caps
It does not occur 'later' anymore

https://bugzilla.gnome.org/show_bug.cgi?id=732662
2017-12-02 15:10:25 +00:00
Matthew Waters
42e2933133 videoaggregator: push the caps event as soon as we receive it
Along with the required mandatory dependent events.

Some elements need to perform an allocation query inside
::negotiated_caps().  Without the caps event being sent prior,
downstream elements will be unable to answer and will return
an error.

https://bugzilla.gnome.org/show_bug.cgi?id=732662
2017-12-02 15:10:25 +00:00
Mathieu Duponchelle
5ef53b8a29 aggregator: Reset flow_return *after* stopping the srcpad task.
Otherwise it might be set in an already running aggregate function.
2017-12-02 15:10:25 +00:00
Thibault Saunier
e3075ed3ef aggregator: Flush sinkpads when stopping
All values are meaningless in that case, so we should make sure that
we clean everything
2017-12-02 15:10:25 +00:00
Thibault Saunier
7e5d9daf8e aggregator: Do not forget to reset the flow return when stoping
Setting it to FLUSHING when the element is not started, and to OK
when it starts.
2017-12-02 15:10:25 +00:00
Thibault Saunier
9a88538eca aggregator: Handle event seqnum 2017-12-02 15:10:25 +00:00
Tim-Philipp Müller
3a49e1c311 aggregator: fix locking
We would unlock a mutex we never locked on SEGMENT
events.
2017-12-02 15:10:25 +00:00
Thibault Saunier
481476c439 aggregator: Avoid destroying sources we do not own
+ Unref the maincontext in a new dispose function
+ Make sure to remove all sources on dispose

https://bugzilla.gnome.org/show_bug.cgi?id=732445
2017-12-02 15:10:25 +00:00
Thiago Santos
259a73a3dc aggregator: always store or unref the buffer on the _chain function
Otherwise it leaks, and it is very common to go to flushing when the
pipeline is stopping, leaking a buffer.
2017-12-02 15:10:25 +00:00