Adding C++ versions of currently spec'd "RtpParameters" structs.

Adding C++ versions of currently spec'd "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013, webrtc:7112
Review-Url: https://codereview.webrtc.org/2651883010
Cr-Commit-Position: refs/heads/master@{#16437}
Committed: https://chromium.googlesource.com/external/webrtc/+/e702b30fec47933114161691369bd27a90371862

[Taylor Brandstetter, 2017-01-25 23:22:09]

Description was changed from

==========
Adding C++ versions of currently specified "RtpParameters" structs.

This also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

to

==========
Adding C++ versions of currently specified "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents.

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

[Taylor Brandstetter, 2017-01-26 00:05:20]

deadbeef@webrtc.org changed reviewers:
+ magjed@webrtc.org, tkchin@webrtc.org

[Taylor Brandstetter, 2017-01-26 00:05:20]

magjed: PTAL at Java/JNI code.
tkchin: PTAL at Obj-C code.

I'm making minor tweaks to the "codec parameters" structure, which should
hopefully be safe since I only introduced it recently.

[Taylor Brandstetter, 2017-01-26 00:05:32]

Description was changed from

==========
Adding C++ versions of currently specified "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents.

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

to

==========
Adding C++ versions of currently specified "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

[Taylor Brandstetter, 2017-01-26 00:07:31]

Description was changed from

==========
Adding C++ versions of currently specified "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

to

==========
Adding C++ versions of currently spec'd "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

[tkchin_webrtc, 2017-01-26 05:24:09]

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm (right):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm:54: if
(nativeParameters.kind == cricket::MEDIA_TYPE_AUDIO) {
switch on enum with all cases and rely on compiler error when more are added but
misses this code? Can DCHECK in data case and return nil.

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h (left):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:47:
@property(nonatomic, nonnull) NSString *mimeType;
Do we care about API migration/deprecation for Rtp* files? This is a public API
file.

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h (right):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:47:
@property(nonatomic, readonly, nonnull) NSString *name;
If it's readonly, do we need to expose the strings as constants above or will an
enum suffice?

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:55:
@property(nonatomic, readonly, nonnull) NSString *kind;
since underlying type is enum, does it make sense to make this enum as well?

[magjed_webrtc, 2017-01-26 09:48:06]

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/android/src/jn...
File webrtc/sdk/android/src/jni/peerconnection_jni.cc (right):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/android/src/jn...
webrtc/sdk/android/src/jni/peerconnection_jni.cc:160: static cricket::MediaType
JavaMediaTypeToJsepMediaType(JNIEnv* jni,
Can you keep this code in the existing position during review so that I can see
the diff, and then you can move it here in a final patch set once you get lgtm?

[Taylor Brandstetter, 2017-01-26 19:08:20]

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/android/src/jn...
File webrtc/sdk/android/src/jni/peerconnection_jni.cc (right):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/android/src/jn...
webrtc/sdk/android/src/jni/peerconnection_jni.cc:160: static cricket::MediaType
JavaMediaTypeToJsepMediaType(JNIEnv* jni,
On 2017/01/26 09:48:06, magjed_webrtc wrote:
> Can you keep this code in the existing position during review so that I can
see
> the diff, and then you can move it here in a final patch set once you get
lgtm?

Done; sorry about that.

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm (right):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm:54: if
(nativeParameters.kind == cricket::MEDIA_TYPE_AUDIO) {
On 2017/01/26 05:24:09, tkchin_webrtc wrote:
> switch on enum with all cases and rely on compiler error when more are added
but
> misses this code? Can DCHECK in data case and return nil.

Done.

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h (left):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:47:
@property(nonatomic, nonnull) NSString *mimeType;
On 2017/01/26 05:24:09, tkchin_webrtc wrote:
> Do we care about API migration/deprecation for Rtp* files? This is a public
API
> file.

I couldn't find any code that uses this, so I was hoping I could get away with
just changing it. Otherwise we'd have a confusing situation where "mimeType"
means different things at different layers.

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h (right):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:47:
@property(nonatomic, readonly, nonnull) NSString *name;
On 2017/01/26 05:24:09, tkchin_webrtc wrote:
> If it's readonly, do we need to expose the strings as constants above or will
an
> enum suffice?

An enum doesn't work, because if we support external encoder/decoder factories,
there may be codecs we don't know about.

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:55:
@property(nonatomic, readonly, nonnull) NSString *kind;
On 2017/01/26 05:24:09, tkchin_webrtc wrote:
> since underlying type is enum, does it make sense to make this enum as well?

I was going for consistency with "kind" in RTCMediaStreamTrack.h, but I could go
either way.

[magjed_webrtc, 2017-01-29 22:13:27]

webrtc/sdk/android lgtm

[pthatcher2, 2017-01-31 22:19:05]

pthatcher@chromium.org changed reviewers:
+ pthatcher@chromium.org

[pthatcher2, 2017-01-31 22:19:06]

lgtm

https://codereview.webrtc.org/2651883010/diff/80001/webrtc/api/rtpparameters.h
File webrtc/api/rtpparameters.h (right):

https://codereview.webrtc.org/2651883010/diff/80001/webrtc/api/rtpparameters....
webrtc/api/rtpparameters.h:115: rtc::Optional<int> maxptime;
max_ptime would probably be better.

[Taylor Brandstetter, 2017-02-02 03:34:34]

Pinging Zeke

https://codereview.webrtc.org/2651883010/diff/80001/webrtc/api/rtpparameters.h
File webrtc/api/rtpparameters.h (right):

https://codereview.webrtc.org/2651883010/diff/80001/webrtc/api/rtpparameters....
webrtc/api/rtpparameters.h:115: rtc::Optional<int> maxptime;
On 2017/01/31 22:19:06, pthatcher2 wrote:
> max_ptime would probably be better.

Done.

[tkchin_webrtc, 2017-02-02 20:20:39]

lgtm

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h (left):

https://codereview.webrtc.org/2651883010/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/Headers/WebRTC/RTCRtpCodecParameters.h:47:
@property(nonatomic, nonnull) NSString *mimeType;
On 2017/01/26 19:08:20, Taylor Brandstetter wrote:
> On 2017/01/26 05:24:09, tkchin_webrtc wrote:
> > Do we care about API migration/deprecation for Rtp* files? This is a public
> API
> > file.
> 
> I couldn't find any code that uses this, so I was hoping I could get away with
> just changing it. Otherwise we'd have a confusing situation where "mimeType"
> means different things at different layers.

We should have a better deprecation plan in the future for public APIs (though
the bar can be arbitrarily low - e.g. calling a particular method
experimental/temporary). The issue is that we don't know who might be using
these APIs, even if we don't use them ourselves.

https://codereview.webrtc.org/2651883010/diff/120001/webrtc/sdk/objc/Framewor...
File webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm (right):

https://codereview.webrtc.org/2651883010/diff/120001/webrtc/sdk/objc/Framewor...
webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm:83: } else {
nit: might be safer to else if video, then have a final else with an
RTC_NOTREACHED

[Taylor Brandstetter, 2017-02-03 02:00:49]

https://codereview.webrtc.org/2651883010/diff/120001/webrtc/sdk/objc/Framewor...
File webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm (right):

https://codereview.webrtc.org/2651883010/diff/120001/webrtc/sdk/objc/Framewor...
webrtc/sdk/objc/Framework/Classes/RTCRtpCodecParameters.mm:83: } else {
On 2017/02/02 20:20:38, tkchin_webrtc wrote:
> nit: might be safer to else if video, then have a final else with an
> RTC_NOTREACHED

Done.

[Taylor Brandstetter, 2017-02-03 02:01:24]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2017-02-03 02:01:24]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
pthatcher@chromium.org, tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2651883010/#ps160001 (title:
"Merge with master")

[commit-bot: I haz the power, 2017-02-03 02:01:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-03 02:06:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 02:06:58]

Try jobs failed on following builders:
  ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...)
  ios64_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios9_dbg/buil...)

[hbos, 2017-02-03 16:34:23]

Description was changed from

==========
Adding C++ versions of currently spec'd "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013
==========

to

==========
Adding C++ versions of currently spec'd "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013, webrtc:7112
==========

[Taylor Brandstetter, 2017-02-04 19:09:04]

The CQ bit was checked by deadbeef@webrtc.org

[Taylor Brandstetter, 2017-02-04 19:09:05]

The patchset sent to the CQ was uploaded after l-g-t-m from magjed@webrtc.org,
pthatcher@chromium.org, tkchin@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2651883010/#ps180001 (title:
"Update unit tests (due to switch from special-case values to rtc::Optional)")

