New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 804277 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit 19 days ago
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Remove the render audio buffer in the MediaStreamAudioProcessor

Project Member Reported by saza@chromium.org, Jan 22 2018

Issue description

The MediaStreamAudioProcessor has an internal buffer, render_fifo_ [1], to ensure the APM gets 10 ms chunks of audio. It appears redundant, because it is always fed 10 ms chunks provided by WebRTC. It should be removed in order to:
2) make it easier to switch to using the APM via ProcessReverseStream over the non-modifying AnalyzeReverseStream;
1) reduce code complexity;
3) avoid copying the audio buffer an extra time.

The only non-unittest caller of MediaStreamAudioProcessor::OnPlayoutData is WebRtcAudioDeviceImpl::RenderData [2], which explicitly DCHECKs that the audio is 10 ms long.

The unit tests (via ProcessDataAndVerifyFormat [3]) use the same frame size for capture as for render, not necessarily 10 ms.

[1] https://cs.chromium.org/chromium/src/content/renderer/media/media_stream_audio_processor.cc?q=media_stream_audio_processor&sq=package:chromium&dr=C&l=526

[2] https://cs.chromium.org/chromium/src/content/renderer/media/webrtc_audio_device_impl.cc?sq=package:chromium&dr=C&l=74

[3] https://cs.chromium.org/chromium/src/content/renderer/media/media_stream_audio_processor_unittest.cc?sq=package:chromium&dr=C&l=98
 

Comment 1 by saza@chromium.org, Jan 23 2018

Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9a421fa170335630f5c714e5a12049664095f964

commit 9a421fa170335630f5c714e5a12049664095f964
Author: Sam Zackrisson <saza@chromium.org>
Date: Fri Feb 02 07:48:16 2018

Remove redundant APM render buffer

This CL removes the render_fifo_ buffer from MediaStreamAudioProcessor
and DCHECKs that the input audio is 10 ms long.

MediaStreamAudioProcessor currently buffers audio provided to
OnPlayoutData, to feed frames in 10 ms chunks to the APM. However, the
caller of OnPlayoutData already DCHECKs that this is the case.

This CL also modifies the unit tests. They use the same
configuration for capture input as for render input, even though only
capture needs to support input other than 10 ms.

Bug:  804277 

Change-Id: Ia20cf0e0810c59570305f1ca295016e0ca11057e
Reviewed-on: https://chromium-review.googlesource.com/875862
Commit-Queue: Sam Zackrisson <saza@chromium.org>
Reviewed-by: Olga Sharonova <olka@chromium.org>
Reviewed-by: Henrik Grunell <grunell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533990}
[modify] https://crrev.com/9a421fa170335630f5c714e5a12049664095f964/content/renderer/media/media_stream_audio_processor.cc
[modify] https://crrev.com/9a421fa170335630f5c714e5a12049664095f964/content/renderer/media/media_stream_audio_processor.h
[modify] https://crrev.com/9a421fa170335630f5c714e5a12049664095f964/content/renderer/media/media_stream_audio_processor_unittest.cc
[modify] https://crrev.com/9a421fa170335630f5c714e5a12049664095f964/content/renderer/media/webrtc_audio_device_impl.cc
[modify] https://crrev.com/9a421fa170335630f5c714e5a12049664095f964/content/renderer/media/webrtc_audio_device_impl.h

Comment 3 by saza@chromium.org, Feb 2 2018

Status: Fixed (was: Assigned)

Sign in to add a comment