[Bug 763326] New: gst-launch of 8-channel deinterleave pipeline stalls

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Bug ID: 763326
Summary: gst-launch of 8-channel deinterleave pipeline stalls
Classification: Platform
Product: GStreamer
Version: git master
OS: Linux
Status: NEW
Severity: normal
Priority: Normal
Component: gstreamer (core)
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@toolsonair.com
QA Contact: gstreamer-***@lists.freedesktop.org
GNOME version: ---

The following launch line hang on my machine, i.e. never transitions to
PLAYING:

gst-launch-1.0 -e \
audiotestsrc is-live=TRUE ! "audio/x-raw,rate=48000,channels=8" ! queue !
deinterleave name=deinter \
deinter.src_0 ! queue ! fakesink \
deinter.src_1 ! queue ! fakesink \
deinter.src_2 ! queue ! fakesink \
deinter.src_3 ! queue ! fakesink \
deinter.src_4 ! queue ! fakesink \
deinter.src_5 ! queue ! fakesink \
deinter.src_6 ! queue ! fakesink \
deinter.src_7 ! queue ! fakesink

This has been observed on a 12-core CPU. Limiting the amount of CPU cores used
to four or less (i.e. taskset -c 0-3 gst-launch-1.0 ...) makes everything work
again. This suggests the bug to be a race condition.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #1 from Heinrich Fink <***@toolsonair.com> ---
Created attachment 323401
--> https://bugzilla.gnome.org/attachment.cgi?id=323401&action=edit
level 6 debug log

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #2 from Heinrich Fink <***@toolsonair.com> ---
Created attachment 323402
--> https://bugzilla.gnome.org/attachment.cgi?id=323402&action=edit
pipeline graph

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@coaxion.net

--- Comment #3 from Sebastian Dröge (slomo) <***@coaxion.net> ---
The pipeline graph is not very useful. Would be better to get the one when the
pipeline stalled. This now is before deinterleave received any buffer at all :)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #4 from Sebastian Dröge (slomo) <***@coaxion.net> ---
This pipeline never creates any buffer it seems.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #5 from Heinrich Fink <***@toolsonair.com> ---
the pipeline already fails with two or more downstream connections (a single
one works), i.e. that will stall already

gst-launch-1.0 -e \
audiotestsrc is-live=TRUE ! "audio/x-raw,rate=48000,channels=8" ! queue !
deinterleave name=deinter \
deinter.src_0 ! queue ! fakesink \
deinter.src_1 ! queue ! fakesink

I am updating the debug log with the output of this pipeline, might be easier
to read with less elements in the play.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Heinrich Fink <***@toolsonair.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #323401|0 |1
is obsolete| |

--- Comment #6 from Heinrich Fink <***@toolsonair.com> ---
Created attachment 323404
--> https://bugzilla.gnome.org/attachment.cgi?id=323404&action=edit
level 6 debug log

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Heinrich Fink <***@toolsonair.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #323402|0 |1
is obsolete| |

--- Comment #7 from Heinrich Fink <***@toolsonair.com> ---
Created attachment 323405
--> https://bugzilla.gnome.org/attachment.cgi?id=323405&action=edit
pipeline graph

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #8 from Sebastian Dröge (slomo) <***@coaxion.net> ---
The audiotestsrc task is blocked by waiting for the ALLOCATION query to return
from the queue. And the queue task seems to be busy forever with caps query
things.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #9 from Heinrich Fink <***@toolsonair.com> ---
<slomo> put a tee before deinterleave
still stalls, but less often

<slomo> add a tee after audiotestsrc directly?
still stalls, but less often

not using GST_DEBUG_DUMP_DOT_DIR in the environment also makes it less likely
to hang (but still happens)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #10 from Heinrich Fink <***@toolsonair.com> ---
Created attachment 323408
--> https://bugzilla.gnome.org/attachment.cgi?id=323408&action=edit
gdb stacktrace of stalled pipeline

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #11 from Heinrich Fink <***@toolsonair.com> ---
Created attachment 323409
--> https://bugzilla.gnome.org/attachment.cgi?id=323409&action=edit
gdb stacktrace of stalled pipeline (full)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #12 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Ok so it's a deadlock

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #13 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 324129
--> https://bugzilla.gnome.org/attachment.cgi?id=324129&action=edit
0001-deinterleave-Use-GstIterator-for-iterating-all-pads-.patch

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #14 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Created attachment 324130
--> https://bugzilla.gnome.org/attachment.cgi?id=324130&action=edit
0002-deinterleave-Return-the-current-caps-on-the-srcpads-.patch

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #15 from Sebastian Dröge (slomo) <***@coaxion.net> ---
The second patch is not actually needed but simplifies all the caps query logic
a bit.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #16 from Heinrich Fink <***@toolsonair.com> ---
Thanks, your patches fix my deadlock, i.e. I wasn't able to reproduce the
deadlock anymore.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #324130|none |accepted-commit_after_freez
status| |e

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #324129|none |accepted-commit_after_freez
status| |e

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #324129|accepted-commit_after_freez |accepted-commit_now
status|e |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #324129|accepted-commit_now |committed
status| |