[commit-bot: I haz the power, 2017-02-04 19:09:13]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-02-04 20:09:03]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1486235344844610,
"parent_rev": "d1f5fdac5c24204bb90594b81492c33d4acfcc62", "commit_rev":
"e702b30fec47933114161691369bd27a90371862"}

[commit-bot: I haz the power, 2017-02-04 20:09:07]

Description was changed from

==========
Adding C++ versions of currently spec'd "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013, webrtc:7112
==========

to

==========
Adding C++ versions of currently spec'd "RtpParameters" structs.

These structs will be used for ORTC objects (and their WebRTC
equivalents).

This CL also introduces some minor changes to the existing implemented
structs:

- max_bitrate_bps uses rtc::Optional instead of "-1 means unset"
- "mime_type" turned into "name"/"kind" (which can be used to form the
  MIME type string, if needed).
- clock_rate and channels changed to rtc::Optional, since they will
  need to be for RtpSender.send().
- Renamed "channels" to "num_channels" (the ORTC name, which I prefer).

BUG=webrtc:7013, webrtc:7112

Review-Url: https://codereview.webrtc.org/2651883010
Cr-Commit-Position: refs/heads/master@{#16437}
Committed:
https://chromium.googlesource.com/external/webrtc/+/e702b30fec479331141616913...
==========

[commit-bot: I haz the power, 2017-02-04 20:09:08]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/external/webrtc/+/e702b30fec479331141616913...