Chromecast: communicate HDMI-sink codec changes over IPC.

Chromecast: communicate HDMI-sink codec changes over IPC.

R=lcwu@chromium.org,servolk@chromium.org,tsepez@chromium.org
BUG=

Committed: https://crrev.com/7ee951aad36bcd3d9cb5c1ef5ce504822a53073e
Cr-Commit-Position: refs/heads/master@{#319078}

[gunsch, 2015-03-02 21:58:10]

gunsch@chromium.org changed reviewers:
+ tsepez@chromium.org

[gunsch, 2015-03-02 21:58:31]

On 2015/03/02 21:57:48, gunsch wrote:

+tsepez for IPC security review of cast_messages.h

[gunsch, 2015-03-02 21:58:32]

On 2015/03/02 21:57:48, gunsch wrote:

+tsepez for IPC security review of cast_messages.h

[Tom Sepez, 2015-03-02 22:06:15]

https://codereview.chromium.org/971913002/diff/20001/chromecast/common/media/...
File chromecast/common/media/cast_messages.h (right):

https://codereview.chromium.org/971913002/diff/20001/chromecast/common/media/...
chromecast/common/media/cast_messages.h:17: int /* Codec support */)
How is codec support defined?  Should it be an enum?

[gunsch, 2015-03-02 22:12:41]

https://codereview.chromium.org/971913002/diff/20001/chromecast/common/media/...
File chromecast/common/media/cast_messages.h (right):

https://codereview.chromium.org/971913002/diff/20001/chromecast/common/media/...
chromecast/common/media/cast_messages.h:17: int /* Codec support */)
On 2015/03/02 22:06:14, Tom Sepez wrote:
> How is codec support defined?  Should it be an enum?

It's a bitmask of the enum values defined in
https://codereview.chromium.org/971913002/diff/20001/chromecast/media/base/me...
(added comments to describe)

[Tom Sepez, 2015-03-02 22:13:37]

On 2015/03/02 22:12:41, gunsch wrote:
>
https://codereview.chromium.org/971913002/diff/20001/chromecast/common/media/...
> File chromecast/common/media/cast_messages.h (right):
> 
>
https://codereview.chromium.org/971913002/diff/20001/chromecast/common/media/...
> chromecast/common/media/cast_messages.h:17: int /* Codec support */)
> On 2015/03/02 22:06:14, Tom Sepez wrote:
> > How is codec support defined?  Should it be an enum?
> 
> It's a bitmask of the enum values defined in
>
https://codereview.chromium.org/971913002/diff/20001/chromecast/media/base/me...
> (added comments to describe)
Ah, OK.  Probably fine to use an int then. LGTM

[lcwu1, 2015-03-03 20:11:11]

https://codereview.chromium.org/971913002/diff/40001/chromecast/common/media/...
File chromecast/common/media/cast_message_generator.h (right):

https://codereview.chromium.org/971913002/diff/40001/chromecast/common/media/...
chromecast/common/media/cast_message_generator.h:7: #include
"chromecast/common/media/cast_messages.h"
It doesn't seem necessary to have multiple xxx_message_generator.[h|cc] here.
Could we merge cast_message_generator and cma_message_generator into one that
includes both cast_message.h and cma_message.h? (Having one message_generator
that includes multiple message headers also appear to be a common pattern in
both //content/ and //chrome/.)

(We might generate additional methods for cast_messages that are not used if we
combine the two, but I think that should be fine.)

https://codereview.chromium.org/971913002/diff/40001/chromecast/media/base/me...
File chromecast/media/base/media_caps.cc (right):

https://codereview.chromium.org/971913002/diff/40001/chromecast/media/base/me...
chromecast/media/base/media_caps.cc:10: int hdmi_codecs = 0;
s/hdmi_codecs/g_hdmi_codecs

https://codereview.chromium.org/971913002/diff/40001/chromecast/renderer/cast...
File chromecast/renderer/cast_content_renderer_client.cc (right):

https://codereview.chromium.org/971913002/diff/40001/chromecast/renderer/cast...
chromecast/renderer/cast_content_renderer_client.cc:117: &hdmi_codecs_mask)) {
Indentation

[gunsch, 2015-03-04 02:35:47]

https://codereview.chromium.org/971913002/diff/40001/chromecast/common/media/...
File chromecast/common/media/cast_message_generator.h (right):

https://codereview.chromium.org/971913002/diff/40001/chromecast/common/media/...
chromecast/common/media/cast_message_generator.h:7: #include
"chromecast/common/media/cast_messages.h"
The choice to keep them separate isn't for idiomatic or stylistic reasons, but
rather because this file is common to both the Android/Linux cast_shell builds,
and the IPC traits we define for some of the common ::media structs cause
re-definition errors at link time.

I tried solving this awhile ago to be able to unconditionally include all the
CMA code (even though it's unused on Android), but the technical roadblock is
that we would have to move the IPC traits somewhere common to share them ---
currently they're content-private.

https://codereview.chromium.org/971913002/diff/40001/chromecast/media/base/me...
File chromecast/media/base/media_caps.cc (right):

https://codereview.chromium.org/971913002/diff/40001/chromecast/media/base/me...
chromecast/media/base/media_caps.cc:10: int hdmi_codecs = 0;
On 2015/03/03 20:11:11, lcwu1 wrote:
> s/hdmi_codecs/g_hdmi_codecs

Done.

https://codereview.chromium.org/971913002/diff/40001/chromecast/renderer/cast...
File chromecast/renderer/cast_content_renderer_client.cc (right):

https://codereview.chromium.org/971913002/diff/40001/chromecast/renderer/cast...
chromecast/renderer/cast_content_renderer_client.cc:117: &hdmi_codecs_mask)) {
On 2015/03/03 20:11:11, lcwu1 wrote:
> Indentation

Done.

[lcwu1, 2015-03-04 02:49:29]

lgtm

[gunsch, 2015-03-04 17:17:21]

The CQ bit was checked by gunsch@chromium.org

[gunsch, 2015-03-04 17:17:22]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/971913002/#ps60001 (title:
"rebase, address lcwu comments")

[commit-bot: I haz the power, 2015-03-04 17:17:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/971913002/60001

[commit-bot: I haz the power, 2015-03-04 17:21:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-04 17:22:11]

Patchset 4 (id:??) landed as
https://crrev.com/7ee951aad36bcd3d9cb5c1ef5ce504822a53073e
Cr-Commit-Position: refs/heads/master@{#319078}