Skip to content

Commit

Permalink
Make AudioDecoder::DecodeCB a OnceCallback
Browse files Browse the repository at this point in the history
Bug: 751838
Change-Id: Id02455bb7215e048d6f1cdbf1ddcea594e54a6f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2273481
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#784123}
  • Loading branch information
chcunningham authored and Commit Bot committed Jun 30, 2020
1 parent 6c770bc commit 1926c86
Show file tree
Hide file tree
Showing 14 changed files with 56 additions and 54 deletions.
4 changes: 2 additions & 2 deletions media/base/audio_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class MEDIA_EXPORT AudioDecoder {

// Callback for Decode(). Called after the decoder has accepted corresponding
// DecoderBuffer, indicating that the pipeline can send next buffer to decode.
using DecodeCB = base::RepeatingCallback<void(DecodeStatus)>;
using DecodeCB = base::OnceCallback<void(DecodeStatus)>;

AudioDecoder();

Expand Down Expand Up @@ -86,7 +86,7 @@ class MEDIA_EXPORT AudioDecoder {
// |output_cb| must be called for each frame pending in the queue and
// |decode_cb| must be called after that.
virtual void Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) = 0;
DecodeCB decode_cb) = 0;

// Resets decoder state. All pending Decode() requests will be finished or
// aborted before |closure| is called.
Expand Down
3 changes: 1 addition & 2 deletions media/base/mock_filters.h
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,7 @@ class MockAudioDecoder : public AudioDecoder {
InitCB& init_cb,
const OutputCB& output_cb,
const WaitingCB& waiting_cb));
MOCK_METHOD2(Decode,
void(scoped_refptr<DecoderBuffer> buffer, const DecodeCB&));
MOCK_METHOD2(Decode, void(scoped_refptr<DecoderBuffer> buffer, DecodeCB));
void Reset(base::OnceClosure cb) override { Reset_(cb); }
MOCK_METHOD1(Reset_, void(base::OnceClosure&));

Expand Down
21 changes: 11 additions & 10 deletions media/filters/android/media_codec_audio_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,13 @@ bool MediaCodecAudioDecoder::CreateMediaCodecLoop() {
}

void MediaCodecAudioDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) {
DecodeCB bound_decode_cb = BindToCurrentLoop(decode_cb);
DecodeCB decode_cb) {
DecodeCB bound_decode_cb = BindToCurrentLoop(std::move(decode_cb));

if (!buffer->end_of_stream() && buffer->timestamp() == kNoTimestamp) {
DVLOG(2) << __func__ << " " << buffer->AsHumanReadableString()
<< ": no timestamp, skipping this buffer";
bound_decode_cb.Run(DecodeStatus::DECODE_ERROR);
std::move(bound_decode_cb).Run(DecodeStatus::DECODE_ERROR);
return;
}

Expand All @@ -189,7 +189,7 @@ void MediaCodecAudioDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
DVLOG(2) << __func__ << " " << buffer->AsHumanReadableString()
<< ": Error state, returning decode error for all buffers";
ClearInputQueue(DecodeStatus::DECODE_ERROR);
bound_decode_cb.Run(DecodeStatus::DECODE_ERROR);
std::move(bound_decode_cb).Run(DecodeStatus::DECODE_ERROR);
return;
}

Expand All @@ -203,7 +203,8 @@ void MediaCodecAudioDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
// time".
DCHECK(input_queue_.empty());

input_queue_.push_back(std::make_pair(std::move(buffer), bound_decode_cb));
input_queue_.push_back(
std::make_pair(std::move(buffer), std::move(bound_decode_cb)));

codec_loop_->ExpectWork();
}
Expand Down Expand Up @@ -339,16 +340,16 @@ void MediaCodecAudioDecoder::OnInputDataQueued(bool success) {
if (input_queue_.front().first->end_of_stream() && success)
return;

input_queue_.front().second.Run(success ? DecodeStatus::OK
: DecodeStatus::DECODE_ERROR);
std::move(input_queue_.front().second)
.Run(success ? DecodeStatus::OK : DecodeStatus::DECODE_ERROR);
input_queue_.pop_front();
}