--- Comment #17 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Comment on attachment 324129
--> https://bugzilla.gnome.org/attachment.cgi?id=324129
0001-deinterleave-Use-GstIterator-for-iterating-all-pads-.patch

commit 605175b8c4ca07932f15a5035072a31314dfa5a6
Author: Sebastian Dröge <***@centricular.com>
Date: Wed Mar 16 20:18:41 2016 +0200

deinterleave: Use GstIterator for iterating all pads instead of manually
iterating them while holding the object lock all the time

Doing queries while holding the object lock is a bit dangerous, and in this
case causes deadlocks.

https://bugzilla.gnome.org/show_bug.cgi?id=763326

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Component|gstreamer (core) |gst-plugins-good

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #324130|accepted-commit_after_freez |committed
status|e |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|git master |1.9.1

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #18 from ***@imgtec.com ---
Review of attachment 324129:
--> (https://bugzilla.gnome.org/review?bug=763326&attachment=324129)

::: gst/interleave/deinterleave.c
@@ -568,42 +569,62 @@
- GstPad *ourpad = GST_PAD (l->data);
- GstCaps *peercaps = NULL, *ourcaps;
- GstCaps *templ_caps = gst_pad_get_pad_template_caps (ourpad);
... 39 more ...
+ switch (res) {
+ /* Only ask for peer caps for other pads than pad
+ ourcaps = gst_caps_copy (templ_caps);
... 59 more ...

Shouldnt this line be not present as ret is already initialised with any caps.
Also if we keep empty then in case of resync, it try to intersect and result in
always empty.

In our setup, which uses appsrc with deinterleave element, this line causes
caps to return empty (sometime) which in turn in gstbasetranform.c:2913, cause
it to fail the check which then return empty caps and caps negotiation fails.
Is changing to empty instead of any is correct here?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

***@imgtec.com changed:

What |Removed |Added
----------------------------------------------------------------------------

Post by "GStreamer" (GNOME Bugzilla)
gstbasetranform.c:2913

at this line, should we also check for empty peercaps also along with peercaps
not NULL check? Is this scenario possible or is it always expected that element
should return either NULL caps or valid?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #20 from ***@imgtec.com ---
i see it is already fixed here, sorry for spam
https://cgit.freedesktop.org/gstreamer/gst-plugins-good/commit/gst/interleave/deinterleave.c?id=4f426f6f54987dc28c59b52faf419cd6ac5b13f2

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #21 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Is anything left to be done here?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #22 from ***@imgtec.com ---
No, but what do you think of this?

gstbasetranform.c:745

at this line, should we also check for empty peercaps also along with peercaps
not NULL check? Is this scenario possible or is it always expected that element
should return either NULL caps or valid?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #23 from Sebastian Dröge (slomo) <***@coaxion.net> ---
If they're empty, they stay empty, no? Might just be a fast-path to error out
faster here. If you want to provide a patch, just do so :)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #24 from ***@imgtec.com ---
Created attachment 345310
--> https://bugzilla.gnome.org/attachment.cgi?id=345310&action=edit
0001-dont-intersect-if-peercaps-is-empty_new

Check of empty peercaps as it should be similar to null peercaps.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #345310|none |needs-work
status| |

--- Comment #25 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Comment on attachment 345310
--> https://bugzilla.gnome.org/attachment.cgi?id=345310
0001-dont-intersect-if-peercaps-is-empty_new

This does not apply to latest GIT master, and would also cause the empty
peercaps to not be unreffed and leaked.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

***@imgtec.com changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #345310|needs-work |none
status| |
Attachment #345310|0 |1
is obsolete| |

--- Comment #26 from ***@imgtec.com ---
Created attachment 345712
--> https://bugzilla.gnome.org/attachment.cgi?id=345712&action=edit
new patch for fixing empty peercaps usecase

New patch created from master

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

and would also cause the empty peercaps to not be unreffed and leaked.

This seems not the case on master as at line 724, peercaps is being freed. All
path seems to go through it.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #345712|none |needs-work
status| |

--- Comment #28 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Review of attachment 345712:
--> (https://bugzilla.gnome.org/review?bug=763326&attachment=345712)

::: libs/gst/base/gstbasetransform.c
@@ +672,3 @@
gst_caps_unref (peerfilter);

+ if (peercaps && !gst_caps_is_empty (peercaps)) {

This is not equivalent though. If the caps are empty, now we would work with
the template caps. Before we would work with the empty caps and error out.

I think you want to add a "if (gst_caps_is_empty(peercaps)) goto done;" above
instead

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

I think you want to add a "if (gst_caps_is_empty(peercaps)) goto done;" above instead

This will cause caps to return NULL which will still fail linking. we would
need to return some caps either it could be peerfilter as in case of peerfilter
empty or we return template as it is now. I think empty caps is kind of similar
to NULL caps practically?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #30 from ***@imgtec.com ---
any update on this?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@zen.co.uk

--- Comment #31 from Tim-Philipp Müller <***@zen.co.uk> ---
Probably best if you clone this bug and/or open a new bug for your patch.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #32 from ***@imgtec.com ---
Thanks tim, create bugid 779055

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=763326

--- Comment #33 from ***@imgtec.com ---
link https://bugzilla.gnome.org/show_bug.cgi?id=779055