From 26f4672670b0e804aa9885297dc8abd3bf6c4804 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Sun, 17 Jan 2021 21:19:43 -0500
Subject: [PATCH] opensl: fix crashes

Change-Id: I790fd96b17309d2203d5ba18b675d998124b758a
---
 src/media/audio/opensl/audio_player.cpp |  4 +++-
 src/media/audio/opensl/opensllayer.cpp  | 23 ++++++++++++++++++++++-
 src/media/audio/opensl/opensllayer.h    |  4 ++--
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/src/media/audio/opensl/audio_player.cpp b/src/media/audio/opensl/audio_player.cpp
index 5fc9b50a5f..7760c803f3 100644
--- a/src/media/audio/opensl/audio_player.cpp
+++ b/src/media/audio/opensl/audio_player.cpp
@@ -197,6 +197,7 @@ bool
 AudioPlayer::start()
 {
     JAMI_DBG("OpenSL playback start");
+    std::unique_lock<std::mutex> lk(m_);
     SLuint32 state;
     SLresult result = (*playItf_)->GetPlayState(playItf_, &state);
     if (result != SL_RESULT_SUCCESS)
@@ -211,6 +212,7 @@ AudioPlayer::start()
         (*playBufferQueueItf_)->Enqueue(playBufferQueueItf_, silentBuf_.buf_, silentBuf_.size_));
     devShadowQueue_.push(&silentBuf_);
 
+    lk.unlock();
     result = (*playItf_)->SetPlayState(playItf_, SL_PLAYSTATE_PLAYING);
     SLASSERT(result);
 
@@ -233,13 +235,13 @@ AudioPlayer::stop()
     JAMI_DBG("OpenSL playback stop");
     SLuint32 state;
 
+    std::lock_guard<std::mutex> lk(m_);
     SLresult result = (*playItf_)->GetPlayState(playItf_, &state);
     SLASSERT(result);
 
     if (state == SL_PLAYSTATE_STOPPED)
         return;
 
-    std::lock_guard<std::mutex> lk(m_);
     callback_ = {};
     result = (*playItf_)->SetPlayState(playItf_, SL_PLAYSTATE_STOPPED);
     SLASSERT(result);