void MediaCodecAudioDecoder::ClearInputQueue(DecodeStatus decode_status) {
DVLOG(2) << __func__;

for (const auto& entry : input_queue_)
entry.second.Run(decode_status);
for (auto& entry : input_queue_)
std::move(entry.second).Run(decode_status);

input_queue_.clear();
}
Expand Down Expand Up @@ -384,7 +385,7 @@ bool MediaCodecAudioDecoder::OnDecodedEos(
// So, we shouldn't be in that state. So, just DCHECK here.
DCHECK_NE(state_, STATE_ERROR);

input_queue_.front().second.Run(DecodeStatus::OK);
std::move(input_queue_.front()).second.Run(DecodeStatus::OK);
input_queue_.pop_front();

return true;
Expand Down
3 changes: 1 addition & 2 deletions media/filters/android/media_codec_audio_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ class MEDIA_EXPORT MediaCodecAudioDecoder : public AudioDecoder,
InitCB init_cb,
const OutputCB& output_cb,
const WaitingCB& waiting_cb) override;
void Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) override;
void Decode(scoped_refptr<DecoderBuffer> buffer, DecodeCB decode_cb) override;
void Reset(base::OnceClosure closure) override;
bool NeedsBitstreamConversion() const override;

Expand Down
4 changes: 2 additions & 2 deletions media/filters/audio_decoder_stream_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class AudioDecoderStreamTest : public testing::Test {
}

void ProduceDecoderOutput(scoped_refptr<DecoderBuffer> buffer,
const AudioDecoder::DecodeCB& decode_cb) {
AudioDecoder::DecodeCB decode_cb) {
// Make sure successive AudioBuffers have increasing timestamps.
last_timestamp_ += base::TimeDelta::FromMilliseconds(27);
const auto& config = demuxer_stream_.audio_decoder_config();
Expand All @@ -89,7 +89,7 @@ class AudioDecoderStreamTest : public testing::Test {
config.channel_layout(), config.channels(),
config.samples_per_second(), 1221, last_timestamp_)));
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(decode_cb, DecodeStatus::OK));
FROM_HERE, base::BindOnce(std::move(decode_cb), DecodeStatus::OK));
}

void RunUntilIdle() { task_environment_.RunUntilIdle(); }
Expand Down
6 changes: 3 additions & 3 deletions media/filters/decoder_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,9 @@ void DecoderStream<StreamType>::DecodeInternal(
const int buffer_size = is_eos ? 0 : buffer->data_size();
decoder_->Decode(
std::move(buffer),
base::BindRepeating(&DecoderStream<StreamType>::OnDecodeDone,
fallback_weak_factory_.GetWeakPtr(), buffer_size,
decoding_eos_, base::Passed(&trace_event)));
base::BindOnce(&DecoderStream<StreamType>::OnDecodeDone,
fallback_weak_factory_.GetWeakPtr(), buffer_size,
decoding_eos_, base::Passed(&trace_event)));
}

template <DemuxerStream::Type StreamType>
Expand Down
4 changes: 2 additions & 2 deletions media/filters/decrypting_audio_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,14 @@ void DecryptingAudioDecoder::Initialize(const AudioDecoderConfig& config,
}

void DecryptingAudioDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) {
DecodeCB decode_cb) {
DVLOG(3) << "Decode()";
DCHECK(task_runner_->BelongsToCurrentThread());
DCHECK(state_ == kIdle || state_ == kDecodeFinished) << state_;
DCHECK(decode_cb);
CHECK(!decode_cb_) << "Overlapping decodes are not supported.";

decode_cb_ = BindToCurrentLoop(decode_cb);
decode_cb_ = BindToCurrentLoop(std::move(decode_cb));

// Return empty (end-of-stream) frames if decoding has finished.
if (state_ == kDecodeFinished) {
Expand Down
3 changes: 1 addition & 2 deletions media/filters/decrypting_audio_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ class MEDIA_EXPORT DecryptingAudioDecoder : public AudioDecoder {
InitCB init_cb,
const OutputCB& output_cb,
const WaitingCB& waiting_cb) override;
void Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) override;
void Decode(scoped_refptr<DecoderBuffer> buffer, DecodeCB decode_cb) override;
void Reset(base::OnceClosure closure) override;

private:
Expand Down
18 changes: 9 additions & 9 deletions media/filters/ffmpeg_audio_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,24 +107,24 @@ void FFmpegAudioDecoder::Initialize(const AudioDecoderConfig& config,
}

void FFmpegAudioDecoder::Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) {
DecodeCB decode_cb) {
DCHECK(task_runner_->BelongsToCurrentThread());
DCHECK(decode_cb);
CHECK_NE(state_, kUninitialized);
DecodeCB decode_cb_bound = BindToCurrentLoop(decode_cb);
DecodeCB decode_cb_bound = BindToCurrentLoop(std::move(decode_cb));

if (state_ == kError) {
decode_cb_bound.Run(DecodeStatus::DECODE_ERROR);
std::move(decode_cb_bound).Run(DecodeStatus::DECODE_ERROR);
return;
}

// Do nothing if decoding has finished.
if (state_ == kDecodeFinished) {
decode_cb_bound.Run(DecodeStatus::OK);
std::move(decode_cb_bound).Run(DecodeStatus::OK);
return;
}

DecodeBuffer(*buffer, decode_cb_bound);
DecodeBuffer(*buffer, std::move(decode_cb_bound));
}

