GNOME Bugzilla – Bug 791353
interaudio: Add option to enable/disable gap flags in nullsample buffers
Last modified: 2018-11-03 14:16:15 UTC
If the interaudio channel runs out of data, nullsample buffers are created. interaudiosrc sets the GAP flag of these buffers. In some cases, it may be preferable to not set the flag (for example, to produce one continuous seamless stream even if the other side of the channel is currently paused). One such case would be a system with a producer and a consumer pipeline, linked together through an interaudio channel. The consumer pipeline always runs, while the producer pipeline may sometimes be stopped, seeking, paused etc. In such a case, it is necessary to make sure the consumer's interaudiosrc produces a continuous stream. GAP flags would introduce unnecessary resynchronization events inside the consumer pipeline's audio sink.
Created attachment 365206 [details] [review] nullsample gap gobject property patch
You can remove GAP flags on buffers with .. ! identity drop-buffer-flags=gap ! ... for what it's worth. I feel this shouldn't really be needed though, including this property. If the BUFFER_FLAG_GAP handling in baseaudiosink is problematic in some way, perhaps it needs to be revisited? It sounds like there might either be a bug, undesired side effect or oversight. It's only supposed to be an optimisation afaik.
Oh, it is needed. See my producer-consumer example above. You can't do that with gap events enabled. Also, identity does not do the same. It _drops_ buffers with the GAP flag. I do not want to drop any buffer; instead, I want to prevent the GAP flag from ever being added.
> Oh, it is needed. See my producer-consumer example above. You can't do that > with gap events enabled. Why not? I was saying that the resync problems might be a bug. > Also, identity does not do the same. It _drops_ buffers with the GAP flag. I > do not want to drop any buffer; instead, I want to prevent the GAP flag from > ever being added. Right, that's my bad, I was confused about what it does, sorry :)
> Why not? I was saying that the resync problems might be a bug. Hm, is it? Because this seems like a fundamental design decision in GstBaseSink. Look at https://cgit.freedesktop.org/gstreamer/gstreamer/tree/libs/gst/base/gstbasesink.c#n1964 how it sets do_sync to TRUE.
Sorry if I'm missing something fundamental here, which is quite possible. The link you sent is about GAP events, not buffers with the GAP flag set, no? This bug report is about the latter, isn't it? I have a vague memory that we transform GAP events into buffers with the GAP flag set somewhere in audiosink, but I couldn't actually find code for that so I'm probably confusing that with somewhere else.
Oh, right, my mistake. I confused the event with the buffer flag. I meant the latter. Anyway, here is code in audiobasesink that is affected by this flag: https://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst-libs/gst/audio/gstaudiobasesink.c#n1870 Note how this forces a resync.
Sure, but that was added fairly recently, so can hardly be a fundamental design decision :)
This has been part of an extensive GAP flag/event handling rework though. Perhaps Jan can comment too? After all, he wrote it :)
I think the reason is that I was testing scenarios where you switch streams and you have no audio for a while - so the demuxer is periodically sending GAP events to trigger preroll, and to catch up the audio chain with the 'current time'. The GAP events aren't continuous though - so you need to resync on the next buffer or risk being slightly out of sync with the video. Maybe in that case the next buffer should carry a DISCONT flag or something instead and the forced resync isn't essential. Or, it really fixed a problem and I've forgotten what after 2 years.
OK, thanks. My gut feeling is that changing this behavior may subtly break something else and it is easier and safer to add the gap-flag-removal patch instead. What do you think?
I think maybe we're mixing up GAP events and buffers with the GAP flag set here? Raw audio buffers with the GAP flag carry silence data, so elements can choose for optimisation purposes to not process them.
(In reply to Tim-Philipp Müller from comment #12) > I think maybe we're mixing up GAP events and buffers with the GAP flag set > here? > > Raw audio buffers with the GAP flag carry silence data, so elements can > choose for optimisation purposes to not process them. you're correct - the code I was referring to in baseaudiosink deals with converting *buffers with GAP flag* into *GAP events* to process them, and then marks the sink as needing a resync. So it's not about GAP events the sink receives but actually buffers, and I don't remember where they come from.
Ping. Since this doesn't really change anything in the interaudio's behavior by default, can we add this?
-- 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-bad/issues/638.