diff --git a/src/media/audio/opensl/opensllayer.cpp b/src/media/audio/opensl/opensllayer.cpp
index 2558cff64a..9766ff9469 100644
--- a/src/media/audio/opensl/opensllayer.cpp
+++ b/src/media/audio/opensl/opensllayer.cpp
@@ -112,7 +112,7 @@ OpenSLLayer::startStream(AudioDeviceType stream)
 void
 OpenSLLayer::stopStream(AudioDeviceType stream)
 {
-    std::unique_lock<std::mutex> lock(mutex_);
+    std::lock_guard<std::mutex> lock(mutex_);
     JAMI_WARN("Stopping OpenSL audio layer for type %u", (unsigned) stream);
 
     if (stream == AudioDeviceType::PLAYBACK) {
@@ -152,6 +152,7 @@ allocateSampleBufs(unsigned count, size_t sizeInByte)
 void
 OpenSLLayer::initAudioEngine()
 {
+    JAMI_WARN("OpenSL init started");
     std::vector<int32_t> hw_infos;
     hw_infos.reserve(4);
     emitSignal<DRing::ConfigurationSignal::GetHardwareAudioFormat>(&hw_infos);
@@ -159,6 +160,7 @@ OpenSLLayer::initAudioEngine()
     hardwareBuffSize_ = hw_infos[1];
     hardwareFormatAvailable(hardwareFormat_, hardwareBuffSize_);
 
+    std::lock_guard<std::mutex> lock(mutex_);
     SLASSERT(slCreateEngine(&engineObject_, 0, nullptr, 0, nullptr, nullptr));
     SLASSERT((*engineObject_)->Realize(engineObject_, SL_BOOLEAN_FALSE));
     SLASSERT((*engineObject_)->GetInterface(engineObject_, SL_IID_ENGINE, &engineInterface_));
@@ -171,11 +173,13 @@ OpenSLLayer::initAudioEngine()
         freeRingBufQueue_.push(&bufs_[i]);
     for (int i = 2 * BUF_COUNT; i < 3 * BUF_COUNT; i++)
         freeRecBufQueue_.push(&bufs_[i]);
+    JAMI_WARN("OpenSL init ended");
 }
 
 void
 OpenSLLayer::shutdownAudioEngine()
 {
+    JAMI_DBG("Stopping OpenSL");
     stopAudioCapture();
     freeRecBufQueue_.clear();
     recBufQueue_.clear();
@@ -241,6 +245,15 @@ OpenSLLayer::engineServicePlay()
         if (auto dat = getToPlay(hardwareFormat_, hardwareBuffSize_)) {
             buf->size_ = dat->pointer()->nb_samples * dat->pointer()->channels
                          * sizeof(AudioSample);
+            if (buf->size_ > buf->cap_) {
+                JAMI_ERR("buf->size_(%zu) > buf->cap_(%zu)", buf->size_, buf->cap_);
+                break;
+            }
+            if (not dat->pointer()->data[0] or not buf->buf_) {
+                JAMI_ERR("null bufer %p -> %p %d", dat->pointer()->data[0], buf->buf_, dat->pointer()->nb_samples);
+                break;
+            }
+            //JAMI_ERR("std::copy_n %p -> %p %zu", dat->pointer()->data[0], buf->buf_, dat->pointer()->nb_samples);
             std::copy_n((const AudioSample*) dat->pointer()->data[0],
                         dat->pointer()->nb_samples,
                         (AudioSample*) buf->buf_);
@@ -264,6 +277,14 @@ OpenSLLayer::engineServiceRing()
         if (auto dat = getToRing(hardwareFormat_, hardwareBuffSize_)) {
             buf->size_ = dat->pointer()->nb_samples * dat->pointer()->channels
                          * sizeof(AudioSample);
+            if (buf->size_ > buf->cap_) {
+                JAMI_ERR("buf->size_(%zu) > buf->cap_(%zu)", buf->size_, buf->cap_);
+                break;
+            }
+            if (not dat->pointer()->data[0] or not buf->buf_) {
+                JAMI_ERR("null bufer %p -> %p %d", dat->pointer()->data[0], buf->buf_, dat->pointer()->nb_samples);
+                break;
+            }
             std::copy_n((const AudioSample*) dat->pointer()->data[0],
                         dat->pointer()->nb_samples,
                         (AudioSample*) buf->buf_);
diff --git a/src/media/audio/opensl/opensllayer.h b/src/media/audio/opensl/opensllayer.h
index a3ab566e84..2f3e562160 100644
--- a/src/media/audio/opensl/opensllayer.h
+++ b/src/media/audio/opensl/opensllayer.h
@@ -150,6 +150,8 @@ private:
     AudioFormat hardwareFormat_ {AudioFormat::MONO()};
     size_t hardwareBuffSize_ {BUFFER_SIZE};
 
+    std::vector<sample_buf> bufs_ {};
+
     AudioQueue freePlayBufQueue_ {BUF_COUNT};
     AudioQueue playBufQueue_ {BUF_COUNT};
 
@@ -159,8 +161,6 @@ private:
     AudioQueue freeRecBufQueue_ {BUF_COUNT}; // Owner of the queue
     AudioQueue recBufQueue_ {BUF_COUNT};     // Owner of the queue
 
-    std::vector<sample_buf> bufs_ {};
-
     std::unique_ptr<opensl::AudioPlayer> player_ {};
     std::unique_ptr<opensl::AudioPlayer> ringtone_ {};
     std::unique_ptr<opensl::AudioRecorder> recorder_ {};
-- 
GitLab