Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

Since we can now have multiple H264 payload type, we need to move all of them to the beginning of the codec list, instead of greedily taking the first payload type that matches the preferred codec name.

BUG=webrtc:6738
Review-Url: https://codereview.webrtc.org/2658573009
Cr-Commit-Position: refs/heads/master@{#16349}
Committed: https://chromium.googlesource.com/external/webrtc/+/93f01be4fd463f98c08e99d87aa350fb5567ddbf

[magjed_webrtc, 2017-01-27 14:38:29]

The CQ bit was checked by magjed@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 14:38:31]

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

[magjed_webrtc, 2017-01-27 14:45:34]

magjed@webrtc.org changed reviewers:
+ sakal@webrtc.org

[magjed_webrtc, 2017-01-27 14:45:35]

Sami - please take a look.

[magjed_webrtc, 2017-01-27 14:49:35]

Description was changed from

==========
Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

BUG=webrtc:6738
==========

to

==========
Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

Since we can now have multiple H264 payload type, we need to move all of them to
the beginning of the codec list, instead of greedily taking the first payload
type that matches the preferred codec name.

BUG=webrtc:6738
==========

[sakal, 2017-01-27 15:05:15]

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java
(right):

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java:975:
// Returns the line number containing "m=audio|video", or -1 if no such line
exists.
nit: Use javadoc

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java:1017:
newLineParts.addAll(preferredPayloadTypes);
This will add all preferred payload types regardless if they originally existed
in the string?

[magjed_webrtc, 2017-01-27 15:16:44]

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java
(right):

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java:975:
// Returns the line number containing "m=audio|video", or -1 if no such line
exists.
On 2017/01/27 15:05:15, sakal wrote:
> nit: Use javadoc

Done.

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java:1017:
newLineParts.addAll(preferredPayloadTypes);
On 2017/01/27 15:05:15, sakal wrote:
> This will add all preferred payload types regardless if they originally
existed
> in the string?
The preferred payload types are expected to exist in the string. This is how we
use it from the preferCodec() function.

[sakal, 2017-01-27 15:21:44]

lgtm

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
File webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java
(right):

https://codereview.webrtc.org/2658573009/diff/20001/webrtc/examples/androidap...
webrtc/examples/androidapp/src/org/appspot/apprtc/PeerConnectionClient.java:1017:
newLineParts.addAll(preferredPayloadTypes);
On 2017/01/27 15:16:43, magjed_webrtc wrote:
> On 2017/01/27 15:05:15, sakal wrote:
> > This will add all preferred payload types regardless if they originally
> existed
> > in the string?
> The preferred payload types are expected to exist in the string. This is how
we
> use it from the preferCodec() function.

Ok, apparently SDP is required to use all codec types it defines.

[magjed_webrtc, 2017-01-29 22:07:29]

The CQ bit was checked by magjed@webrtc.org

[commit-bot: I haz the power, 2017-01-29 22:07:37]

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-01-29 23:14:18]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485727649519840,
"parent_rev": "c7d928b9ed310668036a013c3b40cab15594d96f", "commit_rev":
"93f01be4fd463f98c08e99d87aa350fb5567ddbf"}

[commit-bot: I haz the power, 2017-01-29 23:14:20]

Description was changed from

==========
Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

Since we can now have multiple H264 payload type, we need to move all of them to
the beginning of the codec list, instead of greedily taking the first payload
type that matches the preferred codec name.

BUG=webrtc:6738
==========

to

==========
Android AppRTCMobile: Fix SDP video codec reordering for multiple H264 profiles

Since we can now have multiple H264 payload type, we need to move all of them to
the beginning of the codec list, instead of greedily taking the first payload
type that matches the preferred codec name.

BUG=webrtc:6738

Review-Url: https://codereview.webrtc.org/2658573009
Cr-Commit-Position: refs/heads/master@{#16349}
Committed:
https://chromium.googlesource.com/external/webrtc/+/93f01be4fd463f98c08e99d87...
==========

[commit-bot: I haz the power, 2017-01-29 23:14:21]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/93f01be4fd463f98c08e99d87...