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 665814 - mpegtsdemux: add a property to control whether to signal no-more-pads
mpegtsdemux: add a property to control whether to signal no-more-pads
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
unspecified
Other All
: Normal blocker
: 0.10.23
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-12-08 19:17 UTC by Vincent Penquerc'h
Modified: 2012-02-08 18:13 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mpegtsdemux: add a property to control whether to signal no-more-pads (5.10 KB, patch)
2011-12-08 19:17 UTC, Vincent Penquerc'h
rejected Details | Review
mpegtsdemux: do not emit no-more-pads (2.68 KB, patch)
2012-02-03 11:10 UTC, Vincent Penquerc'h
committed Details | Review

Description Vincent Penquerc'h 2011-12-08 19:17:55 UTC
mpegtsdemux: add a property to control whether to signal no-more-pads
Comment 1 Vincent Penquerc'h 2011-12-08 19:17:57 UTC
Created attachment 203107 [details] [review]
mpegtsdemux: add a property to control whether to signal no-more-pads

Unconditionally signalling no-more-pads will apparently break some cases
where more streams are added partway through.
Comment 2 Tim-Philipp Müller 2011-12-09 10:59:22 UTC
Not sure if a property is the way to go there
Comment 3 Vincent Penquerc'h 2011-12-09 11:23:08 UTC
I have to agree with that comment. I was mainly interested in avoiding a regression with the original patch. I wouldn't be opposed to reverting it altogether (and the property one too, then) if preferred.
Comment 4 Sebastian Dröge (slomo) 2011-12-13 14:01:42 UTC
Currently no-more-pads is emitted when the PAT is parsed if I understand it correctly. Is it allowed to add new streams to a TS afterwards?

A property is definitely not the solution here, either we need to emit no-more-pads only in situations where we're really sure that no new streams can be added afterwards (is this possible with TS?) or revert the change.
Comment 5 Raimo Järvi 2011-12-14 09:45:14 UTC
(In reply to comment #4)
> Currently no-more-pads is emitted when the PAT is parsed if I understand it
> correctly. Is it allowed to add new streams to a TS afterwards?

Did you mean PMT? In a live TS new streams can be added at any time, e.g. audio stream with a new language. When streams are added (or removed), a new version of PMT is sent.
Comment 6 Vincent Penquerc'h 2012-01-05 11:28:55 UTC
So... the best thing to do is to revert the patches (or part of patches) that call no-more-pads, as well as this property one ? That will re-break some of the stuff, but these are no regressions.
Unless we can have a "actually-i-do-have-some-more-pads-after-all" call, since the no-more-pads is essentially a "go ahead and play", so it would seem to make sense to allow a "hold on, we're rejigging streams" call. It's probably very non trivial though :/
Comment 7 Vincent Penquerc'h 2012-02-02 18:18:09 UTC
Just removing the calls to gst_element_no_more_pads plays well with playbin2 on local files. hI'm tempted to push this as a not too intrusive change that would avoid precluding future new pads.
Comment 8 Sebastian Dröge (slomo) 2012-02-03 08:20:54 UTC
You mean removing the no-more-pads calls but adding pads later? Sure, why not?
Comment 9 Vincent Penquerc'h 2012-02-03 11:10:18 UTC
Created attachment 206679 [details] [review]
mpegtsdemux: do not emit no-more-pads

Doing so may fix some things, but breaks others (new streams being
added in the future).
Comment 10 Vincent Penquerc'h 2012-02-03 11:11:22 UTC
Here we go, then.
A bit underwhelming, but safe I guess.
Will push soon if no comments.
Comment 11 Vincent Penquerc'h 2012-02-03 17:39:42 UTC
In it goes, then:

commit 2eef9828d6735572e5f1bfd0409108e40963696c
Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk>
Date:   Fri Feb 3 11:08:48 2012 +0000

    mpegtsdemux: do not emit no-more-pads
    
    Doing so may fix some things, but breaks others (new streams being
    added in the future).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=665814
Comment 12 Tvrtko Ursulin 2012-02-08 15:51:09 UTC
Does this re-break 659924?
Comment 13 Vincent Penquerc'h 2012-02-08 18:13:04 UTC
Hmm, it probably would, yes, as playbin2 would not get the signal anymore, so would have to rely on the queue filling up to decide to stop waiting.

For your use case, you may be better off not using this patch, since (at least from what I saw while testing) the addition of new streams appears when switching stations, and in this case the "detect dry source" message would allow you to restart streaming, in theory shielding you from the issue.

A proper solution would probably involve calling no-more-pads, and having new a more-pads-incoming signal to tell playbin2 to get back to a preroll-like phase, but this seems like a non trivial task.