Commit graph

414 commits

Author SHA1 Message Date
Jun Xie
f88b20397f decodebin: Fix typo in docs
https://bugzilla.gnome.org/show_bug.cgi?id=791058
2017-12-01 10:03:40 +02:00
Edward Hervey
38c835ec1e decodebin2: Don't spawn threads on shutdown
If we are shutting down, don't spawn a cleanup thread to cleanup old
groups and instead queue them to be cleaned up in the state change
thread.

This avoids (hopefully for good) having a race between the state change
thread and other threads trying to deactivate elements/pads.
2017-11-16 18:22:20 +01:00
Edward Hervey
e72aa501b0 decodebin2: Avoid deactivation races
Deactivating pads from two threads isn't 100% MT-safe. There is a
slim chance that the GstPadActivateFunc might be called twice with
the same values (in this case from the cleanup thread *and* from
the GstElement change_state function when going from PAUSED to READY).

In order to avoid that, call any existing cleanup function *before*
calling the parent change_state implementation on downwards state
changes.
2017-11-16 06:44:10 +01:00
Edward Hervey
ce8f51e30e decodebin2: Don't try to add pads when shutting down
Be even more agressive than before and just hold the DYN lock when/if
adding pads and return if we are shutting down.
2017-11-14 13:40:50 +01:00
Edward Hervey
77e5a71a15 decodebin2: Don't take locks when deactivating pads
When deactivating pads, we need to ensure that the streaming threads
going through the pads we wish to deactivate can cleanly return.
Failure to do that would result in the streaming locks of those
pads never being released. The end result would be a deadlock
when stopping decodebin2.

In order to avoid that situation, release the "dyn" lock around
the deactivation code. And refactor the code to cope with the
list of blocked pads having potentially changed when re-acquiring
the lock.
2017-11-14 13:40:48 +01:00
Edward Hervey
43b9bcbddb decodebin2: Don't let thread run after unref
We have a dedicated one-shot thread to handle cleanup of old groups.

While this is a good idea. It's an even better idea to make sure
that thread is *completed* before the decodebin2 element to which
it is related isn't freed/gone.

* There can only be one cleanup thread happening at any point in time.
  If there is already one, we wait for the previous one to finish.
* When shutting down (NULL=>READY) make sure the thread is finished

https://bugzilla.gnome.org/show_bug.cgi?id=790007
2017-11-10 17:06:10 +01:00
Edward Hervey
f448cecceb decodebin2: Recheck shutdown behaviour in error case
If we can expose the main chain, recheck whether we are shutting
down or not.

decodebin2 might have been set to READY/NULL during the attempt
to expose, which would cause it to fail ... but it is not a fatal
issue.
2017-11-02 07:55:16 +01:00
Reynaldo H. Verdejo Pinochet
e81c334ca9 Use proper GtkDoc notation for NULL/FALSE/TRUE 2017-10-03 14:31:18 -07:00
Sebastian Dröge
d5069d27ef Revert "decodebin2: Set a time limit on "upstream" multiqueues"
This reverts commit 07dc9ba071. It causes
timeouts in validate because queues run full before prerolling.
2017-05-31 12:30:40 +03:00
Edward Hervey
07dc9ba071 decodebin2: Set a time limit on "upstream" multiqueues
Those multiqueue are the ones dealing with adaptive demuxers. They should
have a time limit set so that they don't end up buffering too much data.

They would previously be set with no limits at all, which would cause them
to grow indefinitely until downstream blocks.
2017-05-31 10:27:19 +03:00
Vincent Penquerc'h
b97cbe678f decodebin2: fix use after free from demuxer flush pad probe
In some cases, we could get a flush-stop event after the chain structure
containing the demuxer was freed.

