After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769521 - rtspmedia unit test failure: 'gst_rtsp_media_n_streams (media) == 2' failed
rtspmedia unit test failure: 'gst_rtsp_media_n_streams (media) == 2' failed
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
git master
Other Linux
: Normal normal
: 1.12.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-04 15:01 UTC by Kseniya Vasilchuk
Modified: 2017-11-21 07:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
log of another error 1 (55.71 KB, text/plain)
2016-08-10 13:59 UTC, Kseniya Vasilchuk
  Details
log 2 (56.37 KB, text/plain)
2016-08-10 13:59 UTC, Kseniya Vasilchuk
  Details
rtsp-media: Handle multiple dynamic elements (3.62 KB, patch)
2017-11-20 08:38 UTC, Edward Hervey
committed Details | Review
rtsp-media: Don't unblock with remaining dynamic payloaders (1.55 KB, patch)
2017-11-20 17:41 UTC, Edward Hervey
committed Details | Review

Description Kseniya Vasilchuk 2016-08-04 15:01:47 UTC
Running suite(s): rtspmedia
85%: Checks: 7, Failures: 1, Errors: 0
gst/media.c:434:F:general:test_media_multidyn_prepare:0: Assertion 'gst_rtsp_media_n_streams (media) == 2' failed

The test doesn't fail every time. The best way to reproduce is launch as 'make gst/media.forever'.
Comment 1 Tim-Philipp Müller 2016-08-07 19:51:45 UTC
I was able to reproduce this with git master, but it took about 10-15 minutes of .forever for me to fail. Haven't been able to get it to fail with GST_DEBUG debug logging enabled yet, you?
Comment 2 Kseniya Vasilchuk 2016-08-10 13:59:38 UTC
Created attachment 333065 [details]
log of another error 1

(In reply to Tim-Philipp Müller from comment #1)
> I was able to reproduce this with git master, but it took about 10-15
> minutes of .forever for me to fail. Haven't been able to get it to fail with
> GST_DEBUG debug logging enabled yet, you?

No, I haven't, but when I tried I got:

85%: Checks: 7, Failures: 1, Errors: 0
gstcheck.c:79:F:general:test_media_dyn_prepare:0: Unexpected critical/warning: adding flushing pad 'src_1' to running element 'tee3', you need to use gst_pad_set_active(pad,TRUE) before adding it.

and

85%: Checks: 7, Failures: 1, Errors: 0
gst/media.c:76:F:general:test_launch:0: Assertion 'g_str_equal (str, "npt=5-")' failed

logs in attachment
Comment 3 Kseniya Vasilchuk 2016-08-10 13:59:54 UTC
Created attachment 333066 [details]
log 2
Comment 4 Edward Hervey 2017-11-17 10:38:45 UTC
still happens on master.
Comment 5 Edward Hervey 2017-11-17 11:13:49 UTC
Essentially it's a race between:
* payloaders adding their dynamic pads
* pipeline being prerolled

If the pipeline is prerolled, "new" pads will be ignored (and streams won't be created).

The pipeline manages to preroll because the moment *any* payloader emits no-more-pads ... it removes the fakesink and causes the pipeline to preroll.

Maybe we should wait for *all* dynamic elements to have posted no-more-pads before doing that ?
Comment 6 Sebastian Dröge (slomo) 2017-11-17 11:19:39 UTC
Sounds reasonable
Comment 7 Edward Hervey 2017-11-20 08:38:54 UTC
Created attachment 364030 [details] [review]
rtsp-media: Handle multiple dynamic elements

If we have more than one dynamic payloader in the pipeline, we need
to wait until the *last* one emits 'no-more-pads' before switching
to PREPARED.

Failure to do so would result in a race where some of the streams
wouldn't properly be prepared
Comment 8 Edward Hervey 2017-11-20 08:39:39 UTC
Comment on attachment 364030 [details] [review]
rtsp-media: Handle multiple dynamic elements

commit 2386e91c3642f3a83de6a74c981e0b9ac85a3dd1 (HEAD -> master, origin/master, origin/HEAD)
Author: Edward Hervey <edward@centricular.com>
Date:   Mon Nov 20 09:32:07 2017 +0100

    rtsp-media: Handle multiple dynamic elements
    
    If we have more than one dynamic payloader in the pipeline, we need
    to wait until the *last* one emits 'no-more-pads' before switching
    to PREPARED.
    
    Failure to do so would result in a race where some of the streams
    wouldn't properly be prepared
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769521
Comment 9 Edward Hervey 2017-11-20 09:02:35 UTC
And backported to 1.12 also.
Comment 10 Edward Hervey 2017-11-20 10:45:45 UTC
Too good to be true, it still fails sometimes.
Comment 11 Edward Hervey 2017-11-20 17:41:05 UTC
Created attachment 364063 [details] [review]
rtsp-media: Don't unblock with remaining dynamic payloaders

If we still have some dynamic paylaoders which haven't posted
no-more-pads yet, don't go to PREPARED if one of the streams
blocked.

The risk was that we would end up not exposing/using all specified
streams.

The downside is that if you have _multiple_ _live_ _dynamic_ payloaders
then it will take a bit more time to start. But only if those 3
conditions are present.
Comment 12 Mathieu Duponchelle 2017-11-20 17:54:10 UTC
Review of attachment 364063 [details] [review]:

lgtm
Comment 13 Edward Hervey 2017-11-21 07:01:15 UTC
Comment on attachment 364063 [details] [review]
rtsp-media: Don't unblock with remaining dynamic payloaders

commit 6371f2fc294ef427351a470532c27bfe2caa9acb (HEAD -> master, origin/master, origin/HEAD)
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Mon Nov 20 18:30:19 2017 +0100

    rtsp-media: Don't unblock with remaining dynamic payloaders
    
    If we still have some dynamic paylaoders which haven't posted
    no-more-pads yet, don't go to PREPARED if one of the streams
    blocked.
    
    The risk was that we would end up not exposing/using all specified
    streams.
    
    The downside is that if you have _multiple_ _live_ _dynamic_ payloaders
    then it will take a bit more time to start. But only if those 3
    conditions are present.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769521