Skip to content

Commit e93cccb

Browse files
Eric LaurentAndroid (Google) Code Review
Eric Laurent
authored and
Android (Google) Code Review
committed
Merge "Fix threading issues in AudioRecord JNI"
2 parents 4190a04 + 532bc1c commit e93cccb

File tree

2 files changed

+248
-226
lines changed

2 files changed

+248
-226
lines changed

‎core/jni/android_media_AudioRecord.cpp‎

Lines changed: 96 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
#include <android_runtime/AndroidRuntime.h>
2929

3030
#include <utils/Log.h>
31+
#include <utils/SortedVector.h>
32+
#include <utils/threads.h>
3133
#include <media/AudioRecord.h>
3234
#include <media/mediarecorder.h>
3335

@@ -55,9 +57,12 @@ static fields_t javaAudioRecordFields;
5557
struct audiorecord_callback_cookie {
5658
jclass audioRecord_class;
5759
jobject audioRecord_ref;
58-
};
60+
bool busy;
61+
Condition cond;
62+
};
5963

60-
Mutex sLock;
64+
static Mutex sLock;
65+
static SortedVector <audiorecord_callback_cookie *> sAudioRecordCallBackCookies;
6166

6267
// ----------------------------------------------------------------------------
6368

@@ -87,13 +92,21 @@ jint android_media_translateRecorderErrorCode(int code) {
8792

8893
// ----------------------------------------------------------------------------
8994
static void recorderCallback(int event, void* user, void *info) {
95+
96+
audiorecord_callback_cookie *callbackInfo = (audiorecord_callback_cookie *)user;
97+
{
98+
Mutex::Autolock l(sLock);
99+
if (sAudioRecordCallBackCookies.indexOf(callbackInfo) < 0) {
100+
return;
101+
}
102+
callbackInfo->busy = true;
103+
}
90104
if (event == AudioRecord::EVENT_MORE_DATA) {
91105
// set size to 0 to signal we're not using the callback to read more data
92106
AudioRecord::Buffer* pBuff = (AudioRecord::Buffer*)info;
93107
pBuff->size = 0;
94108

95109
} else if (event == AudioRecord::EVENT_MARKER) {
96-
audiorecord_callback_cookie *callbackInfo = (audiorecord_callback_cookie *)user;
97110
JNIEnv *env = AndroidRuntime::getJNIEnv();
98111
if (user && env) {
99112
env->CallStaticVoidMethod(
@@ -107,7 +120,6 @@ static void recorderCallback(int event, void* user, void *info) {
107120
}
108121

109122
} else if (event == AudioRecord::EVENT_NEW_POS) {
110-
audiorecord_callback_cookie *callbackInfo = (audiorecord_callback_cookie *)user;
111123
JNIEnv *env = AndroidRuntime::getJNIEnv();
112124
if (user && env) {
113125
env->CallStaticVoidMethod(
@@ -120,8 +132,36 @@ static void recorderCallback(int event, void* user, void *info) {
120132
}
121133
}
122134
}
135+
{
136+
Mutex::Autolock l(sLock);
137+
callbackInfo->busy = false;
138+
callbackInfo->cond.broadcast();
139+
}
123140
}
124141

142+
// ----------------------------------------------------------------------------
143+
static sp<AudioRecord> getAudioRecord(JNIEnv* env, jobject thiz)
144+
{
145+
Mutex::Autolock l(sLock);
146+
AudioRecord* const ar =
147+
(AudioRecord*)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
148+
return sp<AudioRecord>(ar);
149+
}
150+
151+
static sp<AudioRecord> setAudioRecord(JNIEnv* env, jobject thiz, const sp<AudioRecord>& ar)
152+
{
153+
Mutex::Autolock l(sLock);
154+
sp<AudioRecord> old =
155+
(AudioRecord*)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
156+
if (ar.get()) {
157+
ar->incStrong(thiz);
158+
}
159+
if (old != 0) {
160+
old->decStrong(thiz);
161+
}
162+
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, (int)ar.get());
163+
return old;
164+
}
125165

126166
// ----------------------------------------------------------------------------
127167
static int
@@ -162,6 +202,12 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
162202
return AUDIORECORD_ERROR_SETUP_INVALIDSOURCE;
163203
}
164204

205+
jclass clazz = env->GetObjectClass(thiz);
206+
if (clazz == NULL) {
207+
ALOGE("Can't find %s when setting up callback.", kClassPathName);
208+
return AUDIORECORD_ERROR_SETUP_NATIVEINITFAILED;
209+
}
210+
165211
if (jSession == NULL) {
166212
ALOGE("Error creating AudioRecord: invalid session ID pointer");
167213
return AUDIORECORD_ERROR;
@@ -176,27 +222,20 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
176222
env->ReleasePrimitiveArrayCritical(jSession, nSession, 0);
177223
nSession = NULL;
178224

179-
audiorecord_callback_cookie *lpCallbackData = NULL;
180-
AudioRecord* lpRecorder = NULL;
181-
182225
// create an uninitialized AudioRecord object
183-
lpRecorder = new AudioRecord();
184-
if (lpRecorder == NULL) {
226+
sp<AudioRecord> lpRecorder = new AudioRecord();
227+
if (lpRecorder == NULL) {
185228
ALOGE("Error creating AudioRecord instance.");
186229
return AUDIORECORD_ERROR_SETUP_NATIVEINITFAILED;
187230
}
188231

189232
// create the callback information:
190233
// this data will be passed with every AudioRecord callback
191-
jclass clazz = env->GetObjectClass(thiz);
192-
if (clazz == NULL) {
193-
ALOGE("Can't find %s when setting up callback.", kClassPathName);
194-
goto native_track_failure;
195-
}
196-
lpCallbackData = new audiorecord_callback_cookie;
234+
audiorecord_callback_cookie *lpCallbackData = new audiorecord_callback_cookie;
197235
lpCallbackData->audioRecord_class = (jclass)env->NewGlobalRef(clazz);
198236
// we use a weak reference so the AudioRecord object can be garbage collected.
199237
lpCallbackData->audioRecord_ref = env->NewGlobalRef(weak_this);
238+
lpCallbackData->busy = false;
200239

201240
lpRecorder->set((audio_source_t) source,
202241
sampleRateInHertz,
@@ -225,9 +264,13 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
225264
env->ReleasePrimitiveArrayCritical(jSession, nSession, 0);
226265
nSession = NULL;
227266

267+
{ // scope for the lock
268+
Mutex::Autolock l(sLock);
269+
sAudioRecordCallBackCookies.add(lpCallbackData);
270+
}
228271
// save our newly created C++ AudioRecord in the "nativeRecorderInJavaObj" field
229272
// of the Java object
230-
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, (int)lpRecorder);
273+
setAudioRecord(env, thiz, lpRecorder);
231274

232275
// save our newly created callback information in the "nativeCallbackCookie" field
233276
// of the Java object (in mNativeCallbackCookie) so we can free the memory in finalize()
@@ -240,11 +283,6 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
240283
env->DeleteGlobalRef(lpCallbackData->audioRecord_class);
241284
env->DeleteGlobalRef(lpCallbackData->audioRecord_ref);
242285
delete lpCallbackData;
243-
244-
native_track_failure:
245-
delete lpRecorder;
246-
247-
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, 0);
248286
env->SetIntField(thiz, javaAudioRecordFields.nativeCallbackCookie, 0);
249287

250288
return AUDIORECORD_ERROR_SETUP_NATIVEINITFAILED;
@@ -256,8 +294,7 @@ android_media_AudioRecord_setup(JNIEnv *env, jobject thiz, jobject weak_this,
256294
static int
257295
android_media_AudioRecord_start(JNIEnv *env, jobject thiz, jint event, jint triggerSession)
258296
{
259-
AudioRecord *lpRecorder =
260-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
297+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
261298
if (lpRecorder == NULL ) {
262299
jniThrowException(env, "java/lang/IllegalStateException", NULL);
263300
return AUDIORECORD_ERROR;
@@ -272,8 +309,7 @@ android_media_AudioRecord_start(JNIEnv *env, jobject thiz, jint event, jint trig
272309
static void
273310
android_media_AudioRecord_stop(JNIEnv *env, jobject thiz)
274311
{
275-
AudioRecord *lpRecorder =
276-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
312+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
277313
if (lpRecorder == NULL ) {
278314
jniThrowException(env, "java/lang/IllegalStateException", NULL);
279315
return;
@@ -285,30 +321,35 @@ android_media_AudioRecord_stop(JNIEnv *env, jobject thiz)
285321

286322

287323
// ----------------------------------------------------------------------------
324+
325+
#define CALLBACK_COND_WAIT_TIMEOUT_MS 1000
288326
static void android_media_AudioRecord_release(JNIEnv *env, jobject thiz) {
327+
sp<AudioRecord> lpRecorder = setAudioRecord(env, thiz, 0);
328+
if (lpRecorder == NULL) {
329+
return;
330+
}
331+
ALOGV("About to delete lpRecorder: %x\n", (int)lpRecorder.get());
332+
lpRecorder->stop();
289333

290-
// serialize access. Ugly, but functional.
291-
Mutex::Autolock lock(&sLock);
292-
AudioRecord *lpRecorder =
293-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
294334
audiorecord_callback_cookie *lpCookie = (audiorecord_callback_cookie *)env->GetIntField(
295335
thiz, javaAudioRecordFields.nativeCallbackCookie);
296336

297337
// reset the native resources in the Java object so any attempt to access
298338
// them after a call to release fails.
299-
env->SetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj, 0);
300339
env->SetIntField(thiz, javaAudioRecordFields.nativeCallbackCookie, 0);
301340

302-
// delete the AudioRecord object
303-
if (lpRecorder) {
304-
ALOGV("About to delete lpRecorder: %x\n", (int)lpRecorder);
305-
lpRecorder->stop();
306-
delete lpRecorder;
307-
}
308-
309341
// delete the callback information
310342
if (lpCookie) {
343+
Mutex::Autolock l(sLock);
311344
ALOGV("deleting lpCookie: %x\n", (int)lpCookie);
345+
while (lpCookie->busy) {
346+
if (lpCookie->cond.waitRelative(sLock,
347+
milliseconds(CALLBACK_COND_WAIT_TIMEOUT_MS)) !=
348+
NO_ERROR) {
349+
break;
350+
}
351+
}
352+
sAudioRecordCallBackCookies.remove(lpCookie);
312353
env->DeleteGlobalRef(lpCookie->audioRecord_class);
313354
env->DeleteGlobalRef(lpCookie->audioRecord_ref);
314355
delete lpCookie;
@@ -327,11 +368,8 @@ static jint android_media_AudioRecord_readInByteArray(JNIEnv *env, jobject thiz
327368
jbyteArray javaAudioData,
328369
jint offsetInBytes, jint sizeInBytes) {
329370
jbyte* recordBuff = NULL;
330-
AudioRecord *lpRecorder = NULL;
331-
332371
// get the audio recorder from which we'll read new audio samples
333-
lpRecorder =
334-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
372+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
335373
if (lpRecorder == NULL) {
336374
ALOGE("Unable to retrieve AudioRecord object, can't record");
337375
return 0;
@@ -378,12 +416,8 @@ static jint android_media_AudioRecord_readInShortArray(JNIEnv *env, jobject thi
378416
// ----------------------------------------------------------------------------
379417
static jint android_media_AudioRecord_readInDirectBuffer(JNIEnv *env, jobject thiz,
380418
jobject jBuffer, jint sizeInBytes) {
381-
AudioRecord *lpRecorder = NULL;
382-
//ALOGV("Entering android_media_AudioRecord_readInBuffer");
383-
384419
// get the audio recorder from which we'll read new audio samples
385-
lpRecorder =
386-
(AudioRecord *)env->GetIntField(thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
420+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
387421
if (lpRecorder==NULL)
388422
return 0;
389423

@@ -411,71 +445,60 @@ static jint android_media_AudioRecord_readInDirectBuffer(JNIEnv *env, jobject t
411445
static jint android_media_AudioRecord_set_marker_pos(JNIEnv *env, jobject thiz,
412446
jint markerPos) {
413447

414-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
415-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
416-
417-
if (lpRecorder) {
418-
return
419-
android_media_translateRecorderErrorCode( lpRecorder->setMarkerPosition(markerPos) );
420-
} else {
448+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
449+
if (lpRecorder == NULL) {
421450
jniThrowException(env, "java/lang/IllegalStateException",
422451
"Unable to retrieve AudioRecord pointer for setMarkerPosition()");
423452
return AUDIORECORD_ERROR;
424453
}
454+
return android_media_translateRecorderErrorCode( lpRecorder->setMarkerPosition(markerPos) );
425455
}
426456

427457

428458
// ----------------------------------------------------------------------------
429459
static jint android_media_AudioRecord_get_marker_pos(JNIEnv *env, jobject thiz) {
430460

431-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
432-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
461+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
433462
uint32_t markerPos = 0;
434463

435-
if (lpRecorder) {
436-
lpRecorder->getMarkerPosition(&markerPos);
437-
return (jint)markerPos;
438-
} else {
464+
if (lpRecorder == NULL) {
439465
jniThrowException(env, "java/lang/IllegalStateException",
440466
"Unable to retrieve AudioRecord pointer for getMarkerPosition()");
441467
return AUDIORECORD_ERROR;
442468
}
469+
lpRecorder->getMarkerPosition(&markerPos);
470+
return (jint)markerPos;
443471
}
444472

445473

446474
// ----------------------------------------------------------------------------
447475
static jint android_media_AudioRecord_set_pos_update_period(JNIEnv *env, jobject thiz,
448476
jint period) {
449477

450-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
451-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
478+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
452479

453-
if (lpRecorder) {
454-
return
455-
android_media_translateRecorderErrorCode( lpRecorder->setPositionUpdatePeriod(period) );
456-
} else {
480+
if (lpRecorder == NULL) {
457481
jniThrowException(env, "java/lang/IllegalStateException",
458482
"Unable to retrieve AudioRecord pointer for setPositionUpdatePeriod()");
459483
return AUDIORECORD_ERROR;
460484
}
485+
return android_media_translateRecorderErrorCode( lpRecorder->setPositionUpdatePeriod(period) );
461486
}
462487

463488

464489
// ----------------------------------------------------------------------------
465490
static jint android_media_AudioRecord_get_pos_update_period(JNIEnv *env, jobject thiz) {
466491

467-
AudioRecord *lpRecorder = (AudioRecord *)env->GetIntField(
468-
thiz, javaAudioRecordFields.nativeRecorderInJavaObj);
492+
sp<AudioRecord> lpRecorder = getAudioRecord(env, thiz);
469493
uint32_t period = 0;
470494

471-
if (lpRecorder) {
472-
lpRecorder->getPositionUpdatePeriod(&period);
473-
return (jint)period;
474-
} else {
495+
if (lpRecorder == NULL) {
475496
jniThrowException(env, "java/lang/IllegalStateException",
476497
"Unable to retrieve AudioRecord pointer for getPositionUpdatePeriod()");
477498
return AUDIORECORD_ERROR;
478499
}
500+
lpRecorder->getPositionUpdatePeriod(&period);
501+
return (jint)period;
479502
}
480503

481504

@@ -486,7 +509,8 @@ static jint android_media_AudioRecord_get_pos_update_period(JNIEnv *env, jobjec
486509
static jint android_media_AudioRecord_get_min_buff_size(JNIEnv *env, jobject thiz,
487510
jint sampleRateInHertz, jint nbChannels, jint audioFormat) {
488511

489-
ALOGV(">> android_media_AudioRecord_get_min_buff_size(%d, %d, %d)", sampleRateInHertz, nbChannels, audioFormat);
512+
ALOGV(">> android_media_AudioRecord_get_min_buff_size(%d, %d, %d)",
513+
sampleRateInHertz, nbChannels, audioFormat);
490514

491515
int frameCount = 0;
492516
status_t result = AudioRecord::getMinFrameCount(&frameCount,

0 commit comments

Comments
 (0)