Fix BWE bug where audio has timestamps in us.

Fix BWE bug where audio has timestamps in us.

The BWE expects arrival timestamps in ms, while the audio path delivered
them in us, causing the BWE to break down under the combined audio/video
BWE experiment. This was introduced in r9892 (https://chromium.googlesource.com/external/webrtc/+/68786d20400f1f3744ad83549325665c18ea9e5b).

BUG=webrtc:4758
R=mflodman@webrtc.org, sprang@webrtc.org

Committed: https://crrev.com/8bffba71072577ef8e81f48b615f231880de25a0
Cr-Commit-Position: refs/heads/master@{#10032}

[stefan-webrtc, 2015-09-23 07:31:05]

stefan@webrtc.org changed reviewers:
+ pbos@webrtc.org

[stefan-webrtc, 2015-09-23 07:31:06]

PTAL

[stefan-webrtc, 2015-09-23 08:39:25]

stefan@webrtc.org changed reviewers:
+ sprang@webrtc.org

[stefan-webrtc, 2015-09-23 08:39:38]

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

[commit-bot: I haz the power, 2015-09-23 08:39:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360913004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360913004/1

[sprang_webrtc, 2015-09-23 08:46:26]

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
File webrtc/video/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
webrtc/video/audio_receive_stream.cc:105: arrival_time_ms =
(packet_time.timestamp + 500) / 1000;
I can't seem to find a unit tests at all for this method. Should we add small
one, at least for this?

[commit-bot: I haz the power, 2015-09-23 08:50:53]

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

[commit-bot: I haz the power, 2015-09-23 08:50:54]

Dry run: Try jobs failed on following builders:
  win_x64_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_gn_dbg/builds/4127)

[stefan-webrtc, 2015-09-23 08:53:07]

On 2015/09/23 08:46:26, språng wrote:
>
https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
> File webrtc/video/audio_receive_stream.cc (right):
> 
>
https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
> webrtc/video/audio_receive_stream.cc:105: arrival_time_ms =
> (packet_time.timestamp + 500) / 1000;
> I can't seem to find a unit tests at all for this method. Should we add small
> one, at least for this?

I was looking for one as well, and couldn't find it. I'll write a simple one
which mocks the remote bitrate estimator.

[mflodman, 2015-09-23 12:11:41]

mflodman@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[mflodman, 2015-09-23 12:11:41]

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
File webrtc/video/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
webrtc/video/audio_receive_stream.cc:105: arrival_time_ms =
(packet_time.timestamp + 500) / 1000;
Not sure I understood correctly, but shouldn't AudioReceiveStream::DeliverRtp
and VideoReceiveStream::DeliverRtp assume the same units in PacketTime?

I.e. I think this should be done earlier than here.

[pbos-webrtc, 2015-09-23 12:18:54]

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
File webrtc/video/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
webrtc/video/audio_receive_stream.cc:105: arrival_time_ms =
(packet_time.timestamp + 500) / 1000;
On 2015/09/23 12:11:41, mflodman wrote:
> Not sure I understood correctly, but shouldn't AudioReceiveStream::DeliverRtp
> and VideoReceiveStream::DeliverRtp assume the same units in PacketTime?
> 
> I.e. I think this should be done earlier than here.

Or IncomingPacket accept a PacketTime?

[stefan-webrtc, 2015-09-23 12:43:32]

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

[commit-bot: I haz the power, 2015-09-23 12:43:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360913004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360913004/20001

[commit-bot: I haz the power, 2015-09-23 12:46:19]

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

[commit-bot: I haz the power, 2015-09-23 12:46:20]

Dry run: Try jobs failed on following builders:
  android_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_dbg/builds/5818)

[stefan-webrtc, 2015-09-23 12:46:53]

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
File webrtc/video/audio_receive_stream.cc (right):

https://codereview.webrtc.org/1360913004/diff/1/webrtc/video/audio_receive_st...
webrtc/video/audio_receive_stream.cc:105: arrival_time_ms =
(packet_time.timestamp + 500) / 1000;
On 2015/09/23 12:18:54, pbos-webrtc wrote:
> On 2015/09/23 12:11:41, mflodman wrote:
> > Not sure I understood correctly, but shouldn't
AudioReceiveStream::DeliverRtp
> > and VideoReceiveStream::DeliverRtp assume the same units in PacketTime?
> > 
> > I.e. I think this should be done earlier than here.
> 
> Or IncomingPacket accept a PacketTime?

The same conversion is done for video in vie_receiver.cc. I could pass in
PacketTime instead, but I don't think it's worth it as PacketTime doesn't make
as much sense on the send-side, and we are moving all of it there.

[mflodman, 2015-09-23 12:47:01]

My mistake, ignore my previous comment.

LGTM

[stefan-webrtc, 2015-09-23 12:48:50]

Erik, could you take a look at the unittest I added?

[stefan-webrtc, 2015-09-23 12:48:55]

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

[commit-bot: I haz the power, 2015-09-23 12:49:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1360913004/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1360913004/40001

[sprang_webrtc, 2015-09-23 13:12:30]

lgtm

[commit-bot: I haz the power, 2015-09-23 13:14:21]

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

[commit-bot: I haz the power, 2015-09-23 13:14:22]

Dry run: Try jobs failed on following builders:
  win_drmemory_light on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_drmemory_light/buil...)

[stefan-webrtc, 2015-09-23 13:54:10]

Committed patchset #3 (id:40001) manually as
8bffba71072577ef8e81f48b615f231880de25a0 (presubmit successful).

[commit-bot: I haz the power, 2015-09-23 13:54:12]

Patchset 3 (id:??) landed as
https://crrev.com/8bffba71072577ef8e81f48b615f231880de25a0
Cr-Commit-Position: refs/heads/master@{#10032}