Skip to content

Commit

Permalink
Only increase audio queue capacity once per underflow
Browse files Browse the repository at this point in the history
Underflow may manifest as a partial render (provide some frames, but
less than requested) followed by an empty render (provide no frames).
This should not increase the queue size twice back to back. Any
increase to the queue size should only occur once the renderer has
refilled to the current size and again underflowed (demonstrating that
the size may not have been enough).

Bug: 713540
Change-Id: I752d6b027b9ae39a29c5a4a451e0fe55b7bfa146
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1671900
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672375}
  • Loading branch information
chcunningham authored and Commit Bot committed Jun 26, 2019
1 parent e50b416 commit 28f1359
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 10 deletions.
6 changes: 5 additions & 1 deletion media/renderers/audio_renderer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1090,10 +1090,14 @@ int AudioRendererImpl::Render(base::TimeDelta delay,
algorithm_->IncreaseQueueCapacity();
SetBufferingState_Locked(BUFFERING_HAVE_NOTHING);
}
} else if (frames_written < frames_requested && !received_end_of_stream_) {
} else if (frames_written < frames_requested && !received_end_of_stream_ &&
state_ == kPlaying &&
buffering_state_ != BUFFERING_HAVE_NOTHING) {
// If we only partially filled the request and should have more data, go
// ahead and increase queue capacity to try and meet the next request.
// Trigger underflow to give us a chance to refill up to the new cap.
algorithm_->IncreaseQueueCapacity();
SetBufferingState_Locked(BUFFERING_HAVE_NOTHING);
}

audio_clock_->WroteAudio(frames_written + frames_after_end_of_stream,
Expand Down
58 changes: 49 additions & 9 deletions media/renderers/audio_renderer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -711,18 +711,58 @@ TEST_F(AudioRendererImplTest, Underflow_CapacityIncreasesBeforeHaveNothing) {
EXPECT_GT(buffer_capacity().value, initial_capacity.value);
}

TEST_F(AudioRendererImplTest, CapacityAppropriateForHardware) {
// Verify that initial capacity is reasonable in normal case.
TEST_F(AudioRendererImplTest, Underflow_OneCapacityIncreasePerUnderflow) {
Initialize();
EXPECT_GT(buffer_capacity().value, hardware_params_.frames_per_buffer());
Preroll();
StartTicking();

// Verify in the no-config-changes-expected case.
ConfigureBasicRenderer(AudioParameters(AudioParameters::AUDIO_PCM_LOW_LATENCY,
kChannelLayout,
kOutputSamplesPerSecond, 1024 * 15));
OutputFrames prev_capacity = buffer_capacity();

Initialize();
EXPECT_GT(buffer_capacity().value, hardware_params_.frames_per_buffer());
// Consume more than is available (partial read) to trigger underflow.
EXPECT_CALL(*this, OnBufferingStateChange(BUFFERING_HAVE_NOTHING));
EXPECT_FALSE(ConsumeBufferedData(OutputFrames(frames_buffered().value + 1)));

// Verify first underflow triggers an increase to buffer capacity and
// signals HAVE_NOTHING.
EXPECT_GT(buffer_capacity().value, prev_capacity.value);
prev_capacity = buffer_capacity();
// Give HAVE_NOTHING a chance to post.
base::RunLoop().RunUntilIdle();
testing::Mock::VerifyAndClearExpectations(this);

// Try reading again, this time with the queue totally empty. We should expect
// NO additional HAVE_NOTHING and NO increase to capacity because we still
// haven't refilled the queue since the previous underflow.
EXPECT_EQ(0, frames_buffered().value);
EXPECT_CALL(*this, OnBufferingStateChange(BUFFERING_HAVE_NOTHING)).Times(0);
EXPECT_FALSE(ConsumeBufferedData(OutputFrames(1)));
EXPECT_EQ(buffer_capacity().value, prev_capacity.value);
// Give HAVE_NOTHING a chance to NOT post.
base::RunLoop().RunUntilIdle();
testing::Mock::VerifyAndClearExpectations(this);

// Fill the buffer back up.
WaitForPendingRead();
DeliverRemainingAudio();
EXPECT_GT(frames_buffered().value, 0);

// Consume all available data without underflowing. Expect no buffer state
// change and no change to capacity.
EXPECT_CALL(*this, OnBufferingStateChange(BUFFERING_HAVE_NOTHING)).Times(0);
EXPECT_TRUE(ConsumeBufferedData(OutputFrames(frames_buffered().value)));
EXPECT_EQ(buffer_capacity().value, prev_capacity.value);
// Give HAVE_NOTHING a chance to NOT post.
base::RunLoop().RunUntilIdle();
testing::Mock::VerifyAndClearExpectations(this);

// Now empty, trigger underflow attempting to read one frame. This should
// signal buffering state change and increase capacity.
EXPECT_CALL(*this, OnBufferingStateChange(BUFFERING_HAVE_NOTHING));
EXPECT_FALSE(ConsumeBufferedData(OutputFrames(1)));
EXPECT_GT(buffer_capacity().value, prev_capacity.value);
// Give HAVE_NOTHING a chance to NOT post.
base::RunLoop().RunUntilIdle();
testing::Mock::VerifyAndClearExpectations(this);
}

// Verify that the proper reduced search space is configured for playback rate
Expand Down

0 comments on commit 28f1359

Please sign in to comment.