Track volume cleanup
Always read and write track volumes atomically. In most places this was
already being done, but there were a couple places where the left and
right channels were read independently.
Changed constant MAX_GAIN_INT to be a uint32_t instead of a float.
It is always used as a uint32_t in comparisons and assignments.
Use MAX_GAIN_INT in more places.
Now that volume is always accessed atomically, removed the union
and alias for uint16_t volume[2], and kept only volumeLR.
Removed volatile as it's meaningless.
In AudioFlinger, clamp the track volumes read from shared memory
before applying master and stream volume.
Change-Id: If65e2b27e5bc3db5bf75540479843041b58433f0
diff --git a/include/private/media/AudioTrackShared.h b/include/private/media/AudioTrackShared.h
index 33a92cd..ffc546e 100644
--- a/include/private/media/AudioTrackShared.h
+++ b/include/private/media/AudioTrackShared.h
@@ -71,10 +71,13 @@
uint32_t loopStart;
uint32_t loopEnd;
int loopCount;
- volatile union {
- uint16_t volume[2];
- uint32_t volumeLR;
- };
+
+ // Channel volumes are fixed point U4.12, so 0x1000 means 1.0.
+ // Left channel is in [0:15], right channel is in [16:31].
+ // Always read and write the combined pair atomically.
+ // For AudioTrack only, not used by AudioRecord.
+ uint32_t volumeLR;
+
uint32_t sampleRate;
// NOTE: audio_track_cblk_t::frameSize is not equal to AudioTrack::frameSize() for
// 8 bit PCM data: in this case, mCblk->frameSize is based on a sample size of
diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp
index 97b2312..7e338de 100644
--- a/media/libmedia/AudioTrack.cpp
+++ b/media/libmedia/AudioTrack.cpp
@@ -480,7 +480,6 @@
mVolume[LEFT] = left;
mVolume[RIGHT] = right;
- // write must be atomic
mCblk->volumeLR = (uint32_t(uint16_t(right * 0x1000)) << 16) | uint16_t(left * 0x1000);
return NO_ERROR;
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 593f558..fb2a072 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -72,7 +72,7 @@
//static const nsecs_t kStandbyTimeInNsecs = seconds(3);
static const float MAX_GAIN = 4096.0f;
-static const float MAX_GAIN_INT = 0x1000;
+static const uint32_t MAX_GAIN_INT = 0x1000;
// retry counts for buffer fill timeout
// 50 * ~20msecs = 1 second
@@ -2190,14 +2190,29 @@
// read original volumes with volume control
float typeVolume = mStreamTypes[track->type()].volume;
float v = masterVolume * typeVolume;
- vl = (uint32_t)(v * cblk->volume[0]) << 12;
- vr = (uint32_t)(v * cblk->volume[1]) << 12;
+ uint32_t vlr = cblk->volumeLR;
+ vl = vlr & 0xFFFF;
+ vr = vlr >> 16;
+ // track volumes come from shared memory, so can't be trusted and must be clamped
+ if (vl > MAX_GAIN_INT) {
+ ALOGV("Track left volume out of range: %04X", vl);
+ vl = MAX_GAIN_INT;
+ }
+ if (vr > MAX_GAIN_INT) {
+ ALOGV("Track right volume out of range: %04X", vr);
+ vr = MAX_GAIN_INT;
+ }
+ // now apply the master volume and stream type volume
+ vl = (uint32_t)(v * vl) << 12;
+ vr = (uint32_t)(v * vr) << 12;
+ // assuming master volume and stream type volume each go up to 1.0,
+ // vl and vr are now in 8.24 format
uint16_t sendLevel = cblk->getSendLevel_U4_12();
// send level comes from shared memory and so may be corrupt
- if (sendLevel >= 0x1000) {
+ if (sendLevel >= MAX_GAIN_INT) {
ALOGV("Track send level out of range: %04X", sendLevel);
- sendLevel = 0x1000;
+ sendLevel = MAX_GAIN_INT;
}
va = (uint32_t)(v * sendLevel);
}
@@ -2217,6 +2232,7 @@
// Convert volumes from 8.24 to 4.12 format
int16_t left, right, aux;
+ // This additional clamping is needed in case chain->setVolume_l() overshot
uint32_t v_clamped = (vl + (1 << 11)) >> 12;
if (v_clamped > MAX_GAIN_INT) v_clamped = MAX_GAIN_INT;
left = int16_t(v_clamped);
@@ -2699,10 +2715,11 @@
} else {
float typeVolume = mStreamTypes[track->type()].volume;
float v = mMasterVolume * typeVolume;
- float v_clamped = v * cblk->volume[0];
+ uint32_t vlr = cblk->volumeLR;
+ float v_clamped = v * (vlr & 0xFFFF);
if (v_clamped > MAX_GAIN) v_clamped = MAX_GAIN;
left = v_clamped/MAX_GAIN;
- v_clamped = v * cblk->volume[1];
+ v_clamped = v * (vlr >> 16);
if (v_clamped > MAX_GAIN) v_clamped = MAX_GAIN;
right = v_clamped/MAX_GAIN;
}
@@ -3436,6 +3453,7 @@
void AudioFlinger::PlaybackThread::Track::dump(char* buffer, size_t size)
{
+ uint32_t vlr = mCblk->volumeLR;
snprintf(buffer, size, " %05d %05d %03u %03u 0x%08x %05u %04u %1d %1d %1d %05u %05u %05u 0x%08x 0x%08x 0x%08x 0x%08x\n",
mName - AudioMixer::TRACK0,
(mClient == NULL) ? getpid() : mClient->pid(),
@@ -3448,8 +3466,8 @@
mMute,
mFillingUpStatus,
mCblk->sampleRate,
- mCblk->volume[0],
- mCblk->volume[1],
+ vlr & 0xFFFF,
+ vlr >> 16,
mCblk->server,
mCblk->user,
(int)mMainBuffer,
@@ -3794,7 +3812,7 @@
if (mCblk != NULL) {
mCblk->flags |= CBLK_DIRECTION_OUT;
mCblk->buffers = (char*)mCblk + sizeof(audio_track_cblk_t);
- mCblk->volume[0] = mCblk->volume[1] = 0x1000;
+ mCblk->volumeLR = (MAX_GAIN_INT << 16) | MAX_GAIN_INT;
mOutBuffer.frameCount = 0;
playbackThread->mTracks.add(this);
ALOGV("OutputTrack constructor mCblk %p, mBuffer %p, mCblk->buffers %p, " \