https://bugzilla.gnome.org/show_bug.cgi?id=782095
2017-05-03 17:37:12 +01:00
Jan Schmidt
0b6a933e01 decodebin: Close a small race posting 100% buffering
When posting 100% buffering due to removing the last
buffering element, we still need to hold the posting
lock as well, to avoid any race with other elements
that might post a buffering message at that exact
moment
2017-03-18 02:03:47 +11:00
Thibault Saunier
099ac9faf2 docs: Convert gtkdoc comments to markdown
Modernizing the documentation, making it simpler to read an
modify and allowing us to possibly switch to hotdoc in the
future.
2017-03-10 18:19:17 -03:00
Jan Schmidt
54bf104274 decodebin: Don't leak blocked pad references on errors
When the decodebin state change fails because of an error
message, we might not go through PAUSED->READY. Don't leak
a ref to decodebin pads due to pad blocking in that case.

This is because we return ASYNC going to PAUSED, and if
we fail before reaching PAUSED the only transition we'll
see is READY->NULL.

https://bugzilla.gnome.org/show_bug.cgi?id=775893
2017-01-18 13:09:06 +02:00
Jan Schmidt
c2a91d2cfd playback: Fix a small race on decodebin/parsebin shutdown.
When shutting down decodebin2 and parsebin, they set their
output pads to flushing, and there is a very small window
where elements might send a sticky event such as a tag event
(which silently fails due to flushing) and then sends a buffer,
and the buffer will return GST_FLOW_ERROR because it can't
forward sticky events. The element will then send an error
message on the bus. This can also happen when elements send EOS
just as shutdown is happening. Since we're about to destroy all
the elements inside parsebin and decodebin anyway, just discard
error messages from them.

A nicer but more difficult fix for GStreamer 2.0 is to make
all event pushing / handling in core return a GstFlowReturn
like buffers do, so we can report a FLUSHING state cleanly.
2017-01-03 02:27:51 +11:00
Sebastian Dröge
ce693174f4 decodebin2: Put the correct element srcpad into the topology for the very last element of a chain
We were putting the decode pad there, which is the ghostpad linked to
the last element. The decode pad is already in the pad field.
2016-12-17 22:01:10 +02:00
Sebastian Dröge
46835f550d decodebin2: Put the correct pad into the stream-topology if a parser/converter is used
We have to take the capsfilter into account then as the elements are not
linked directly. Previously this caused NULL be set in these cases.
2016-12-17 22:01:10 +02:00
Sebastian Dröge
991758c3d6 decodebin: For adaptive streaming, ensure to put the buffering multiqueue after a parser or demuxer
There are cases when there is no demuxer involved that could do the
buffering, e.g. HLS with raw MP3 or AAC. In this case we want to place
the buffering multiqueue after the parser.

Before this change, we've considered the first element after the
adaptive streaming demuxer as a parser. This is not always true, e.g.
id3demux. Instead we now wait until we actually have a parser (or
decoder).

Fixes playback on such HLS streams.
2016-12-15 16:31:20 +02:00
Wonchul Lee
0cc3f199ca playbackutils: Move compare_factories_func
Move _decode_bin_compare_factories_func function to playbackutils

https://bugzilla.gnome.org/show_bug.cgi?id=770692
2016-09-01 13:06:51 +03:00
Josep Torra
970ea49d30 decodebin: forward sticky events on multiqueue
When connecting a demuxer through a multiqueue ensure to copy sticky
events in order to allow the following factory being properly
checked that it is functional.

https://bugzilla.gnome.org/show_bug.cgi?id=769580
2016-08-25 11:22:46 +02:00
Jan Schmidt
828cb0d86a decodebin: Send stream-group-done to unblock downstream
When processing EOS for a pad, send a stream-group-done
for the pad in case downstream is waiting for more
data on this stream before it can process related
streams from the group.