void FFmpegAudioDecoder::Reset(base::OnceClosure closure) {
Expand All @@ -137,7 +137,7 @@ void FFmpegAudioDecoder::Reset(base::OnceClosure closure) {
}

void FFmpegAudioDecoder::DecodeBuffer(const DecoderBuffer& buffer,
const DecodeCB& decode_cb) {
DecodeCB decode_cb) {
DCHECK(task_runner_->BelongsToCurrentThread());
DCHECK_NE(state_, kUninitialized);
DCHECK_NE(state_, kDecodeFinished);
Expand All @@ -147,20 +147,20 @@ void FFmpegAudioDecoder::DecodeBuffer(const DecoderBuffer& buffer,
// occurs with some damaged files.
if (!buffer.end_of_stream() && buffer.timestamp() == kNoTimestamp) {
DVLOG(1) << "Received a buffer without timestamps!";
decode_cb.Run(DecodeStatus::DECODE_ERROR);
std::move(decode_cb).Run(DecodeStatus::DECODE_ERROR);
return;
}

if (!FFmpegDecode(buffer)) {
state_ = kError;
decode_cb.Run(DecodeStatus::DECODE_ERROR);
std::move(decode_cb).Run(DecodeStatus::DECODE_ERROR);
return;
}

if (buffer.end_of_stream())
state_ = kDecodeFinished;

decode_cb.Run(DecodeStatus::OK);
std::move(decode_cb).Run(DecodeStatus::OK);
}

bool FFmpegAudioDecoder::FFmpegDecode(const DecoderBuffer& buffer) {
Expand Down
5 changes: 2 additions & 3 deletions media/filters/ffmpeg_audio_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder {
InitCB init_cb,
const OutputCB& output_cb,
const WaitingCB& waiting_cb) override;
void Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) override;
void Decode(scoped_refptr<DecoderBuffer> buffer, DecodeCB decode_cb) override;
void Reset(base::OnceClosure closure) override;

// Callback called from within FFmpeg to allocate a buffer based on the
Expand Down Expand Up @@ -85,7 +84,7 @@ class MEDIA_EXPORT FFmpegAudioDecoder : public AudioDecoder {
void DoReset();

// Handles decoding an unencrypted encoded buffer.
void DecodeBuffer(const DecoderBuffer& buffer, const DecodeCB& decode_cb);
void DecodeBuffer(const DecoderBuffer& buffer, DecodeCB decode_cb);
bool FFmpegDecode(const DecoderBuffer& buffer);
bool OnNewFrame(const DecoderBuffer& buffer,
bool* decoded_frame_this_loop,
Expand Down
10 changes: 6 additions & 4 deletions media/mojo/clients/mojo_audio_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,26 +88,28 @@ void MojoAudioDecoder::Initialize(const AudioDecoderConfig& config,
}

void MojoAudioDecoder::Decode(scoped_refptr<DecoderBuffer> media_buffer,
const DecodeCB& decode_cb) {
DecodeCB decode_cb) {
DVLOG(3) << __func__;
DCHECK(task_runner_->BelongsToCurrentThread());

if (!remote_decoder_.is_connected()) {
task_runner_->PostTask(
FROM_HERE, base::BindOnce(decode_cb, DecodeStatus::DECODE_ERROR));
FROM_HERE,
base::BindOnce(std::move(decode_cb), DecodeStatus::DECODE_ERROR));
return;
}

mojom::DecoderBufferPtr buffer =
mojo_decoder_buffer_writer_->WriteDecoderBuffer(std::move(media_buffer));
if (!buffer) {
task_runner_->PostTask(
FROM_HERE, base::BindOnce(decode_cb, DecodeStatus::DECODE_ERROR));
FROM_HERE,
base::BindOnce(std::move(decode_cb), DecodeStatus::DECODE_ERROR));
return;
}

DCHECK(!decode_cb_);
decode_cb_ = decode_cb;
decode_cb_ = std::move(decode_cb);

remote_decoder_->Decode(std::move(buffer),
base::BindOnce(&MojoAudioDecoder::OnDecodeStatus,
Expand Down
3 changes: 1 addition & 2 deletions media/mojo/clients/mojo_audio_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ class MojoAudioDecoder : public AudioDecoder, public mojom::AudioDecoderClient {
InitCB init_cb,
const OutputCB& output_cb,
const WaitingCB& waiting_cb) final;
void Decode(scoped_refptr<DecoderBuffer> buffer,
const DecodeCB& decode_cb) final;
void Decode(scoped_refptr<DecoderBuffer> buffer, DecodeCB decode_cb) final;
void Reset(base::OnceClosure closure) final;
bool NeedsBitstreamConversion() const final;

Expand Down
17 changes: 10 additions & 7 deletions media/mojo/clients/mojo_audio_decoder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "testing/gtest/include/gtest/gtest.h"

using ::base::test::RunCallback;
using ::base::test::RunOnceCallback;
using ::testing::_;
using ::testing::DoAll;
Expand Down Expand Up @@ -119,9 +118,11 @@ class MojoAudioDecoderTest : public ::testing::Test {
.WillRepeatedly(DoAll(SaveArg<3>(&output_cb_), SaveArg<4>(&waiting_cb_),
RunOnceCallback<2>(OkStatus())));
EXPECT_CALL(*mock_audio_decoder_, Decode(_, _))
.WillRepeatedly(
DoAll(InvokeWithoutArgs(this, &MojoAudioDecoderTest::ReturnOutput),
RunCallback<1>(DecodeStatus::OK)));
.WillRepeatedly([&](scoped_refptr<DecoderBuffer> buffer,
AudioDecoder::DecodeCB decode_cb) {
ReturnOutput();
std::move(decode_cb).Run(DecodeStatus::OK);
});
EXPECT_CALL(*mock_audio_decoder_, Reset_(_))
.WillRepeatedly(RunOnceCallback<0>());

Expand Down Expand Up @@ -291,9 +292,11 @@ TEST_F(MojoAudioDecoderTest, Reset_DuringDecode_ChunkedWrite) {
TEST_F(MojoAudioDecoderTest, WaitingForKey) {
Initialize();
EXPECT_CALL(*mock_audio_decoder_, Decode(_, _))
.WillOnce(
DoAll(InvokeWithoutArgs(this, &MojoAudioDecoderTest::WaitForKey),
RunCallback<1>(DecodeStatus::OK)));
.WillOnce([&](scoped_refptr<DecoderBuffer> buffer,
AudioDecoder::DecodeCB decode_cb) {
WaitForKey();
std::move(decode_cb).Run(DecodeStatus::OK);
});
EXPECT_CALL(*this, OnWaiting(WaitingReason::kNoDecryptionKey)).Times(1);
EXPECT_CALL(*this, OnDecoded(DecodeStatus::OK))
.WillOnce(InvokeWithoutArgs(this, &MojoAudioDecoderTest::QuitLoop));
Expand Down
9 changes: 5 additions & 4 deletions media/renderers/audio_renderer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -509,17 +509,18 @@ class AudioRendererImplTest : public ::testing::Test, public RendererClient {
bool ended() const { return ended_; }

void DecodeDecoder(scoped_refptr<DecoderBuffer> buffer,
const AudioDecoder::DecodeCB& decode_cb) {
AudioDecoder::DecodeCB decode_cb) {
// TODO(scherkus): Make this a DCHECK after threading semantics are fixed.
if (!main_thread_task_runner_->BelongsToCurrentThread()) {
main_thread_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AudioRendererImplTest::DecodeDecoder,
base::Unretained(this), buffer, decode_cb));
FROM_HERE,
base::BindOnce(&AudioRendererImplTest::DecodeDecoder,
base::Unretained(this), buffer, std::move(decode_cb)));
return;
}

CHECK(!decode_cb_) << "Overlapping decodes are not permitted";
decode_cb_ = decode_cb;
decode_cb_ = std::move(decode_cb);

// Wake up WaitForPendingRead() if needed.
if (wait_for_pending_decode_cb_)
Expand Down

0 comments on commit 1926c86

Please sign in to comment.