Signal logger #558
Signal logger #558
Conversation
alexlivekit
force-pushed
the
signal-logger
branch
8 times, most recently
from
2df31f9 to
0464494
Compare
…on't need this at this time
…he config from some place
alexlivekit
force-pushed
the
signal-logger
branch
from
0464494 to
e030c5f
Compare
| disp.RingingTimeout = defaultRingingTimeout | ||
| } | ||
| disp.Room.JitterBuf = c.jitterBuf | ||
| if disp.FeatureFlags != nil { |
There was a problem hiding this comment.
It is okay to omit this change, map lookup will still work on nil value.
| } | ||
|
|
||
| logSignalChanges := false | ||
| if featureFlags != nil { |
There was a problem hiding this comment.
Same here, no need for a check
| if p.logSignalChanges { | ||
| audioOut, err = NewSignalLogger(p.log, "mixed", audioOut) | ||
| if err != nil { | ||
| return err |
There was a problem hiding this comment.
We need to close original audioOut here.
| codec, err := opus.Decode(mTrack, channels, log) | ||
| var out msdk.PCM16Writer = mTrack | ||
| if rconf.LogSignalChanges { | ||
| out, _ = NewSignalLogger(log, track.ID(), out) |
There was a problem hiding this comment.
Not even logging an error? This could silenty write nil to out.
| // Keeps an internal state of whether we're currently transmitting signal (voice or noise), or silence. | ||
| // This implements msdk.PCM16Writer to inspect decoded packet content. | ||
| // Used to log changes betweem those states. | ||
| type SignalLogger struct { |
There was a problem hiding this comment.
We should consider moving it to media-sdk, since it's not SIP-specific.
|
|
||
| s.framesProcessed++ | ||
| if s.framesProcessed > 10 { // Don't log any changes at first | ||
| if isSignal && isSignal != s.lastIsSignal { // silence -> signal: Immediate transition |
There was a problem hiding this comment.
This could potentially hit each frame, wouldn't it be too spammy?
| } | ||
|
|
||
| func (s *SignalLogger) WriteSample(sample msdk.PCM16Sample) error { | ||
| currentEnergy := s.MeanAbsoluteDeviation(sample) |
There was a problem hiding this comment.
Probably doesn't matter here, but energy is usually computed as a square of amplitude. Maybe rename to ampMean?
| totalAbs += int64(v) | ||
| } | ||
| } | ||
| return float64(totalAbs) / float64(len(sample)) |
There was a problem hiding this comment.
We could also consider calculating a median, it will be less affected by sudden spikes.
| hangoverDuration: DefaultHangoverDuration, | ||
| signalMultiplier: DefaultSignalMultiplier, | ||
| noiseFloor: DefaultNoiseFloor, | ||
| lastSignalTime: time.Time{}, |
There was a problem hiding this comment.
No need to initialize explicitly.
If we ever want a fancier version (used mostly for VAD, or Voice Activity Detection), where our noise floor is dynamically calculated (this helps indicate speech even in noisier environments), we can look at the implementation in signal-logger-adaptive.
For now, the purpose of this feature is to output whether there's any signal, voice or noise, for sanity and debugging.