https://bugzilla.gnome.org/show_bug.cgi?id=768995
2016-07-25 19:56:17 +10:00
Sebastian Dröge
17d04998c0 decodebin: Create a new decode element with the parser/convert capsfilter if there is a multiqueue after the parser
https://bugzilla.gnome.org/show_bug.cgi?id=767102
2016-06-02 10:50:58 +03:00
Alessandro Decina
fe4e9bb02c decodebin: an element can negotiate before we block it
When we initialize an element in decodebin, we 1) set it to PAUSED and
push sticky events on its sinkpad to trigger negotiation 2) block its
src pad(s) to detect CAPS events. We can't block before 1) as that
would lead to a deadlock.

It's possible (and common) tho that an element configures its srcpad
during 1) and before 2). Therefore before this change we would
typically block and expose an element's pad only once the element
output its first buffer, triggering sticky events to be resent. One
consequence of this behaviour is that it sometimes broke
renegotiation.

With this change now we consider a pad ready to be exposed when it's
->blocked or has fixed caps (which were set before we could block it).

https://bugzilla.gnome.org/show_bug.cgi?id=765456
2016-05-04 10:13:44 +03:00
Vivia Nikolaidou
2b53646715 decodebin: Always add a multiqueue in single-stream use-buffering pipelines
If we are configured to use buffering and there is no demuxer in the chain, we
still want a multiqueue, otherwise we will ignore the use-buffering property.
In that case, we will insert a multiqueue after the parser or decoder - not
elsewhere, otherwise we won't have timestamps.

https://bugzilla.gnome.org/show_bug.cgi?id=764948
2016-04-19 10:19:07 +03:00
Vivia Nikolaidou
9d96959dde decodebin: Rename misleading variable is_parser_converter into is_parser
In that place, the variable isn't checking whether the element is a
converter, only if it is a parser.

https://bugzilla.gnome.org/show_bug.cgi?id=764948
2016-04-12 17:34:18 +03:00
Jan Schmidt
fd92bdf894 decodebin2: Hold new buffering_post lock while posting msgs
There's a small window between decodebin choosing a buffering level
to post and another thread choosing a different buffering level
where things can race. Close that window by holding a new lock
that's only for posting buffering messages - like what was done
in multiqueue.

https://bugzilla.gnome.org/show_bug.cgi?id=764020
2016-03-24 15:01:15 +02:00
Jimmy Ohn
090d0d1961 decodebin: Modify result of seekable in check_upstream_seekable function
In check_upstream_seekable function, it returns FALSE value even though
we already declare about the seekable variable. So, This patch return
result of seekable in check_upstream_seekable function.

https://bugzilla.gnome.org/show_bug.cgi?id=763975
2016-03-24 14:26:23 +02:00
Vineeth TM
44b70ca3a1 base: use new gst_element_class_add_static_pad_template()
https://bugzilla.gnome.org/show_bug.cgi?id=763075
2016-03-24 14:25:41 +02:00
Sebastian Dröge
9c2d76fb9f decodebin: Shut down all elements explicitly to NULL state before freeing the decode chain
Due to transient locked state during autoplugging, some elements might be
ignored by the GstBin::change_state() and might still be running. Which could
then cause pad-added and similar accessing decodebin state that does not exist
anymore, and crash.

https://bugzilla.gnome.org/show_bug.cgi?id=763625
2016-03-14 17:09:32 +02:00
Sebastian Dröge
916746e731 decodebin: expose_pad() is always called with lock==TRUE, simplify code
This basically reverts ee44337fc3 .

https://bugzilla.gnome.org/show_bug.cgi?id=763491
2016-03-14 12:45:29 +02:00
Sebastian Dröge
65d09c1495 decodebin: Don't check twice if the decode chain is complete in pad_added_cb()
expose_pad() already does the same.

https://bugzilla.gnome.org/show_bug.cgi?id=763491
2016-03-14 12:45:29 +02:00
Sebastian Dröge
001c7f04a0 decodebin: Don't hold EXPOSE_LOCK in type_found() outside the stream lock
In other places we lock it the other way around, leading to possible
deadlocks. Also this will deadlock if analyze_pad() causes a new element to be
autoplugged that adds new pads on itself when its state is changed.

