Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(109)

Issue 614903007: Fix race in audio playback. (Closed)

Created:
6 years, 2 months ago by rkc
Modified:
6 years, 2 months ago
Reviewers:
xiyuan
CC:
chromium-reviews, Charlie
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Fix race in audio playback. This is a race condition that gets triggered if we get *both* audible and inaudible directives in the same instruction. This causes us to encode both the tokens, but when ProcessNextTransmit gets called after the samples for the first token are received, it tries to play the other type of token also (for which samples may not have been received yet). The simplest way to fix it is adding a HasKey check to the cache for both types of tokens. We will get called after each of the samples tokens are added to their respective caches, so if we skip playing one particular type of token (since it hasn't landed in its cache yet), we will get called again after it gets added to its cache, so both the tokens will definitely play. R=xiyuan@chromium.org BUG=419585 Committed: https://crrev.com/60cbacb9f8741c70fdf4212a5e954f7df489196c Cr-Commit-Position: refs/heads/master@{#297844}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -2 lines) Patch
M components/copresence/handlers/audio/audio_directive_handler.cc View 2 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (1 generated)
rkc
6 years, 2 months ago (2014-10-02 00:04:15 UTC) #1
xiyuan
lgtm
6 years, 2 months ago (2014-10-02 00:39:38 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/614903007/1
6 years, 2 months ago (2014-10-02 14:55:20 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1) as 165746f5960d4f8ef4e541b11fc352c9492a71ef
6 years, 2 months ago (2014-10-02 15:57:58 UTC) #5
commit-bot: I haz the power
6 years, 2 months ago (2014-10-02 15:58:38 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/60cbacb9f8741c70fdf4212a5e954f7df489196c
Cr-Commit-Position: refs/heads/master@{#297844}

Powered by Google App Engine
This is Rietveld 408576698