[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

BUG= internal b/25701982

Committed: https://crrev.com/a95f19786b3b0e18e9b5d188384c7263e892742f
Cr-Commit-Position: refs/heads/master@{#362848}

[kmackay, 2015-12-02 19:41:41]

kmackay@chromium.org changed reviewers:
+ dalecurtis@chromium.org, halliwell@chromium.org, slan@chromium.org

[halliwell, 2015-12-02 19:51:08]

On 2015/12/02 19:41:41, kmackay wrote:

Conventionally we word commit messages for upstreaming as if it was new code
added for first time (rather than talking about moving code out of internal).

[halliwell, 2015-12-02 19:51:24]

https://codereview.chromium.org/1494713002/diff/1/chromecast/media/base/cast_...
File chromecast/media/base/cast_audio_decoder.h (right):

https://codereview.chromium.org/1494713002/diff/1/chromecast/media/base/cast_...
chromecast/media/base/cast_audio_decoder.h:6: #define
CHROMECAST_INTERNAL_MEDIA_DECODER_CAST_AUDIO_DECODER_H_
nit: update header guard

https://codereview.chromium.org/1494713002/diff/1/chromecast/media/base/cast_...
chromecast/media/base/cast_audio_decoder.h:20: }
I think CastAudioDecoder should also be in chromecast::media namespace.

[kmackay, 2015-12-02 19:52:26]

nvm, I am probably going to move the decoder stuff into
chromecast/media/cma/decoder instead.

[DaleCurtis, 2015-12-02 19:58:08]

Presumably you just need my approval for the DEPS change? lgtm if so.

[kmackay, 2015-12-02 20:05:41]

On 2015/12/02 19:58:08, DaleCurtis wrote:
> Presumably you just need my approval for the DEPS change? lgtm if so.

Yes, for DEPS, thanks.

[kmackay, 2015-12-02 21:11:53]

https://codereview.chromium.org/1494713002/diff/1/chromecast/media/base/cast_...
File chromecast/media/base/cast_audio_decoder.h (right):

https://codereview.chromium.org/1494713002/diff/1/chromecast/media/base/cast_...
chromecast/media/base/cast_audio_decoder.h:6: #define
CHROMECAST_INTERNAL_MEDIA_DECODER_CAST_AUDIO_DECODER_H_
On 2015/12/02 19:51:23, halliwell wrote:
> nit: update header guard

Done.

https://codereview.chromium.org/1494713002/diff/1/chromecast/media/base/cast_...
chromecast/media/base/cast_audio_decoder.h:20: }
On 2015/12/02 19:51:23, halliwell wrote:
> I think CastAudioDecoder should also be in chromecast::media namespace.

Done.

[slan, 2015-12-02 21:55:26]

Code looks pretty high quality, no major comments.

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
File chromecast/media/cma/decoder/cast_audio_decoder_linux.cc (right):

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc:67: TRACE_EVENT0("cma",
"CastAudioDecoderImpl::Initialize");
nit: are trace events needed?

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc:93: #if
!defined(CHROMECAST_BUILD)
hmmm, i am suspicious that this is needed upstream.... What's going on here?

[kmackay, 2015-12-02 21:58:49]

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
File chromecast/media/cma/decoder/cast_audio_decoder_linux.cc (right):

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc:67: TRACE_EVENT0("cma",
"CastAudioDecoderImpl::Initialize");
On 2015/12/02 21:55:26, slan wrote:
> nit: are trace events needed?

Probably not, I'm not sure why they were added.

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc:93: #if
!defined(CHROMECAST_BUILD)
On 2015/12/02 21:55:26, slan wrote:
> hmmm, i am suspicious that this is needed upstream.... What's going on here?

Looks like there was a compatibility issue between m47 and ToT. I will remove
the #if here and add a shim.

[kmackay, 2015-12-02 22:05:39]

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
File chromecast/media/cma/decoder/cast_audio_decoder_linux.cc (right):

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc:67: TRACE_EVENT0("cma",
"CastAudioDecoderImpl::Initialize");
On 2015/12/02 21:58:49, kmackay wrote:
> On 2015/12/02 21:55:26, slan wrote:
> > nit: are trace events needed?
> 
> Probably not, I'm not sure why they were added.

Done.

https://codereview.chromium.org/1494713002/diff/20001/chromecast/media/cma/de...
chromecast/media/cma/decoder/cast_audio_decoder_linux.cc:93: #if
!defined(CHROMECAST_BUILD)
On 2015/12/02 21:58:49, kmackay wrote:
> On 2015/12/02 21:55:26, slan wrote:
> > hmmm, i am suspicious that this is needed upstream.... What's going on here?
> 
> Looks like there was a compatibility issue between m47 and ToT. I will remove
> the #if here and add a shim.

Done.

[slan, 2015-12-02 22:17:52]

lgtm

[halliwell, 2015-12-03 00:29:43]

On 2015/12/02 22:17:52, slan wrote:
> lgtm

lgtm

[kmackay, 2015-12-03 00:38:04]

The CQ bit was checked by kmackay@chromium.org

[kmackay, 2015-12-03 00:38:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1494713002/#ps40001
(title: "remove tracing and compatibility #ifdef")

[commit-bot: I haz the power, 2015-12-03 00:39:18]

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

[kmackay, 2015-12-03 01:13:30]

Description was changed from

==========
[Chromecast] Move CastAudioDecoder out to chromecast/media/base

Also moved DecoderBufferBase into chromecast/media/base so that we don't
have chromecast/media/base depending on chromecast/media/cma/base.

BUG= internal b/25701982
==========

to

==========
[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

BUG= internal b/25701982
==========

[commit-bot: I haz the power, 2015-12-03 01:37:52]

Description was changed from

==========
[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

BUG= internal b/25701982
==========

to

==========
[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

BUG= internal b/25701982
==========

[commit-bot: I haz the power, 2015-12-03 01:37:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-03 01:39:31]

Description was changed from

==========
[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

BUG= internal b/25701982
==========

to

==========
[Chromecast] Move CastAudioDecoder out to chromecast/media/cma/decoder

BUG= internal b/25701982

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

[commit-bot: I haz the power, 2015-12-03 01:39:31]

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