Implement MojoAudioDecoderService

Implement MojoAudioDecoderService

Implemented the plumbing between the service and AudioDecoder except for
the actual data transfer.

BUG=542910
Committed: https://crrev.com/aa65f0b204028f560fb98c3c7ab2f2eeca3aaca2
Cr-Commit-Position: refs/heads/master@{#382184}

[Tima Vaisburd, 2016-03-18 22:38:34]

timav@chromium.org changed reviewers:
+ sandersd@chromium.org, xhwang@chromium.org

[Tima Vaisburd, 2016-03-18 22:38:34]

PTAL.

[xhwang, 2016-03-18 23:01:13]

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
File media/mojo/services/mojo_audio_decoder_service.cc (right):

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:33: LOG(ERROR) <<
"AudioDecoderService: encrypted streams are not supported";
s/LOG(ERROR)/NOTREACHED()/

This should never happen since we also check at the other side.

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:41: initialize_callback_ =
callback;
You can also bind |callback| in OnInitialized(). See example:
https://code.google.com/p/chromium/codesearch#chromium/src/media/mojo/service...

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:43:
decoder_->Initialize(config.To<media::AudioDecoderConfig>(),
Add a TODO to get CdmContext from cdm_id.

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:46: base::Unretained(this)),
base::Unretained() is not safe. You need weak pointers.

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
File media/mojo/services/mojo_audio_decoder_service.h (right):

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.h:62: // These callbacks sends
info to the Mojo client.
The |client_| has a special meaning. These callbacks actually are not sent
through the |client_| but are just callbacks of mojo calls.

[Tima Vaisburd, 2016-03-19 03:09:06]

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
File media/mojo/services/mojo_audio_decoder_service.cc (right):

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:33: LOG(ERROR) <<
"AudioDecoderService: encrypted streams are not supported";
On 2016/03/18 23:01:13, xhwang wrote:
> s/LOG(ERROR)/NOTREACHED()/
> 
> This should never happen since we also check at the other side.

Done.

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:41: initialize_callback_ =
callback;
On 2016/03/18 23:01:13, xhwang wrote:
> You can also bind |callback| in OnInitialized(). See example:
>
https://code.google.com/p/chromium/codesearch#chromium/src/media/mojo/service...

Done. This seems possible for the client side (i.e. MojoAudioDecoder) too. Shall
we do it there as well?

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:43:
decoder_->Initialize(config.To<media::AudioDecoderConfig>(),
On 2016/03/18 23:01:13, xhwang wrote:
> Add a TODO to get CdmContext from cdm_id.

Done.

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.cc:46: base::Unretained(this)),
On 2016/03/18 23:01:13, xhwang wrote:
> base::Unretained() is not safe. You need weak pointers.

Done.

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
File media/mojo/services/mojo_audio_decoder_service.h (right):

https://chromiumcodereview.appspot.com/1810293005/diff/1/media/mojo/services/...
media/mojo/services/mojo_audio_decoder_service.h:62: // These callbacks sends
info to the Mojo client.
On 2016/03/18 23:01:13, xhwang wrote:
> The |client_| has a special meaning. These callbacks actually are not sent
> through the |client_| but are just callbacks of mojo calls.

These variables got removed, the corresponding callbacks are now bound into
media::AudioDecoder callbacks.

[xhwang, 2016-03-19 03:45:53]

lgtm

https://chromiumcodereview.appspot.com/1810293005/diff/20001/media/mojo/servi...
File media/mojo/services/mojo_audio_decoder_service.cc (right):

https://chromiumcodereview.appspot.com/1810293005/diff/20001/media/mojo/servi...
media/mojo/services/mojo_audio_decoder_service.cc:37: NOTREACHED();  // The
client should prevent this situation.
nit: You can still use a error message for better logging:

NOTREACHED() << "....";

[Tima Vaisburd, 2016-03-19 04:10:34]

https://chromiumcodereview.appspot.com/1810293005/diff/20001/media/mojo/servi...
File media/mojo/services/mojo_audio_decoder_service.cc (right):

https://chromiumcodereview.appspot.com/1810293005/diff/20001/media/mojo/servi...
media/mojo/services/mojo_audio_decoder_service.cc:37: NOTREACHED();  // The
client should prevent this situation.
On 2016/03/19 03:45:53, xhwang wrote:
> nit: You can still use a error message for better logging:
> 
> NOTREACHED() << "....";

Done.

[Tima Vaisburd, 2016-03-19 04:10:42]

The CQ bit was checked by timav@chromium.org

[Tima Vaisburd, 2016-03-19 04:10:42]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1810293005/#ps40001
(title: "Added log for NOTREACHED()")

[commit-bot: I haz the power, 2016-03-19 04:10:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1810293005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1810293005/40001

[commit-bot: I haz the power, 2016-03-19 04:52:11]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-19 05:03:25]

Description was changed from

==========
Implement MojoAudioDecoderService

Implemented the plumbing between the service and AudioDecoder except for
the actual data transfer.

BUG=542910
==========

to

==========
Implement MojoAudioDecoderService

Implemented the plumbing between the service and AudioDecoder except for
the actual data transfer.

BUG=542910

Committed: https://crrev.com/aa65f0b204028f560fb98c3c7ab2f2eeca3aaca2
Cr-Commit-Position: refs/heads/master@{#382184}
==========

[commit-bot: I haz the power, 2016-03-19 05:03:26]

Patchset 3 (id:??) landed as
https://crrev.com/aa65f0b204028f560fb98c3c7ab2f2eeca3aaca2
Cr-Commit-Position: refs/heads/master@{#382184}