Do not use [Start/Stop]Agc()and GetAgcVolume() in PulseAudioInputStream |
||||||||
Issue description
StartAgc() initiates a timer which updates AGC volume level every second.
But it's ignored in PulseAudioInputStream, see [1]:
void PulseAudioInputStream::ReadData() {
double normalized_volume = 0.0;
GetAgcVolume(&normalized_volume);
normalized_volume = volume_ / GetMaxVolume();
...
}
It does not look like there is a point in running the timer.
Overall, the only part of AgcAudioStream<> implementation which is utilized in PulseAudioInputStream is Get/SetAutomaticGainControl(), and yet I'm not sure if setter affects the stream behavior.
How SetAutomaticGainControl() is expected to change the stream behavior here?
Should we stop inheriting from AgcAudioStream<>?
[1] https://cs.chromium.org/chromium/src/media/audio/pulse/pulse_input.cc?type=cs&l=273
,
Mar 14 2018
,
Mar 14 2018
It is not clear to me what you mean by "ignored" above. The latest mic volume is used in the data callback: callback_->OnData(audio_bus, capture_time, normalized_volume); which is inline with all other platforms. The timer is there to ensure that we don't ask for mic-volume changes too often (in every callback) and we ask to detect cases where e.g. the user has manually modified the input gain.
,
Mar 14 2018
I mean: GetAgcVolume(&normalized_volume); // this result will be ignored normalized_volume = volume_ / GetMaxVolume(); // here
,
Mar 14 2018
Got it, missed that. Seems to be lots of changes in this area for Linux since I last worked on it. E.g. https://codereview.chromium.org/12310102
,
Mar 14 2018
,
Mar 14 2018
,
Mar 23 2018
,
Mar 26 2018
Dale, could you comment?
,
Mar 26 2018
Sorry not sure, per the code you're definitely right. But no idea if that code is right, I don't understand the AGC volume process :)
,
May 29 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by olka@chromium.org
, Mar 14 2018Components: Blink>WebRTC>Audio Internals>Media>Capture