https://bugzilla.gnome.org/show_bug.cgi?id=763491
2016-03-14 12:45:29 +02:00
Vineeth T M
5d78aab810 decodebin: return incomplete topology if decode chains' cap could not be obtained
When getting caps of the decode chain, in get_topology, the caps are being
checked if fixed or not. But get_topology will be called when the decode is
chain is being exposed and hence it will always be fixed. Hence removing the
check for fixed caps. Removing gst_pad_get_current_caps for the chain->pad, as
get_pad_caps will again call the same api.

And get_topology can return NULL value if currently shutting down the
pipeline, which on being passed to create message will result in assertion
error. Check if topology is valid before using it

https://bugzilla.gnome.org/show_bug.cgi?id=755918
2016-02-17 10:48:29 +02:00
Sebastian Dröge
6c2ee2853d decodebin: Fix documentation of the autoplug-query signal 2016-02-15 21:28:33 +02:00
Sebastian Dröge
acd08a828d decodebin: Correctly expose pads from elements that have directly exposable pads
analyze_new_pad() can return a new decode chain, which might have a new
GstDecodePad in the end. We should use those two for expose_pad() and not the
original ones that were passed to analyze_new_pad().

This fails when having a demuxer element that has raw pads immediately or
if a decoder with raw caps is after an adaptive demuxer.

https://bugzilla.gnome.org/show_bug.cgi?id=760949
2016-01-25 13:50:26 +01:00
Sebastian Dröge
60bad4815d Revert "decodebin2: fix deadlock on chain shutdown"
This reverts commit 77dc09c3a9.

It can cause the FLUSH_START/STOP events to go to the sink elements, which
then causes state changes and various other problems. We shouldn't really
flush downstream here, the idea is to do *draining*.

Apart from that the testcase for the original bug here works without this
commit now.
2015-12-16 17:09:25 +01:00
Tim-Philipp Müller
71505dfa24 decodebin2: fix "Attempt to unlock mutex that was not locked"
Introduced in commit ee44337f, caused the decodebin
test_text_plain_streams unit test to abort.

https://bugzilla.gnome.org/show_bug.cgi?id=752651
2015-12-02 18:16:05 +00:00
Sebastian Dröge
9e4bf58b8e decodebin: Update buffering messages when removing an element that had buffering pending
Otherwise we'll remove that element while keeping its buffering message in our
list, and because of that never ever report buffering 100% as that element
will always be at a lower percentage.

This fixes e.g. seeking over Period boundaries in DASH and various other
issues when buffering happens between group switches.

Also use a new mutex for protecting the buffering messages. The object lock is
already used by gst_object_has_as_ancestor() and we need to use it now for
checking if the buffering message sender has the to-be-removed element as
ancestor.
2015-12-02 16:16:22 +02:00
Thomas Bluemel
2c62aad159 [PATCH] Fix a race condition accessing the decode_chain field.
Make sure that any access to the GstDecodeBin's decode_chain
field is protected using the EXPOSE_LOCK.  Also add a simple
reference counter to the GstDecodeChain structure so that when
the type_found signal fires it can hold onto the decode chain
even while the EXPOSE_LOCK is not held.  This should fix a
race condition if the type_found signal fires right in the
middle of a state change that messes with the same decode
chain.

https://bugzilla.gnome.org/show_bug.cgi?id=755260
2015-12-01 17:36:31 +00:00
Vincent Penquerc'h
870c6df489 decodebin: early out on pad-added when the pad is inactive
The pad may be recently deactivated if the element is switched
back down very quickly.

https://bugzilla.gnome.org/show_bug.cgi?id=752651
2015-12-01 17:36:31 +00:00
Vincent Penquerc'h
ee44337fc3 decodebin: lock the expose lock around decode_chain use
Helps with a crash in decodebin when quickly switching states.

