Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2127413002:
Check for bad floating point rounding in audio_renderer_algorithm. (Closed)
|
Created:
4 years, 5 months ago by Max Morin Modified:
4 years, 5 months ago Reviewers:
DaleCurtis CC:
chromium-reviews, feature-media-reviews_chromium.org, posciak+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCheck for bad floating point rounding in audio_renderer_algorithm.
BUG=622125
Committed: https://crrev.com/4b4d410a51e381550a06eed06eed502a69ef1508
Cr-Commit-Position: refs/heads/master@{#405062}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Simplify. #Messages
Total messages: 18 (7 generated)
Description was changed from ========== Check for bad floating point rounding in audio_renderer_algorithm. BUG=622125 ========== to ========== Check for bad floating point rounding in audio_renderer_algorithm. BUG=622125 ==========
maxmorin@chromium.org changed reviewers: + dalecurtis@chromium.org
Please take a look.
By the way, playback_rate should never be negative, right? If so, I'll add a DCHECK for that.
https://codereview.chromium.org/2127413002/diff/1/media/filters/audio_rendere... File media/filters/audio_renderer_algorithm.cc (left): https://codereview.chromium.org/2127413002/diff/1/media/filters/audio_rendere... media/filters/audio_renderer_algorithm.cc:163: int seek_frames = static_cast<int>(muted_partial_frame_); Don't we just want std::min((int)(muted_partial_frame_), audio_buffer_.frames()); ?
PTAL. I also added a DCHECK for negative playback rates. https://codereview.chromium.org/2127413002/diff/1/media/filters/audio_rendere... File media/filters/audio_renderer_algorithm.cc (left): https://codereview.chromium.org/2127413002/diff/1/media/filters/audio_rendere... media/filters/audio_renderer_algorithm.cc:163: int seek_frames = static_cast<int>(muted_partial_frame_); On 2016/07/08 18:42:48, DaleCurtis wrote: > Don't we just want std::min((int)(muted_partial_frame_), > audio_buffer_.frames()); ? This might leave muted_partial_frame as 1, but since this is a very rare case I shouldn't over-engineer this :). Just using min is much simpler, so it's better.
lgtm
The CQ bit was checked by maxmorin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by maxmorin@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Check for bad floating point rounding in audio_renderer_algorithm. BUG=622125 ========== to ========== Check for bad floating point rounding in audio_renderer_algorithm. BUG=622125 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Check for bad floating point rounding in audio_renderer_algorithm. BUG=622125 ========== to ========== Check for bad floating point rounding in audio_renderer_algorithm. BUG=622125 Committed: https://crrev.com/4b4d410a51e381550a06eed06eed502a69ef1508 Cr-Commit-Position: refs/heads/master@{#405062} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/4b4d410a51e381550a06eed06eed502a69ef1508 Cr-Commit-Position: refs/heads/master@{#405062} |
