GNOME Bugzilla – Bug 613066
[flvmux] re-enable renamed/fixed is-live property
Last modified: 2010-06-08 12:14:45 UTC
The "is-live" property is maybe not ideally named, since the muxer doesn't sync against the clock. I understand it's been copied from asfmux, but maybe both should be changed. Ideally this property shouldn't be needed at all. The muxer should be able to figure out whether downstream is seakable or streaming or whatever and then act accordingly automatically. It should be possible to find out e.g. by checking the return value of pushing a newsegment event in bytes (to make downstream 'seek'), and/or we should make sinks implement the SEEKING query, or implement some other query. If all that fails, maybe the property should be called "write-index" or so?
is-live is now also used to decide if timestamps should simply wrap around or if EOS should be emitted: commit 96de71d74b3f4d2fc3c6a675dab86ac00a0c151a Author: Jan Urbański <wulczer@wulczer.org> Date: Tue Mar 16 23:32:45 2010 +0100 flvmux: don't put timestamps larger than G_MAXINT32 in the FLV tags For non-live input respond by pushing EOS, for live wrap the timestamps every G_MAXINT32 miliseconds. Fixes #613003.
Yeah, I copied it from asfmux. I spent a fair amount of time trying to make the detection automatic, but the muxer pushes a newsegment bytes event anyway (in gst_flv_mux_collected) and errors out if it fails... So it looks like everything downstream happily accepts that newsegment (I tried with tcpserversink, multifdsink and other things that are in fact not seekable, and they all just accepted it). I also tried pushing a seeking query down the srcpad, but both tcpserversink and multifdsink accepted it :| Ideally it would be fixed, yes. Oh, is-live is also used to decide whether to accumulate keyframes in a glist that then gets used to construct the index. Doing that for live broadcast would simply be a memory leak...
Still need to decide whether we want to rename the property or not.
I don't mind renaming it, but "write-index" is not a good name. "streaming", "broadcast", "live-streaming" are other (lame) choices. If someone could figure out how to reliable check if we're muxing live data I'd be very happy. Otherwise the property would have to stay, although if at some point it will be possible to determine the correct value automatically, would it be an API break to introduce a third value (although it's boolean now), like "auto" and make it the default? People will need a way to force it anyway, for testing etc.
If it's boolean now, it needs to stay a boolean (API/ABI guarantee). We can hack around that and add yet another property though if that should be needed, but if it can be avoided, all the better. I think it was just that 'live' has a very specific meaning in a GStreamer context. It would really be better to fix up these issues in basesrc/basesink etc. instead of just adding properties to work around that. Anyway, will collect some more opinions tomorrow. We can always punt it to the next release (which is planned for May already).
you can reliably check if upstream is live by doing a latency query (it contains the is-live boolean)
So I guess what bothers me is that the (a) the property is not very well-named, since the element itself is not actually 'live', and (b) that it's used both in a context of 'is the input live' (which should be queryable using a latency query; this is implemented pretty much everywhere) and 'is downstream seekable' and/or 'should we produce a file for streaming'.
I will prepare a patch to disable this property for the release, if no other solution is proposed. I think there's agreement that it's bad API as it stands now, and *something* needs fixing.
So what would disabling that property mean? I think we could just remove it from the element interface and treat is as always FALSE. The consequences will be: * for non-seekable downstream a big, empty blob will be sent in the onMetadata script tag, but the output will still be valid FLV, so that's not too bad * for live streaming the element will barf after G_MAXINT32 miliseconds (too lazy to compute how much is that in days), but for the typical use case of "create an FLV file" it's not a problem * for live streaming the element will keep building a keyframes index in memory, which will constitute a small memory leak. Since live streaming wno't work for very long anyway, that's also fine. So I'm for just making all references to mux->is_live a constant FALSE (and fixing it afterwards, of course).
commit f48bc702afa63d47004d8e80c04c410d38ed8b20 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sun Apr 25 21:19:33 2010 +0100 flvmux: hide is-live property for release At the very least it needs a better/less wrong name. See #613066. Dropping severity.
how about: indexed (boolean) True: outputs the index same as is-live=False False: does not output the index and is suitable for live streaming
ping...no comments about my proposed property name change?
The way I see it it should be possible to automatically detect the correct value, but there will always need to be a knob to force it in case the detection fails (because some elements don't reply correctly to latency queries) or for testing. How about a property that can take three values: always try to rewrite index, never try to rewrite index, autodetect. And since autodetection code is not written yet, make the default rewriting the index (which is the current behaviour). No good proposal for a name of that property though.
commit 44c911d255a7a3596c2cdbf0d779cce34684d68a Author: Zaheer Abbas Merali <zaheerabbas@merali.org> Date: Tue Jun 8 14:09:00 2010 +0200 flvmux: Add indexed property to replace disabled is-live. Add indexed property to be the negation of what the disabled is-live propert was. Fixes bug #613066.