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 753385 - media: Adding multiple dynamic payload is broken
media: Adding multiple dynamic payload is broken
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-rtsp-server
unspecified
Other All
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-08 13:46 UTC by Nicolas Dufresne (ndufresne)
Modified: 2015-08-16 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
media: Only add fakesink once per pipeline (1.50 KB, patch)
2015-08-08 13:46 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
media-test: Test for multiple dynamic payload (4.22 KB, patch)
2015-08-08 15:14 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
Removing unnecessary assertion (821 bytes, patch)
2015-08-13 09:36 UTC, Francisco Velazquez
committed Details | Review

Description Nicolas Dufresne (ndufresne) 2015-08-08 13:46:38 UTC
This is not working, and seems to be broken due to multiple reason. Here's a
first patch to progressively address it.
Comment 1 Nicolas Dufresne (ndufresne) 2015-08-08 13:46:42 UTC
Created attachment 308940 [details] [review]
media: Only add fakesink once per pipeline

The intention is to prevent going PLAYING state before pads are created.
If there was mutilple dynamic payload, it would leak few fakesink and
actually prevent from ever reaching playing state.
Comment 2 Nicolas Dufresne (ndufresne) 2015-08-08 15:14:02 UTC
Created attachment 308950 [details] [review]
media-test: Test for multiple dynamic payload
Comment 3 Nicolas Dufresne (ndufresne) 2015-08-08 15:15:31 UTC
Actually, I wrote this test and didn't hit any other issue, maybe Francisco test was broken. This fixes the issue, and works if no dynpay is being used (unlike the reverted patch).
Comment 4 Nicolas Dufresne (ndufresne) 2015-08-08 15:18:58 UTC
Attachment 308940 [details] pushed as 707ac9c - media: Only add fakesink once per pipeline
Attachment 308950 [details] pushed as 3667e71 - media-test: Test for multiple dynamic payload
Comment 5 Francisco Velazquez 2015-08-13 09:36:33 UTC
Created attachment 309196 [details] [review]
Removing unnecessary assertion

test_media_multidyn_prepare works fine in my installation as well. Thank you, Nicolas.
Comment 6 Nicolas Dufresne (ndufresne) 2015-08-13 22:42:05 UTC
Review of attachment 309196 [details] [review]:

::: tests/check/gst/media.c
@@ -470,3 @@
   fail_unless (gst_rtsp_media_n_streams (media) == 0);
 
-  fail_unless (gst_rtsp_media_n_streams (media) == 0);

I agree, I notice when copying form the other test (where I think it's also useless. Can you update, thanks for spotting.
Comment 7 Nicolas Dufresne (ndufresne) 2015-08-13 22:49:36 UTC
Review of attachment 309196 [details] [review]:

commit 418e1fe09001f9a4defc291d9af221d598d9cf45
Author: Francisco Velazquez <francisv@ifi.uio.no>
Date:   Thu Aug 13 11:24:10 2015 +0200

    media-test: Removing unnecessary assertion
    
    https://bugzilla.gnome.org/show_bug.cgi?id=753385