AcmReceiver::InsertPacket and NetEq::InsertPacket: Take ArrayView arguments

AcmReceiver::InsertPacket and NetEq::InsertPacket: Take ArrayView arguments

Instead of separate pointer and size arguments.

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

[kwiberg-webrtc, 2015-11-06 23:43:23]

Patchset #1 (id:1) has been deleted

[kwiberg-webrtc, 2015-11-06 23:43:37]

Patchset #1 (id:20001) has been deleted

[kwiberg-webrtc, 2015-11-06 23:49:46]

Patchset #1 (id:40001) has been deleted

[kwiberg-webrtc, 2015-11-07 00:34:50]

Description was changed from

==========
AcmReceiver::InsertPacket and NetEq::InsertPacket: Take ArrayView arguments

Instead of separate pointer and size arguments.
==========

to

==========
AcmReceiver::InsertPacket and NetEq::InsertPacket: Take ArrayView arguments

Instead of separate pointer and size arguments.
==========

[kwiberg-webrtc, 2015-11-07 00:34:50]

kwiberg@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[kwiberg-webrtc, 2015-11-07 00:35:28]

PTAL

[hlundin-webrtc, 2015-11-10 10:14:35]

lgtm with a nit and a question (demanding a satisfying answer or a fix).

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/acm_receiver.cc (right):

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/acm_receiver.cc:173: const Decoder*
decoder = RtpHeaderToDecoder(*header, incoming_payload[0]);
What happens if it is empty?

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/neteq_impl.cc (right):

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/neteq_impl.cc:451: LOG_F(LS_ERROR) << "payload
== NULL";
"payload == NULL" is not as correct as it used to be.

[kwiberg-webrtc, 2015-11-10 10:27:59]

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/acm_receiver.cc (right):

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/acm_receiver.cc:173: const Decoder*
decoder = RtpHeaderToDecoder(*header, incoming_payload[0]);
On 2015/11/10 10:14:35, hlundin-webrtc wrote:
> What happens if it is empty?

We'll fail a DCHECK, and then proceed to read out of bounds. (The old behavior
was to do the exact same illegal read but without DCHECKing first.)

Would you like an improvement here? (I was thinking of
DCHECK(incoming_payload.size() >= 1).) As part of this CL?

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/neteq/neteq_impl.cc (right):

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/neteq/neteq_impl.cc:451: LOG_F(LS_ERROR) << "payload
== NULL";
On 2015/11/10 10:14:35, hlundin-webrtc wrote:
> "payload == NULL" is not as correct as it used to be.

Done.

[hlundin-webrtc, 2015-11-11 14:55:53]

lgtm

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
File webrtc/modules/audio_coding/main/acm2/acm_receiver.cc (right):

https://codereview.webrtc.org/1429943004/diff/60001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/main/acm2/acm_receiver.cc:173: const Decoder*
decoder = RtpHeaderToDecoder(*header, incoming_payload[0]);
On 2015/11/10 10:27:58, kwiberg-webrtc wrote:
> On 2015/11/10 10:14:35, hlundin-webrtc wrote:
> > What happens if it is empty?
> 
> We'll fail a DCHECK, and then proceed to read out of bounds. (The old behavior
> was to do the exact same illegal read but without DCHECKing first.)
> 
> Would you like an improvement here? (I was thinking of
> DCHECK(incoming_payload.size() >= 1).) As part of this CL?

Let's keep it as it is.

[kwiberg-webrtc, 2015-11-11 16:29:21]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2015-11-11 16:29:31]

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

[commit-bot: I haz the power, 2015-11-11 18:34:04]

Committed patchset #2 (id:80001)

[commit-bot: I haz the power, 2015-11-11 18:34:17]

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