Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1295603002:
Fixing integer underflow in FileAudioDevice (webrtc issue 4554) (Closed)
|
Created:
5 years, 4 months ago by Alexander Brauckmann Modified:
4 years, 11 months ago CC:
webrtc-reviews_webrtc.org, yujie_mao (webrtc), henrika_webrtc, Andrew MacDonald, tterriberry_mozilla.com, qiang.lu, niklas.enbom Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionFixing integer underflow in FileAudioDevice (webrtc issue 4554)
Problem is described here:
https://code.google.com/p/webrtc/issues/detail?id=4554
Committed: https://crrev.com/bedc17be5c594d9821a08036564cb1db2d0e3723
Cr-Commit-Position: refs/heads/master@{#11174}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 20 (9 generated)
a.brauckmann@gmail.com changed reviewers: + henrikg@webrtc.org, phoglund@webrtc.org
Sorry, I completely missed this. lgtm with nits fixed. https://codereview.webrtc.org/1295603002/diff/1/webrtc/modules/audio_device/d... File webrtc/modules/audio_device/dummy/file_audio_device.cc (right): https://codereview.webrtc.org/1295603002/diff/1/webrtc/modules/audio_device/d... webrtc/modules/audio_device/dummy/file_audio_device.cc:519: if(deltaTimeMillis < 10) { Nit: space between if and (. https://codereview.webrtc.org/1295603002/diff/1/webrtc/modules/audio_device/d... webrtc/modules/audio_device/dummy/file_audio_device.cc:554: if(deltaTimeMillis < 10) { Nit: ditto
Please add a BUG=4554 line in the description, so that it's picked up by the bugdroid. lgtm.
Oh, and I didn't see this until now either. You have to "Publish+Mail Comments" (with empty text) so that a mail is sent out.
Aha, that's why we didn't see it. That's not exactly intuitive but yeah, you need to do that :)
A.Brauckmann: do you plan to land this? Or should it be closed?
The CQ bit was checked by a.brauckmann@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1295603002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1295603002/1
The CQ bit was unchecked by a.brauckmann@gmail.com
The CQ bit was checked by a.brauckmann@gmail.com
The CQ bit was unchecked by a.brauckmann@gmail.com
The CQ bit was checked by a.brauckmann@gmail.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1295603002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1295603002/1
Message was sent while issue was closed.
Description was changed from ========== Fixing integer underflow in FileAudioDevice (webrtc issue 4554) Problem is described here: https://code.google.com/p/webrtc/issues/detail?id=4554 ========== to ========== Fixing integer underflow in FileAudioDevice (webrtc issue 4554) Problem is described here: https://code.google.com/p/webrtc/issues/detail?id=4554 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Fixing integer underflow in FileAudioDevice (webrtc issue 4554) Problem is described here: https://code.google.com/p/webrtc/issues/detail?id=4554 ========== to ========== Fixing integer underflow in FileAudioDevice (webrtc issue 4554) Problem is described here: https://code.google.com/p/webrtc/issues/detail?id=4554 Committed: https://crrev.com/bedc17be5c594d9821a08036564cb1db2d0e3723 Cr-Commit-Position: refs/heads/master@{#11174} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/bedc17be5c594d9821a08036564cb1db2d0e3723 Cr-Commit-Position: refs/heads/master@{#11174}
Message was sent while issue was closed.
Hi, you missed fixing the nits by phoglund@. Please create a follow-up CL for that. (Also missed adding the BUG= line in the description, the issue wasn't updated with the fix. I have added that manually.)
Message was sent while issue was closed.
phoglund@webrtc.org changed reviewers: + a.brauckmann@gmail.com - A.Brauckmann@gmail.com
Message was sent while issue was closed.
Yeah, please fix the nits in a separate CL. LGTM with nits means "feel free to submit if you fix all the nits". |
