xpra icon
Bug tracker and wiki

Opened 4 months ago

Closed 3 months ago

Last modified 6 weeks ago

#1617 closed enhancement (fixed)

detect silence and avoid sending empty packets

Reported by: Antoine Martin Owned by: Antoine Martin
Priority: major Milestone: 2.2
Component: sound Version: trunk
Keywords: Cc:

Description

gstreamer's cutter element: Analyses the audio signal for periods of silence. The start and end of silence is signalled by bus messages named cutter - we could use this to tell the receiver to pause and resume.

Not sure if we want to / should wait for the pipeline to drain first.

See also: #1212.

Change History (10)

comment:1 Changed 4 months ago by Antoine Martin

Status: newassigned

Testing using gst-launch-1.0, I found that the opus codec just refuses to work with cutter in the pipeline

gst-launch-1.0 -m \
    pulsesrc ! queue ! cutter \
    opusenc !
    fakesink

All the other codecs+muxers work.
(adding audioconvert after cutter as needed, see ENCODER_NEEDS_AUDIOCONVERT=("flacenc", "wavpackenc"))

Here's the error:

$ gst-launch-1.0 -m  pulsesrc ! queue ! cutter ! audioconvert ! opusenc !  fakesink
(...)
New clock: GstPulseSrcClock
Got message #41 from object "audiosrcringbuffer0" (stream-status): GstMessageStreamStatus, type=(GstStreamStatusType)GST_STREAM_STATUS_TYPE_ENTER, owner=(GstElement)"\(GstPulseSrc\)\ pulsesrc0", object=(GThread)NULL;
Got message #43 from element "opusenc0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_PAUSED, new-state=(GstState)GST_STATE_PLAYING, pending-state=(GstState)GST_STATE_VOID_PENDING;
Got message #44 from element "pulsesrc0" (latency): no message details
Redistribute latency...
Got message #45 from element "audioconvert0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_PAUSED, new-state=(GstState)GST_STATE_PLAYING, pending-state=(GstState)GST_STATE_VOID_PENDING;
Got message #46 from element "cutter0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_PAUSED, new-state=(GstState)GST_STATE_PLAYING, pending-state=(GstState)GST_STATE_VOID_PENDING;
Got message #48 from element "queue0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_PAUSED, new-state=(GstState)GST_STATE_PLAYING, pending-state=(GstState)GST_STATE_VOID_PENDING;
Got message #49 from element "pulsesrc0" (state-changed): GstMessageStateChanged, old-state=(GstState)GST_STATE_PAUSED, new-state=(GstState)GST_STATE_PLAYING, pending-state=(GstState)GST_STATE_VOID_PENDING;
Got message #55 from element "pulsesrc0" (error): GstMessageError, gerror=(GError)NULL, debug=(string)"gstbasesrc.c\(2939\):\ gst_base_src_loop\ \(\):\ /GstPipeline:pipeline0/GstPulseSrc:pulsesrc0:\012streaming\ stopped\,\ reason\ not-negotiated\ \(-4\)", details=(structure)"details\,\ flow-return\=\(int\)-4\;";
ERROR: from element /GstPipeline:pipeline0/GstPulseSrc:pulsesrc0: Internal data stream error.
Additional debug info:
gstbasesrc.c(2939): gst_base_src_loop (): /GstPipeline:pipeline0/GstPulseSrc:pulsesrc0:
streaming stopped, reason not-negotiated (-4)
(..)

comment:2 Changed 4 months ago by Antoine Martin

Owner: changed from Antoine Martin to J. Max Mena
Status: assignednew

This seems to work well enough (with some caveats below), so added in r16675 (see commit for details).

Since this doesn't work with opus (for whatever reason - and maybe other codecs too..), maybe we should switch to another codec as default? (and file an upstream ticket to figure out why / how to fix this)

There was just one annoying limitation: the cutter element doesn't fire initially if the sound level starts already below the threshold. So as of r16676 we now play a short bell sample whenever a client starts forwarding sound. This ensures that the sound level goes above the threshold at least once.

We could also have added a level element in the pipeline and duplicate the "cutter" code to figure out what the initial state should be... but this wouldn't be 100% reliable.


With "-d gstreamer", when the stream goes near silent we see:

sound source on_message: <Gst.Message object at 0x7f1342639258 (GstMessage at 0x556c2f2d0200)>
sound source cutter message, above=False, min-timestamp=0, max-timestamp=63440000000
sound source cutter: skipping buffer with pts=63450000000 (max-timestamp=63440000000)
(...)

Followed by constant "skipping buffer" messages.

And when the level rises again, we see:

(skipping buffer repeated)
sound source on_message: <Gst.Message object at 0x7f1342639258 (GstMessage at 0x556c2f2d0100)>
sound source cutter: skipping buffer with pts=64150000000 (max-timestamp=63440000000)
sound source cutter: skipping buffer with pts=64160000000 (max-timestamp=63440000000)
sound source cutter message, above=True, min-timestamp=64340000000, max-timestamp=0
sound source cutter: skipping buffer with pts=64170000000 (min-timestamp=64340000000)
(..)
sound source cutter: skipping buffer with pts=64330000000 (min-timestamp=64340000000)

@maxmylyn: ready for you to test (just don't use one of the broken codecs like opus). When no sound is being played, the sound subprocess should be mostly idle and there should not be any sound network traffic, the client sound buffer level should be stable near the minimum limit.
When the volume reaches 2%, it should start forwarding again.

Note: there seems to be some problems (at least with the win32 builds) with some of the codecs, I'm not sure which ones are having problems because of this change and which ones have just bitrotted. flac, wav, wavpack and mp3 (on Linux) worked OK during testing. opus and vorbis did not..

Last edited 3 months ago by Antoine Martin (previous) (diff)

comment:3 Changed 4 months ago by J. Max Mena

Owner: changed from J. Max Mena to Antoine Martin

I'm seeing the same behavior as you noted (trunk r16677 Fedora 25 server and client) - flac/wav/wavpack/mp3 seem to behave nicely. Do you want me to go ahead and close this or do you want to do more work and get it working with opus and vorbis as well?

I'm assuming it's the latter.

comment:4 Changed 4 months ago by J. Max Mena

I guess I should clarify a bit. I'm fine with just switching the defaults, especially now that MP3 is a free codec. I think it would be nice if we could get this for all the sound codecs(okay I'm mostly hoping for Vorbis), but it's definitely not necessary if it's a ton of work.

comment:5 Changed 3 months ago by Antoine Martin

Resolution: fixed
Status: newclosed

I've tried everything I could think of (re-ordering the pipeline, caps, etc) but I just cannot get "vorbis" or "opus" to play nice with the "cutter" element.
So r16804 disables cutter with "vorbis" and makes "mp3" the default again. r16805 also disables it for "wavpackenc" and "avenc_aac".

I've asked the gstreamer-devel mailing list: cutter with vorbis or opus: not-negotiated error

If and when we get an answer, I'll update the code accordingly if possible. Closing for now.

comment:6 Changed 3 months ago by Antoine Martin

XPRA_CUTTER_THRESHOLD=VALUE (defaults to 0.02=2%) can be used to control the maximum level required before cutter will kick in.
Setting it to 0 will disable cutter.

Last edited 3 months ago by Antoine Martin (previous) (diff)

comment:7 Changed 3 months ago by Antoine Martin

Related changes:

  • r16831: fix some debug level gstreamer internal warnings with timestamps
  • r16828: re-enable "vorbis" and "vorbis+ogg" with gstreamer versions 1.12 and later (seems to work fine - not tried older versions), also remove outdated code
  • r16829: better location for volume element
  • r16830: fix opus so it can use cutter! (got a reply on the gstreamer mailing list which put me on the right track)
  • r16832 + r16833: make opus the default again, but blacklist all codecs that use a muxer as this still doesn't work with cutter (no idea why)

More information on the muxer problem:

  • with vorbis and webmmux / matroskademux, the error shows up as:
    gst-stream-error-quark: Could not demultiplex stream. (9)
    
  • with vorbis and oggmux / oggdemux, the error shows up as:
    Could not decode stream. gstoggdemux.c(4556)
    gst_ogg_demux_handle_page () /GstPipeline pipeline0/GstOggDemux, oggdemux0, unknown ogg pad for serial 7dfefd6e detected
    
Last edited 3 months ago by Antoine Martin (previous) (diff)

comment:8 Changed 3 months ago by Antoine Martin

r16845 makes it possible to debug the cutter element specifically using the env var XPRA_SOUND_LOG_CUTTER=1, so we can see when the sound level goes above or below the threshold:

sound source cutter message, above=False, min-timestamp=0, max-timestamp=840000000
sound source cutter message, above=True, min-timestamp=13090000000, max-timestamp=0
sound source cutter message, above=False, min-timestamp=0, max-timestamp=19060000000
sound source cutter message, above=True, min-timestamp=22980000000, max-timestamp=0
sound source cutter message, above=False, min-timestamp=0, max-timestamp=28660000000
sound source cutter message, above=True, min-timestamp=30810000000, max-timestamp=0
Last edited 3 months ago by Antoine Martin (previous) (diff)

comment:9 Changed 3 months ago by Antoine Martin

More testing and information in ticket:1638#comment:4.

wavenc was having problems with cutter, so r16847 adds it to the cutter blacklist.

comment:10 Changed 6 weeks ago by Antoine Martin

More fine tuning in r17285.

Note: See TracTickets for help on using tickets.