Fix an implicit narrowing conversion found by MSVC

Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None

Review-Url: https://codereview.webrtc.org/3003133002
Cr-Commit-Position: refs/heads/master@{#19456}
Committed: https://chromium.googlesource.com/external/webrtc/+/0826fb2b3594ea49238da3800056ea20f1c82c97

[oprypin_webrtc, 2017-08-22 14:18:37]

Description was changed from

==========
Fix an implicit narrowing conversion found by MSVC

BUG=None
==========

to

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None
==========

[oprypin_webrtc, 2017-08-22 14:28:11]

oprypin@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[oprypin_webrtc, 2017-08-22 14:34:56]

Description was changed from

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None
==========

to

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None
NOTRY=True
==========

[oprypin_webrtc, 2017-08-22 14:37:13]

Karl fixed one instance of the problem but then the next one appeared.
It's really hard to iterate on this problem due to the inability to reproduce
it. I'm just gonna try it on the bot with chromium roll, then, if it succeeds,
delete patch set 2 and submit it. If not, back to square one.

[kwiberg-webrtc, 2017-08-22 17:40:24]

lgtm, if you change to checked_cast

https://codereview.webrtc.org/3003133002/diff/1/webrtc/api/audio_codecs/L16/a...
File webrtc/api/audio_codecs/L16/audio_encoder_L16.cc (right):

https://codereview.webrtc.org/3003133002/diff/1/webrtc/api/audio_codecs/L16/a...
webrtc/api/audio_codecs/L16/audio_encoder_L16.cc:24: config.num_channels =
rtc::dchecked_cast<size_t>(format.num_channels);
checked_cast is probably better here, since this value may originate from the
network.

[oprypin_webrtc, 2017-08-22 17:44:07]

On 2017/08/22 17:40:24, kwiberg-webrtc wrote:
> lgtm, if you change to checked_cast
> 
>
https://codereview.webrtc.org/3003133002/diff/1/webrtc/api/audio_codecs/L16/a...
> File webrtc/api/audio_codecs/L16/audio_encoder_L16.cc (right):
> 
>
https://codereview.webrtc.org/3003133002/diff/1/webrtc/api/audio_codecs/L16/a...
> webrtc/api/audio_codecs/L16/audio_encoder_L16.cc:24: config.num_channels =
> rtc::dchecked_cast<size_t>(format.num_channels);
> checked_cast is probably better here, since this value may originate from the
> network.

By the way, my fix did not help. If you expect that checked_cast will fix this,
I can try, otherwise please look into this if you can.

[kwiberg-webrtc, 2017-08-22 17:47:15]

On 2017/08/22 17:44:07, oprypin_webrtc wrote:
> On 2017/08/22 17:40:24, kwiberg-webrtc wrote:
> > lgtm, if you change to checked_cast
> > 
> >
>
https://codereview.webrtc.org/3003133002/diff/1/webrtc/api/audio_codecs/L16/a...
> > File webrtc/api/audio_codecs/L16/audio_encoder_L16.cc (right):
> > 
> >
>
https://codereview.webrtc.org/3003133002/diff/1/webrtc/api/audio_codecs/L16/a...
> > webrtc/api/audio_codecs/L16/audio_encoder_L16.cc:24: config.num_channels =
> > rtc::dchecked_cast<size_t>(format.num_channels);
> > checked_cast is probably better here, since this value may originate from
the
> > network.
> 
> By the way, my fix did not help. If you expect that checked_cast will fix
this,
> I can try, otherwise please look into this if you can.

Ah, right. I think you need to cast to int, not size_t. The RHS is size_t and
thge LHS is int, IIRC.

checked_cast and dchecked_cast have exactly the same effect.

[oprypin_webrtc, 2017-08-22 17:51:34]

Patchset #3 (id:40001) has been deleted

[oprypin_webrtc, 2017-08-22 18:28:37]

OK so now the next error appeared. Someone needs to go through these on a
Windows machine :(

[oprypin_webrtc, 2017-08-22 19:15:29]

On 2017/08/22 18:28:37, oprypin_webrtc wrote:
> OK so now the next error appeared. Someone needs to go through these on a
> Windows machine :(

Seems good, actually.
PTAL again.

[oprypin_webrtc, 2017-08-22 19:36:24]

Description was changed from

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None
NOTRY=True
==========

to

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None
==========

[kwiberg-webrtc, 2017-08-22 20:06:05]

lgtm, but see suggestions

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/api/audio_codecs/...
File webrtc/api/audio_codecs/L16/audio_encoder_L16.cc (right):

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/api/audio_codecs/...
webrtc/api/audio_codecs/L16/audio_encoder_L16.cc:16: #include
"webrtc/rtc_base/ptr_util.h"
#include safe_conversions.h here?

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/call/rtp_demuxer_...
File webrtc/call/rtp_demuxer_unittest.cc (right):

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/call/rtp_demuxer_...
webrtc/call/rtp_demuxer_unittest.cc:349: auto packet =
CreatePacketWithSsrcRsid(rtc::dchecked_cast<uint32_t>(i),
There's almost never a reason to use DCHECK instead of CHECK in tests. We don't
care that much about performance, and we never want the checks skipped.

[oprypin_webrtc, 2017-08-22 20:28:39]

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/api/audio_codecs/...
File webrtc/api/audio_codecs/L16/audio_encoder_L16.cc (right):

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/api/audio_codecs/...
webrtc/api/audio_codecs/L16/audio_encoder_L16.cc:16: #include
"webrtc/rtc_base/ptr_util.h"
On 2017/08/22 20:06:05, kwiberg-webrtc wrote:
> #include safe_conversions.h here?

Done in both places

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/call/rtp_demuxer_...
File webrtc/call/rtp_demuxer_unittest.cc (right):

https://codereview.webrtc.org/3003133002/diff/140001/webrtc/call/rtp_demuxer_...
webrtc/call/rtp_demuxer_unittest.cc:349: auto packet =
CreatePacketWithSsrcRsid(rtc::dchecked_cast<uint32_t>(i),
On 2017/08/22 20:06:05, kwiberg-webrtc wrote:
> There's almost never a reason to use DCHECK instead of CHECK in tests. We
don't
> care that much about performance, and we never want the checks skipped.

Done.

[oprypin_webrtc, 2017-08-22 20:29:49]

The CQ bit was checked by oprypin@webrtc.org

[oprypin_webrtc, 2017-08-22 20:29:50]

The patchset sent to the CQ was uploaded after l-g-t-m from kwiberg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/3003133002/#ps170001 (title:
"no roll")

[commit-bot: I haz the power, 2017-08-22 20:29:57]

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-08-22 20:57:49]

CQ is committing da patch.

Bot data: {"patchset_id": 170001, "attempt_start_ts": 1503433789833880,
"parent_rev": "51d49b49837fe80519f39b9a0b263f82afb12c6c", "commit_rev":
"0826fb2b3594ea49238da3800056ea20f1c82c97"}

[commit-bot: I haz the power, 2017-08-22 20:57:56]

Description was changed from

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None
==========

to

==========
Fix an implicit narrowing conversion found by MSVC

Followup to https://codereview.webrtc.org/2999363002/

BUG=None

Review-Url: https://codereview.webrtc.org/3003133002
Cr-Commit-Position: refs/heads/master@{#19456}
Committed:
https://chromium.googlesource.com/external/webrtc/+/0826fb2b3594ea49238da3800...
==========

[commit-bot: I haz the power, 2017-08-22 20:57:57]

Committed patchset #9 (id:170001) as
https://chromium.googlesource.com/external/webrtc/+/0826fb2b3594ea49238da3800...