Data type conversion between extensions API and Cast API

Data type conversion between extensions API and Cast API

Converts to and from Cast API types. Also fixed a couple issues to
match the API between Cast and Extensions API.

Cast Extensions API is now fully redirected to Cast native APIs. Minus
the conversion of MediaStreamTrack.

BUG=301920
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233366

[Alpha Left Google, 2013-10-31 23:18:04]

scherkus: chrome/renderer/media
justinlin: everything else

[scherkus (not reviewing), 2013-11-01 00:26:23]

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:47: return true;
this is silly

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:54: return true;
ditto

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:73: &cast_codec_params)) {
fix indent

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:78: return true;
this function always returns true

please s/bool/void/g for these functions and simplify all the code

[Alpha Left Google, 2013-11-01 00:39:19]

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:47: return true;
On 2013/11/01 00:26:23, scherkus wrote:
> this is silly

Yeah returning a boolean is probably not useful here.

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:54: return true;
On 2013/11/01 00:26:23, scherkus wrote:
> ditto

Done.

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:73: &cast_codec_params)) {
On 2013/11/01 00:26:23, scherkus wrote:
> fix indent

Done.

https://codereview.chromium.org/47343005/diff/30001/chrome/renderer/extension...
chrome/renderer/extensions/webrtc_native_handler.cc:78: return true;
On 2013/11/01 00:26:23, scherkus wrote:
> this function always returns true
> 
> please s/bool/void/g for these functions and simplify all the code

Done.

[scherkus (not reviewing), 2013-11-01 01:37:54]

lgtm

[justinlin, 2013-11-01 16:08:21]

lgtm

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/extensio...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:333:
ToCastRtpParams(*params, &cast_params);
Maybe you've already considered this, but why not have a constructor for
CastRtpCaps that just takes the extension object as a param and move the code
above into that Cast classes?

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/media/ca...
File chrome/renderer/media/cast_send_transport.h (right):

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/media/ca...
chrome/renderer/media/cast_send_transport.h:79: typedef CastRtpCaps
CastRtpParams;
q: so CastRtpCaps == CastRtpParams?

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/media/ca...
chrome/renderer/media/cast_send_transport.h:90: // Return capabilities currently
spported by this transport.
typo here

[Alpha Left Google, 2013-11-01 22:27:47]

+kalman: owner review.

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/extensio...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:333:
ToCastRtpParams(*params, &cast_params);
On 2013/11/01 16:08:21, justinlin wrote:
> Maybe you've already considered this, but why not have a constructor for
> CastRtpCaps that just takes the extension object as a param and move the code
> above into that Cast classes?

I would like to separate cast and extensions types. That's why they are handled
here.

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/media/ca...
File chrome/renderer/media/cast_send_transport.h (right):

https://codereview.chromium.org/47343005/diff/120014/chrome/renderer/media/ca...
chrome/renderer/media/cast_send_transport.h:79: typedef CastRtpCaps
CastRtpParams;
On 2013/11/01 16:08:21, justinlin wrote:
> q: so CastRtpCaps == CastRtpParams?


Yes they are the same thing.

[not at google - send to devlin, 2013-11-01 22:45:33]

lgtm, though those comments are applicable in a bunch of places.

https://codereview.chromium.org/47343005/diff/190001/chrome/renderer/extensio...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/47343005/diff/190001/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:71:
cast_params->codec_specific_params.push_back(cast_codec_params);
I'm not confident in C++ to know whether this is a worthwhile optimisation, but
it does seem like you'll end up doing a bit of unnecessary copying here. This
might work:

cast_params->codec_specific_params.resize(
    ext_params.code_specific_params.size());
for (size_t i = 0; i < ext_params.code_specific_params.size();++i) {
  ToCastCodecSpecificParams(*ext_params.code_specific_params[i],
                            &code_specific_params[i])
}

https://codereview.chromium.org/47343005/diff/190001/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:81: *ext_params->ssrc =
cast_params.ssrc;
why not just ext_params->ssrc.reset(new int(cast_params.ssrc))?

[Alpha Left Google, 2013-11-06 00:15:55]

https://codereview.chromium.org/47343005/diff/190001/chrome/renderer/extensio...
File chrome/renderer/extensions/webrtc_native_handler.cc (right):

https://codereview.chromium.org/47343005/diff/190001/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:71:
cast_params->codec_specific_params.push_back(cast_codec_params);
On 2013/11/01 22:45:34, kalman wrote:
> I'm not confident in C++ to know whether this is a worthwhile optimisation,
but
> it does seem like you'll end up doing a bit of unnecessary copying here. This
> might work:
> 
> cast_params->codec_specific_params.resize(
>     ext_params.code_specific_params.size());
> for (size_t i = 0; i < ext_params.code_specific_params.size();++i) {
>   ToCastCodecSpecificParams(*ext_params.code_specific_params[i],
>                             &code_specific_params[i])
> }

Those are fairly small data structures so copying them doesn't have a large
overhead.

https://codereview.chromium.org/47343005/diff/190001/chrome/renderer/extensio...
chrome/renderer/extensions/webrtc_native_handler.cc:81: *ext_params->ssrc =
cast_params.ssrc;
On 2013/11/01 22:45:34, kalman wrote:
> why not just ext_params->ssrc.reset(new int(cast_params.ssrc))?

Done.

[commit-bot: I haz the power, 2013-11-06 01:49:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/47343005/250001

[commit-bot: I haz the power, 2013-11-06 02:37:36]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2013-11-06 06:53:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/47343005/250001

[commit-bot: I haz the power, 2013-11-06 09:25:10]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-06 09:28:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/47343005/250001

[commit-bot: I haz the power, 2013-11-06 11:41:31]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-11-06 18:20:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/hclam@chromium.org/47343005/250001

[commit-bot: I haz the power, 2013-11-06 21:11:20]

Change committed as 233366