|
|
Created:
8 years, 2 months ago by xhwang Modified:
8 years, 2 months ago CC:
chromium-reviews, feature-media-reviews_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdd DecryptingAudioDecoder.
BUG=123421
TEST=added new tests to media_unittests
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162976
Patch Set 1 #Patch Set 2 : Updated the decoder. Need to update tests later. #Patch Set 3 : Add unittests #
Total comments: 3
Patch Set 4 : ready for review, w/ questions about Stop() #Patch Set 5 : rebase only #Patch Set 6 : Stop() removed. #
Total comments: 2
Patch Set 7 : fix redefinition of ACTIONS in test, add DAD into pipeline. #Patch Set 8 : rebase #
Total comments: 8
Patch Set 9 : Add TODO #Patch Set 10 : rebase #
Messages
Total messages: 18 (0 generated)
This CL is ready for review. I left the Stop() for discussion even if AudioDecoder() doesn't have a Stop() API. PTAL! http://codereview.chromium.org/11198017/diff/4/media/base/decryptor.h File media/base/decryptor.h (right): http://codereview.chromium.org/11198017/diff/4/media/base/decryptor.h#newcode134 media/base/decryptor.h:134: typedef std::list<scoped_refptr<Buffer> > AudioBuffers; This cl is based on 11144036, so the changes in this file are really from that CL. Comments are not accurate here and please ignore. http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... File media/filters/decrypting_audio_decoder.cc (right): http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... media/filters/decrypting_audio_decoder.cc:105: void DecryptingAudioDecoder::Stop(const base::Closure& closure) { If we decided to have Stop(), this needs to be changed to something like fischman's fix. http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... File media/filters/decrypting_audio_decoder.h (right): http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... media/filters/decrypting_audio_decoder.h:65: void Stop(const base::Closure& closure); scherkus: currently nobody is gonna call this Stop(). Are we okay with this during teardown? It's seems working for now since AudioDecoder is refcounted. WDYT?
On 2012/10/18 02:14:10, xhwang wrote: > http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... > media/filters/decrypting_audio_decoder.h:65: void Stop(const base::Closure& > closure); > scherkus: currently nobody is gonna call this Stop(). Are we okay with this > during teardown? It's seems working for now since AudioDecoder is refcounted. > WDYT? So how is it working today? It seems that the DCHECK() in the dtor would fire. How are we doing teardown and setting state_ to kStopped?
On 2012/10/18 17:32:50, scherkus wrote: > On 2012/10/18 02:14:10, xhwang wrote: > > > http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... > > media/filters/decrypting_audio_decoder.h:65: void Stop(const base::Closure& > > closure); > > scherkus: currently nobody is gonna call this Stop(). Are we okay with this > > during teardown? It's seems working for now since AudioDecoder is refcounted. > > WDYT? > > So how is it working today? It seems that the DCHECK() in the dtor would fire. > How are we doing teardown and setting state_ to kStopped? No, it's not working today. I was waiting for fischman's hang fix on DVD to land before I update the Stop() code in this CL. Here's my plan: 1) Copy DVD's new Stop() code to DAD (not Dad :)). 2) Keep Stop() in DAD even if it's not used for now. 2) In DAD's dtor, remove the chcek. It seems that we can't call Stop() in dtor because it's async. WDYT?
On 2012/10/18 17:47:08, xhwang wrote: > On 2012/10/18 17:32:50, scherkus wrote: > > On 2012/10/18 02:14:10, xhwang wrote: > > > > > > http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... > > > media/filters/decrypting_audio_decoder.h:65: void Stop(const base::Closure& > > > closure); > > > scherkus: currently nobody is gonna call this Stop(). Are we okay with this > > > during teardown? It's seems working for now since AudioDecoder is > refcounted. > > > WDYT? > > > > So how is it working today? It seems that the DCHECK() in the dtor would fire. > > How are we doing teardown and setting state_ to kStopped? > > No, it's not working today. I was waiting for fischman's hang fix on DVD to land > before I update the Stop() code in this CL. Here's my plan: > 1) Copy DVD's new Stop() code to DAD (not Dad :)). > 2) Keep Stop() in DAD even if it's not used for now. > 2) In DAD's dtor, remove the chcek. It seems that we can't call Stop() in dtor > because it's async. WDYT? By "not working" do you mean "the DCHECK is firing" or that "it's hanging/crashing/etc"? The only reason why I'd advocate for a Stop() is for the reason I listed in ~FFmpegAudioDecoder(), namely refcounting can lead to teardown happening on the wrong thread. Do you forsee us needing a Stop() for DAD/PD/etc?
On 2012/10/18 18:04:40, scherkus wrote: > On 2012/10/18 17:47:08, xhwang wrote: > > On 2012/10/18 17:32:50, scherkus wrote: > > > On 2012/10/18 02:14:10, xhwang wrote: > > > > > > > > > > http://codereview.chromium.org/11198017/diff/4/media/filters/decrypting_audio... > > > > media/filters/decrypting_audio_decoder.h:65: void Stop(const > base::Closure& > > > > closure); > > > > scherkus: currently nobody is gonna call this Stop(). Are we okay with > this > > > > during teardown? It's seems working for now since AudioDecoder is > > refcounted. > > > > WDYT? > > > > > > So how is it working today? It seems that the DCHECK() in the dtor would > fire. > > > How are we doing teardown and setting state_ to kStopped? > > > > No, it's not working today. I was waiting for fischman's hang fix on DVD to > land > > before I update the Stop() code in this CL. Here's my plan: > > 1) Copy DVD's new Stop() code to DAD (not Dad :)). > > 2) Keep Stop() in DAD even if it's not used for now. > > 2) In DAD's dtor, remove the chcek. It seems that we can't call Stop() in dtor > > because it's async. WDYT? > > By "not working" do you mean "the DCHECK is firing" or that "it's > hanging/crashing/etc"? > > The only reason why I'd advocate for a Stop() is for the reason I listed in > ~FFmpegAudioDecoder(), namely refcounting can lead to teardown happening on the > wrong thread. > > Do you forsee us needing a Stop() for DAD/PD/etc? As discussed offline, we'll remove Stop from DAD all together and remove the check in dtor of DAD.
Stop() removed. This CL is ready for review. PTAL!
lgtm w/ q the amount of duplicated code is a bit of a downer (especially the unit tests!) but let's go w/ this for now http://codereview.chromium.org/11198017/diff/13002/media/filters/decrypting_a... File media/filters/decrypting_audio_decoder.cc (right): http://codereview.chromium.org/11198017/diff/13002/media/filters/decrypting_a... media/filters/decrypting_audio_decoder.cc:107: DecryptingAudioDecoder::~DecryptingAudioDecoder() { can't we still DCHECK for uninitialized?
Added the new DecryptingAudioDecoder into media pipeline and fixed ACTION redefinition in tests. PTAL again! http://codereview.chromium.org/11198017/diff/13002/media/filters/decrypting_a... File media/filters/decrypting_audio_decoder.cc (right): http://codereview.chromium.org/11198017/diff/13002/media/filters/decrypting_a... media/filters/decrypting_audio_decoder.cc:107: DecryptingAudioDecoder::~DecryptingAudioDecoder() { On 2012/10/18 21:13:12, scherkus wrote: > can't we still DCHECK for uninitialized? ISTM dtor can happen in a lot of states now, e.g. kUninitialized, kIdle, kDecodeFinished, etc.
http://codereview.chromium.org/11198017/diff/9009/media/filters/decrypting_au... File media/filters/decrypting_audio_decoder.h (right): http://codereview.chromium.org/11198017/diff/9009/media/filters/decrypting_au... media/filters/decrypting_audio_decoder.h:157: Decryptor::AudioBuffers queued_audio_frames_; sorry just for noticing this... why are we queuing audio frames again? http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers.cc File webkit/media/filter_helpers.cc (right): http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers... webkit/media/filter_helpers.cc:49: filter_collection->GetAudioDecoders()->push_back(decrypting_audio_decoder); here we have ffmpeg > decrypting for audio, but for video the order is reversed intentional?
http://codereview.chromium.org/11198017/diff/9009/media/filters/decrypting_au... File media/filters/decrypting_audio_decoder.h (right): http://codereview.chromium.org/11198017/diff/9009/media/filters/decrypting_au... media/filters/decrypting_audio_decoder.h:157: Decryptor::AudioBuffers queued_audio_frames_; On 2012/10/19 02:12:58, scherkus wrote: > sorry just for noticing this... why are we queuing audio frames again? This is the same as "queued_audio_" in FFAD. We may have multiple buffers returned for one DecryptAndDecode call. But we only have one read cb pending. So we'll satisfy the read cb with one buffer and queue the rest for future Read calls. http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers.cc File webkit/media/filter_helpers.cc (right): http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers... webkit/media/filter_helpers.cc:49: filter_collection->GetAudioDecoders()->push_back(decrypting_audio_decoder); On 2012/10/19 02:12:58, scherkus wrote: > here we have ffmpeg > decrypting for audio, but for video the order is reversed > > > intentional? Nothing happens by chance :) This is intentional. Ideally I'd like decrypting decoders to be after normal decoders because in the real world most videos will not be encrypted. But for video, FFVD can also do decryption. If both decrypt-only and decrypt-and-decode are supported, we'd like to choose decrypt-and-decode. That's why DVD comes before FFVD. WDYT?
http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers.cc File webkit/media/filter_helpers.cc (right): http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers... webkit/media/filter_helpers.cc:49: filter_collection->GetAudioDecoders()->push_back(decrypting_audio_decoder); On 2012/10/19 02:19:45, xhwang wrote: > On 2012/10/19 02:12:58, scherkus wrote: > > here we have ffmpeg > decrypting for audio, but for video the order is > reversed > > > > > > intentional? > > Nothing happens by chance :) This is intentional. > Ideally I'd like decrypting decoders to be after normal decoders because in the > real world most videos will not be encrypted. But for video, FFVD can also do > decryption. If both decrypt-only and decrypt-and-decode are supported, we'd like > to choose decrypt-and-decode. That's why DVD comes before FFVD. WDYT? ...but FFVD doing decryption is temporary, right?
http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers.cc File webkit/media/filter_helpers.cc (right): http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers... webkit/media/filter_helpers.cc:49: filter_collection->GetAudioDecoders()->push_back(decrypting_audio_decoder); On 2012/10/19 02:23:05, scherkus wrote: > On 2012/10/19 02:19:45, xhwang wrote: > > On 2012/10/19 02:12:58, scherkus wrote: > > > here we have ffmpeg > decrypting for audio, but for video the order is > > reversed > > > > > > > > > intentional? > > > > Nothing happens by chance :) This is intentional. > > Ideally I'd like decrypting decoders to be after normal decoders because in > the > > real world most videos will not be encrypted. But for video, FFVD can also do > > decryption. If both decrypt-only and decrypt-and-decode are supported, we'd > like > > to choose decrypt-and-decode. That's why DVD comes before FFVD. WDYT? > > ...but FFVD doing decryption is temporary, right? Yes. But if I move FFVD up now, then decrypt-and-decode (D&D) will not be chosen. For decrypt-only, we don't have the mechanism to ask a CDM "if you support decrypt-only". Decrypt-only can only succeed or fail. So it should always be tried after the D&D path. We'll change the order here when we remove decryption out of FFVD.
lgtm http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers.cc File webkit/media/filter_helpers.cc (right): http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers... webkit/media/filter_helpers.cc:49: filter_collection->GetAudioDecoders()->push_back(decrypting_audio_decoder); On 2012/10/19 02:40:35, xhwang wrote: > On 2012/10/19 02:23:05, scherkus wrote: > > On 2012/10/19 02:19:45, xhwang wrote: > > > On 2012/10/19 02:12:58, scherkus wrote: > > > > here we have ffmpeg > decrypting for audio, but for video the order is > > > reversed > > > > > > > > > > > > intentional? > > > > > > Nothing happens by chance :) This is intentional. > > > Ideally I'd like decrypting decoders to be after normal decoders because in > > the > > > real world most videos will not be encrypted. But for video, FFVD can also > do > > > decryption. If both decrypt-only and decrypt-and-decode are supported, we'd > > like > > > to choose decrypt-and-decode. That's why DVD comes before FFVD. WDYT? > > > > ...but FFVD doing decryption is temporary, right? > > Yes. But if I move FFVD up now, then decrypt-and-decode (D&D) will not be > chosen. For decrypt-only, we don't have the mechanism to ask a CDM "if you > support decrypt-only". Decrypt-only can only succeed or fail. So it should > always be tried after the D&D path. We'll change the order here when we remove > decryption out of FFVD. Sounds good to me -- can you TODOify it w/ a comment?
http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers.cc File webkit/media/filter_helpers.cc (right): http://codereview.chromium.org/11198017/diff/9009/webkit/media/filter_helpers... webkit/media/filter_helpers.cc:49: filter_collection->GetAudioDecoders()->push_back(decrypting_audio_decoder); On 2012/10/19 02:51:48, scherkus wrote: > On 2012/10/19 02:40:35, xhwang wrote: > > On 2012/10/19 02:23:05, scherkus wrote: > > > On 2012/10/19 02:19:45, xhwang wrote: > > > > On 2012/10/19 02:12:58, scherkus wrote: > > > > > here we have ffmpeg > decrypting for audio, but for video the order is > > > > reversed > > > > > > > > > > > > > > > intentional? > > > > > > > > Nothing happens by chance :) This is intentional. > > > > Ideally I'd like decrypting decoders to be after normal decoders because > in > > > the > > > > real world most videos will not be encrypted. But for video, FFVD can also > > do > > > > decryption. If both decrypt-only and decrypt-and-decode are supported, > we'd > > > like > > > > to choose decrypt-and-decode. That's why DVD comes before FFVD. WDYT? > > > > > > ...but FFVD doing decryption is temporary, right? > > > > Yes. But if I move FFVD up now, then decrypt-and-decode (D&D) will not be > > chosen. For decrypt-only, we don't have the mechanism to ask a CDM "if you > > support decrypt-only". Decrypt-only can only succeed or fail. So it should > > always be tried after the D&D path. We'll change the order here when we remove > > decryption out of FFVD. > > Sounds good to me -- can you TODOify it w/ a comment? Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/11198017/15
Failed to apply patch for media/filters/decrypting_audio_decoder.cc: While running patch -p1 --forward --force --no-backup-if-mismatch; A media/filters/decrypting_audio_decoder.cc Copied media/filters/decrypting_video_decoder.cc -> media/filters/decrypting_audio_decoder.cc patching file media/filters/decrypting_audio_decoder.cc Hunk #5 FAILED at 137. Hunk #6 succeeded at 178 (offset 2 lines). Hunk #7 succeeded at 217 (offset 2 lines). Hunk #8 succeeded at 235 (offset 2 lines). Hunk #9 succeeded at 276 (offset 2 lines). Hunk #10 succeeded at 315 (offset 2 lines). Hunk #11 succeeded at 341 (offset 2 lines). Hunk #12 succeeded at 377 (offset 2 lines). 1 out of 12 hunks FAILED -- saving rejects to file media/filters/decrypting_audio_decoder.cc.rej Patch: NR media/filters/decrypting_video_decoder.cc->media/filters/decrypting_audio_decoder.cc Index: media/filters/decrypting_audio_decoder.cc diff --git a/media/filters/decrypting_video_decoder.cc b/media/filters/decrypting_audio_decoder.cc similarity index 65% copy from media/filters/decrypting_video_decoder.cc copy to media/filters/decrypting_audio_decoder.cc index b8200300b89d7ea3b555f4c48e7156525fd93edf..a6871b5335930fa5a2638ad96d4230d91de5edc6 100644 --- a/media/filters/decrypting_video_decoder.cc +++ b/media/filters/decrypting_audio_decoder.cc @@ -2,57 +2,61 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "media/filters/decrypting_video_decoder.h" +#include "media/filters/decrypting_audio_decoder.h" #include "base/bind.h" #include "base/callback_helpers.h" #include "base/location.h" #include "base/message_loop_proxy.h" +#include "media/base/audio_decoder_config.h" #include "media/base/bind_to_loop.h" +#include "media/base/buffers.h" +#include "media/base/data_buffer.h" #include "media/base/decoder_buffer.h" #include "media/base/decryptor.h" #include "media/base/demuxer_stream.h" #include "media/base/pipeline.h" -#include "media/base/video_decoder_config.h" -#include "media/base/video_frame.h" namespace media { #define BIND_TO_LOOP(function) \ media::BindToLoop(message_loop_, base::Bind(function, this)) -DecryptingVideoDecoder::DecryptingVideoDecoder( +DecryptingAudioDecoder::DecryptingAudioDecoder( const MessageLoopFactoryCB& message_loop_factory_cb, const RequestDecryptorNotificationCB& request_decryptor_notification_cb) : message_loop_factory_cb_(message_loop_factory_cb), state_(kUninitialized), request_decryptor_notification_cb_(request_decryptor_notification_cb), decryptor_(NULL), - key_added_while_pending_decode_(false) { + key_added_while_pending_decode_(false), + bits_per_channel_(0), + channel_layout_(CHANNEL_LAYOUT_NONE), + samples_per_second_(0) { } -void DecryptingVideoDecoder::Initialize( +void DecryptingAudioDecoder::Initialize( const scoped_refptr<DemuxerStream>& stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) { DCHECK(!message_loop_); message_loop_ = base::ResetAndReturn(&message_loop_factory_cb_).Run(); message_loop_->PostTask(FROM_HERE, base::Bind( - &DecryptingVideoDecoder::DoInitialize, this, + &DecryptingAudioDecoder::DoInitialize, this, stream, status_cb, statistics_cb)); } -void DecryptingVideoDecoder::Read(const ReadCB& read_cb) { +void DecryptingAudioDecoder::Read(const ReadCB& read_cb) { // Complete operation asynchronously on different stack of execution as per - // the API contract of VideoDecoder::Read() + // the API contract of AudioDecoder::Read() message_loop_->PostTask(FROM_HERE, base::Bind( - &DecryptingVideoDecoder::DoRead, this, read_cb)); + &DecryptingAudioDecoder::DoRead, this, read_cb)); } -void DecryptingVideoDecoder::Reset(const base::Closure& closure) { +void DecryptingAudioDecoder::Reset(const base::Closure& closure) { if (!message_loop_->BelongsToCurrentThread()) { message_loop_->PostTask(FROM_HERE, base::Bind( - &DecryptingVideoDecoder::Reset, this, closure)); + &DecryptingAudioDecoder::Reset, this, closure)); return; } @@ -67,7 +71,7 @@ void DecryptingVideoDecoder::Reset(const base::Closure& closure) { reset_cb_ = closure; - decryptor_->ResetDecoder(Decryptor::kVideo); + decryptor_->ResetDecoder(Decryptor::kAudio); // Reset() cannot complete if the read callback is still pending. // Defer the resetting process in this case. The |reset_cb_| will be fired @@ -88,41 +92,22 @@ void DecryptingVideoDecoder::Reset(const base::Closure& closure) { DoReset(); } -void DecryptingVideoDecoder::Stop(const base::Closure& closure) { - if (!message_loop_->BelongsToCurrentThread()) { - message_loop_->PostTask(FROM_HERE, base::Bind( - &DecryptingVideoDecoder::Stop, this, closure)); - return; - } +int DecryptingAudioDecoder::bits_per_channel() { + return bits_per_channel_; +} - DVLOG(2) << "Stop() - state: " << state_; - - // At this point the render thread is likely paused (in WebMediaPlayerImpl's - // Destroy()), so running |closure| can't wait for anything that requires the - // render thread to be processing messages to complete (such as PPAPI - // callbacks). - if (decryptor_) - decryptor_->DeinitializeDecoder(Decryptor::kVideo); - if (!request_decryptor_notification_cb_.is_null()) { - base::ResetAndReturn(&request_decryptor_notification_cb_).Run( - DecryptorNotificationCB()); - } - pending_buffer_to_decode_ = NULL; - if (!init_cb_.is_null()) - base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED); - if (!read_cb_.is_null()) - base::ResetAndReturn(&read_cb_).Run(kOk, NULL); - if (!reset_cb_.is_null()) - base::ResetAndReturn(&reset_cb_).Run(); - state_ = kStopped; - closure.Run(); +ChannelLayout DecryptingAudioDecoder::channel_layout() { + return channel_layout_; +} + +int DecryptingAudioDecoder::samples_per_second() { + return samples_per_second_; } -DecryptingVideoDecoder::~DecryptingVideoDecoder() { - DCHECK(state_ == kUninitialized || state_ == kStopped) << state_; +DecryptingAudioDecoder::~DecryptingAudioDecoder() { } -void DecryptingVideoDecoder::DoInitialize( +void DecryptingAudioDecoder::DoInitialize( const scoped_refptr<DemuxerStream>& stream, const PipelineStatusCB& status_cb, const StatisticsCB& statistics_cb) { @@ -131,15 +116,14 @@ void DecryptingVideoDecoder::DoInitialize( DCHECK_EQ(state_, kUninitialized) << state_; DCHECK(stream); - const VideoDecoderConfig& config = stream->video_decoder_config(); + const AudioDecoderConfig& config = stream->audio_decoder_config(); if (!config.IsValidConfig()) { - DLOG(ERROR) << "Invalid video stream config: " - << config.AsHumanReadableString(); + DLOG(ERROR) << "Invalid audio stream config."; status_cb.Run(PIPELINE_ERROR_DECODE); return; } - // DecryptingVideoDecoder only accepts potentially encrypted stream. + // DecryptingAudioDecoder only accepts potentially encrypted stream. if (!config.is_encrypted()) { status_cb.Run(DECODER_ERROR_NOT_SUPPORTED); return; @@ -153,38 +137,32 @@ void DecryptingVideoDecoder::DoInitialize( state_ = kDecryptorRequested; request_decryptor_notification_cb_.Run( - BIND_TO_LOOP(&DecryptingVideoDecoder::SetDecryptor)); + BIND_TO_LOOP(&DecryptingAudioDecoder::SetDecryptor)); } -void DecryptingVideoDecoder::SetDecryptor(Decryptor* decryptor) { +void DecryptingAudioDecoder::SetDecryptor(Decryptor* decryptor) { DVLOG(2) << "SetDecryptor()"; DCHECK(message_loop_->BelongsToCurrentThread()); - - if (state_ == kStopped) - return; - DCHECK_EQ(state_, kDecryptorRequested) << state_; DCHECK(!init_cb_.is_null()); + DCHECK(!request_decryptor_notification_cb_.is_null()); + request_decryptor_notification_cb_.Reset(); decryptor_ = decryptor; - scoped_ptr<VideoDecoderConfig> scoped_config(new VideoDecoderConfig()); - scoped_config->CopyFrom(demuxer_stream_->video_decoder_config()); + scoped_ptr<AudioDecoderConfig> scoped_config(new AudioDecoderConfig()); + scoped_config->CopyFrom(demuxer_stream_->audio_decoder_config()); state_ = kPendingDecoderInit; - decryptor_->InitializeVideoDecoder( + decryptor_->InitializeAudioDecoder( scoped_config.Pass(), - BIND_TO_LOOP(&DecryptingVideoDecoder::FinishInitialization), - BIND_TO_LOOP(&DecryptingVideoDecoder::OnKeyAdded)); + BIND_TO_LOOP(&DecryptingAudioDecoder::FinishInitialization), + BIND_TO_LOOP(&DecryptingAudioDecoder::OnKeyAdded)); } -void DecryptingVideoDecoder::FinishInitialization(bool success) { +void DecryptingAudioDecoder::FinishInitialization(bool success) { DVLOG(2) << "FinishInitialization()"; DCHECK(message_loop_->BelongsToCurrentThread()); - - if (state_ == kStopped) - return; - DCHECK_EQ(state_, kPendingDecoderInit) << state_; DCHECK(!init_cb_.is_null()); DCHECK(reset_cb_.is_null()); // No Reset() before initialization finished. @@ -192,25 +170,37 @@ void DecryptingVideoDecoder::FinishInitialization(bool success) { if (!success) { base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED); - state_ = kStopped; + state_ = kDecodeFinished; return; } // Success! + const AudioDecoderConfig& config = demuxer_stream_->audio_decoder_config(); + bits_per_channel_ = config.bits_per_channel(); + channel_layout_ = config.channel_layout(); + samples_per_second_ = config.samples_per_second(); state_ = kIdle; base::ResetAndReturn(&init_cb_).Run(PIPELINE_OK); } -void DecryptingVideoDecoder::DoRead(const ReadCB& read_cb) { +void DecryptingAudioDecoder::DoRead(const ReadCB& read_cb) { DVLOG(3) << "DoRead()"; DCHECK(message_loop_->BelongsToCurrentThread()); DCHECK(state_ == kIdle || state_ == kDecodeFinished) << state_; DCHECK(!read_cb.is_null()); CHECKā¦ (message too large)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/xhwang@chromium.org/11198017/9012
Change committed as 162976 |