https://bugzilla.gnome.org/show_bug.cgi?id=752651
2015-12-01 17:36:31 +00:00
Edward Hervey
d0eface01c decodebin: Properly deactivate ghostpads
Just setting the ghostpad as flushing wasn't enough. It needs to be
consistent on the internal proxypad also, otherwise you end up in
situations where:
* a pending buffer on the target pad triggers the sticky event
  propagation
* the default implementation sees that the proxypad is not flushing,
  so it tries to push it to the other pad (the actual ghostpad)
* the ghostpad is flushing, so returns FALSE
* the push_event function sees that pushing the event failed...
* ... and pending buffer push returns GST_FLOW_ERROR, instead of
  GST_FLOW_FLUSHING

By using gst_pad_set_active(FALSE), we ensure that both the ghostpad
and the proxypad are flushing/deactivated. The situation above will
no longer occur, and a GST_FLOW_FLUSHING will be returned.
2015-11-06 19:38:13 +01:00
Sebastian Dröge
36b80edb72 decodebin: Send SEEK events directly to adaptive streaming demuxers
This makes sure that they will always get SEEK events, even if we're currently
in the middle of a group switch (i.e. switching to another
representation/bitrate/etc).

https://bugzilla.gnome.org/show_bug.cgi?id=606382
2015-10-27 15:50:45 +02:00
Guillaume Desmottes
7d6b6b0313 decodebin: fix event leak
As stated in GST_PAD_PROBE_HANDLED's documentation, we are
supposed to unref the event before returning.

Fixes an event leak in the validate.hls.playback.play_15s.hls_bibbop
validate scenario.

https://bugzilla.gnome.org/show_bug.cgi?id=754459
2015-10-25 11:18:29 +00:00
Matthew Waters
44871680f0 decodebin: track the exposable pads through connect_pad
The logic introduced by
[d50b713: decodebin: set the decode pad target before setting elements to PAUSED]
to expose pads would only ever be able to possibly expose one (the last) pad per element.

Make it so that any exposable pads are able to be exposed rather than just the
last pad returned by connect_element.

https://bugzilla.gnome.org/show_bug.cgi?id=742924
2015-10-20 10:48:05 +03:00
Matthew Waters
94d81fc713 decodebin: return the possibly new chain in analyze_new_pad
In the case of analyzing a demuxer chain, analyze_new_pad may create
a new GstDecodeChain.  This was not propagated to the calling function which as
of [d50b713f decodebin: set the decode pad target before setting elements to PAUSED]
is now required to be able to expose the correct pad.

https://bugzilla.gnome.org/show_bug.cgi?id=742924
2015-10-20 10:47:45 +03:00
Sebastian Dröge
4d6aa0f831 decodebin/playbin/playsink/subtitleoverlay: Post async-done on state change failures
https://bugzilla.gnome.org/show_bug.cgi?id=756611
2015-10-19 11:06:25 +03:00
Matthew Waters
d50b713f44 decodebin: set the decode pad target before setting elements to PAUSED
Otherwise caps and context queries will disappear into nothing and therefore
fail.  With autoplug-query now actually working, users (such as playbin) can
proxy these queries to the selected video sink and be able to select an
more appropriate configuration.

https://bugzilla.gnome.org/show_bug.cgi?id=731204
2015-10-19 11:55:04 +11:00
Rajat Verma
267f4c2bad decodebin: free hidden groups at time of switching groups
hidden groups should be freed at time of switching groups to avoid memory use
from balloning up.

https://bugzilla.gnome.org/show_bug.cgi?id=755770
2015-10-02 17:02:21 +03:00
Sebastian Dröge
2727ca01f5 Revert "decodebin: Handle the preroll multi-queue size"
This reverts commit 5c8ef0ea05.
2015-08-18 18:47:22 +03:00