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 764260 - flvdemux: Expose pads as early as possible and use GstStreams API
flvdemux: Expose pads as early as possible and use GstStreams API
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 743326 747616 773511 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-03-27 16:20 UTC by Yann Jouanin
Modified: 2018-11-03 15:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to add no-more-pads-threshold property (7.78 KB, patch)
2016-03-27 16:20 UTC, Yann Jouanin
rejected Details | Review
another suggestion for the same thing (5.11 KB, patch)
2016-03-28 17:31 UTC, Håvard Graff (hgr)
rejected Details | Review

Description Yann Jouanin 2016-03-27 16:20:21 UTC
Created attachment 324837 [details] [review]
Patch to add no-more-pads-threshold property

Currently when first video packet is more than 6 seconds delayed from first audio packet (or vice versa), the filter sends a no-more-pads events.

Because I encountered some case where this delay is not enough to read the stream (long GOP, still image), I wrote a patch to switch from a constant to a property

add no-more-pads-threshold property , default is set to 6 (like the previous constant)
Comment 1 Tim-Philipp Müller 2016-03-27 16:45:57 UTC
What is the goal here actually?

I'd rather get rid of the hard-coded threshold entirely (if possible).
Comment 2 Yann Jouanin 2016-03-27 16:48:04 UTC
I think it is the case, the threshold is no more hard-coded
Comment 3 Tim-Philipp Müller 2016-03-27 17:00:36 UTC
What I meant is: I'd rather just make it work without anyone having to adjust any properties. If a property has to be set that usually indicates a failure of just making something work out of the box :)
Comment 4 Yann Jouanin 2016-03-27 17:09:16 UTC
Ok, I see your point and agree.

Based on this comment in the code : 

    /* We only emit no more pads when we have audio and video. Indeed we can
     * not trust the FLV header to tell us if there will be only audio or
     * only video and we would just break discovery of some files */

I deduced the 'no-more-pads' events was not optional.

If it is not the case, we can get rid of the threshold.
Additionnaly, I think the value of this threshold only have to be increased only on a few streams, unfortunatly I encountered this case on some live streams.
Comment 5 Nicolas Dufresne (ndufresne) 2016-03-28 12:29:45 UTC
Apparently, there is a massive amount of files in the wild that are only audio or only video, but advertise both in the header. I believe (I didn't wrote that) that this hack was an attempted to support these file. Without the threshold, the buffer will fill up and the pipeline will stall.

That being said, I consider this hack harmful in live mode. Stalls on high interleaving, or missing expected stream is something that may happen with any demuxers. I believe there exist some other way to workaround, using gaps even and fake caps in other demuxers.
Comment 6 Yann Jouanin 2016-03-28 14:36:36 UTC
You are right, this hack is harmful in live mode.
The issue I encountered is the pads and caps are not sent (nor created) based on the header in RTMP/FLV.

This means even if the information in RTMP header are correct, the video pad is not created, and if the first video packet does not appear before the no-more-pads-threshold, the pad will not be present and further video packets will be ignored.
Comment 7 Håvard Graff (hgr) 2016-03-28 17:15:07 UTC
I did the same thing a couple of days ago: https://github.com/pexip/gst-plugins-good/commit/97a933837b1f50ec6501c1e9d63e7c1791802a34
Comment 8 Yann Jouanin 2016-03-28 17:27:11 UTC
Hello Havard, this is quite the same code.
What was your use case?
Comment 9 Håvard Graff (hgr) 2016-03-28 17:29:09 UTC
(In reply to Yann Jouanin from comment #8)
> Hello Havard, this is quite the same code.
> What was your use case?

After a long discussion with stormer on #gstreamer two days ago. We are using rtmp live and can't have this. I also keep a local patch to set it default to -1 to have it disabled.
Comment 10 Yann Jouanin 2016-03-28 17:31:42 UTC
I agree, one other point for me is the lack of information transfer between rtmpsrc and flvdemux.

rtmpsrc uses librtmp but which is also getting information from the stream
Comment 11 Håvard Graff (hgr) 2016-03-28 17:31:49 UTC
Created attachment 324886 [details] [review]
another suggestion for the same thing
Comment 12 Sebastian Dröge (slomo) 2016-03-28 18:27:21 UTC
There are also some other bugs about this. With decodebin3 this should work better, AFAIU we don't really need no-more-pads there anymore and can just add the pads once their first data arrives.
Comment 13 Yann Jouanin 2016-03-28 20:55:50 UTC
Do you suggest we totally get rid of this behaviour?
Shouldn't we rely also on header for live stream? Can't we take advantage of what librtmp reads?
Comment 14 Sebastian Dröge (slomo) 2016-03-29 07:19:43 UTC
We should try to make use of as much information as we can, yes. But apart from that we should probably just get rid of this behaviour for the other cases.
Comment 15 Yann Jouanin 2016-03-29 14:16:15 UTC
What would be the best way to transfer information from rtmpsrc to flvdemux? Can we push it through more specific caps?
Comment 16 Sebastian Dröge (slomo) 2017-01-16 14:41:44 UTC
*** Bug 773511 has been marked as a duplicate of this bug. ***
Comment 17 Sebastian Dröge (slomo) 2017-01-16 14:42:09 UTC
*** Bug 747616 has been marked as a duplicate of this bug. ***
Comment 18 Sebastian Dröge (slomo) 2017-01-16 14:42:33 UTC
*** Bug 743326 has been marked as a duplicate of this bug. ***
Comment 19 Sebastian Dröge (slomo) 2017-01-16 14:45:52 UTC
So let's retarget this bug a bit. With the playbin3/decodebin3/GstStreams new world order, we can now handle late added streams just fine (if the application does not fall apart because of that).

What should be done here now is that
a) flvdemux should not wait for too long (500ms? 100ms?) before exposing pads, and only expose pads if a buffer was found (otherwise we 1) don't know the format, and 2) don't know if there will ever be data)
b) flvdemux should use the GstStreams API to signal the streams, and also signal a newly (late) found streams with that

Then all these use cases should be covered just fine.
Comment 20 Edward Hervey 2018-05-12 07:12:41 UTC
Yann, Havard, want to provide patches making flvdemux streams-aware ?
Comment 21 GStreamer system administrator 2018-11-03 15:08:19 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/263.