Fix the video call crash in processOutputBuffer

1) Change the mutex logic to cover the ImsMediaSourceNode Stop method and ImageReader, Codec and Camera callback.
2) Remove the mutex in IVideoSourceNode to prevent the thread block between Stop() and OnUplinkEvent() invoked after Stop called.
3) Remove the direct frame delivery using recording surface buffer between camera and encoder.

Bug: 270023503
Test: Verified Video Call using ImsMediaTestingApp, atest ImsMediaNativeTests
Change-Id: Ib5fbdb26c44ecb483cfcfb21602a7e38c3c80664

3 files changed, 134 insertions, 169 deletions

diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/include/video/android/ImsMediaVideoSource.h b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/include/video/android/ImsMediaVideoSource.h
index 3bc63092..f74a9caf 100644
--- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/include/video/android/ImsMediaVideoSource.h
+++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/include/video/android/ImsMediaVideoSource.h
@@ -153,11 +153,10 @@ private:
     ANativeWindow* mWindow;
     AMediaCodec* mCodec;
     AMediaFormat* mFormat;
-    ANativeWindow* mRecordingSurface;
     ANativeWindow* mImageReaderSurface;
     AImageReader* mImageReader;
     std::mutex mMutex;
-    std::mutex mImageReaderMutex;
+    ImsMediaCondition mConditionExit;
     IVideoSourceCallback* mListener;
     ImsMediaPauseImageSource mPauseImageSource;
     int32_t mCodecType;
diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/android/ImsMediaVideoSource.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/android/ImsMediaVideoSource.cpp
index b8d69951..a035443f 100644
--- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/android/ImsMediaVideoSource.cpp
+++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/android/ImsMediaVideoSource.cpp
@@ -30,7 +30,6 @@ ImsMediaVideoSource::ImsMediaVideoSource()
     mWindow = nullptr;
     mCodec = nullptr;
     mFormat = nullptr;
-    mRecordingSurface = nullptr;
     mImageReaderSurface = nullptr;
     mImageReader = nullptr;
     mCodecType = -1;
@@ -164,7 +163,6 @@ void ImsMediaVideoSource::SetDeviceOrientation(const uint32_t degree)
 bool ImsMediaVideoSource::Start()
 {
     IMLOGD1("[Start], VideoMode[%d]", mVideoMode);
-    mRecordingSurface = nullptr;
 
     if (mVideoMode == kVideoModeRecording || mVideoMode == kVideoModePauseImage)
     {
@@ -214,29 +212,12 @@ bool ImsMediaVideoSource::Start()
             return false;
         }
 
-        if (mWidth > mHeight)  // Is Landscape Mode
-        {
-            err = AMediaCodec_createInputSurface(mCodec, &mRecordingSurface);
+        mImageReaderSurface = CreateImageReader(mWidth, mHeight);
 
-            if (err != AMEDIA_OK)
-            {
-                IMLOGE1("[Start] create input surface error[%d]", err);
-                AMediaCodec_delete(mCodec);
-                mCodec = nullptr;
-                AMediaFormat_delete(mFormat);
-                mFormat = nullptr;
-                return false;
-            }
-        }
-        else
+        if (mImageReaderSurface == nullptr)
         {
-            mImageReaderSurface = CreateImageReader(mWidth, mHeight);
-
-            if (mImageReaderSurface == nullptr)
-            {
-                IMLOGE0("[Start] create image reader failed");
-                return false;
-            }
+            IMLOGE0("[Start] create image reader failed");
+            return false;
         }
 
         err = AMediaCodec_start(mCodec);
@@ -268,9 +249,7 @@ bool ImsMediaVideoSource::Start()
             return false;
         }
 
-        ANativeWindow* recording = mRecordingSurface ? mRecordingSurface : mImageReaderSurface;
-
-        if (mCamera->CreateSession(mWindow, recording) == false)
+        if (mCamera->CreateSession(mWindow, mImageReaderSurface) == false)
         {
             IMLOGE0("[Start] error create camera session");
             AMediaCodec_delete(mCodec);
@@ -295,14 +274,13 @@ bool ImsMediaVideoSource::Start()
     else if (mVideoMode == kVideoModePauseImage)
     {
         mPauseImageSource.Initialize(mWidth, mHeight);
-    }
-
-    // start encoder output thread
-    if (mCodec != nullptr)
-    {
-        mStopped = false;
-        std::thread t1(&ImsMediaVideoSource::processOutputBuffer, this);
-        t1.detach();
+        // start encoder output thread
+        if (mCodec != nullptr)
+        {
+            mStopped = false;
+            std::thread t1(&ImsMediaVideoSource::EncodePauseImage, this);
+            t1.detach();
+        }
     }
 
     mDeviceOrientation = -1;
@@ -314,19 +292,19 @@ void ImsMediaVideoSource::Stop()
 {
     IMLOGD0("[Stop]");
 
-    mMutex.lock();
+    std::lock_guard<std::mutex> guard(mMutex);
     mStopped = true;
-    mMutex.unlock();
 
-    if (mCamera != nullptr)
+    if (mImageReader != nullptr)
     {
-        mCamera->StopSession();
+        AImageReader_delete(mImageReader);
+        mImageReader = nullptr;
+        mImageReaderSurface = nullptr;
     }
 
-    IMLOGD0("[Stop] deinitialize camera");
-
     if (mCamera != nullptr)
     {
+        mCamera->StopSession();
         mCamera->DeleteSession();
         mCamera->DeInitialize();
         mCamera = nullptr;
@@ -334,19 +312,12 @@ void ImsMediaVideoSource::Stop()
 
     if (mCodec != nullptr)
     {
-        if (mRecordingSurface != nullptr)
+        if (mVideoMode == kVideoModePauseImage)
         {
-            AMediaCodec_signalEndOfInputStream(mCodec);
+            mConditionExit.wait_timeout(mFramerate != 0 ? 1000 / mFramerate : 66);
         }
 
         AMediaCodec_stop(mCodec);
-
-        if (mRecordingSurface != nullptr)
-        {
-            ANativeWindow_release(mRecordingSurface);
-            mRecordingSurface = nullptr;
-        }
-
         AMediaCodec_delete(mCodec);
         mCodec = nullptr;
     }
@@ -357,15 +328,10 @@ void ImsMediaVideoSource::Stop()
         mFormat = nullptr;
     }
 
-    if (mImageReader != nullptr)
+    if (mVideoMode == kVideoModePauseImage)
     {
-        std::lock_guard<std::mutex> guard(mImageReaderMutex);
-        AImageReader_delete(mImageReader);
-        mImageReader = nullptr;
-        mImageReaderSurface = nullptr;
+        mPauseImageSource.Uninitialize();
     }
-
-    mPauseImageSource.Uninitialize();
 }
 
 bool ImsMediaVideoSource::IsStopped()
@@ -376,7 +342,7 @@ bool ImsMediaVideoSource::IsStopped()
 
 void ImsMediaVideoSource::onCameraFrame(AImage* pImage)
 {
-    std::lock_guard<std::mutex> guard(mImageReaderMutex);
+    std::lock_guard<std::mutex> guard(mMutex);
 
     if (mImageReader == nullptr || pImage == nullptr)
     {
@@ -404,31 +370,40 @@ void ImsMediaVideoSource::onCameraFrame(AImage* pImage)
         AImage_getPlaneData(pImage, 0, &yPlane, &ylen);
         AImage_getPlaneData(pImage, 1, &uvPlane, &uvlen);
 
-        int32_t facing, sensorOrientation;
-        mCamera->GetSensorOrientation(mCameraId, &facing, &sensorOrientation);
-
-        switch (facing)
+        if (mWidth > mHeight)  // landscape mode, copy without rotate
         {
-            case ACAMERA_LENS_FACING_FRONT:
-            {
-                ImsMediaImageRotate::YUV420_SP_Rotate270(
-                        encoderBuf, yPlane, uvPlane, width, height);
-            }
-            break;
+            memcpy(encoderBuf, yPlane, ylen);
+            memcpy(encoderBuf + ylen, uvPlane, uvlen);
+        }
+        else
+        {
+            int32_t facing, sensorOrientation;
+            mCamera->GetSensorOrientation(mCameraId, &facing, &sensorOrientation);
 
-            case ACAMERA_LENS_FACING_BACK:
+            switch (facing)
             {
-                ImsMediaImageRotate::YUV420_SP_Rotate90(encoderBuf, yPlane, uvPlane, width, height);
-            }
-            break;
+                case ACAMERA_LENS_FACING_FRONT:
+                {
+                    ImsMediaImageRotate::YUV420_SP_Rotate270(
+                            encoderBuf, yPlane, uvPlane, width, height);
+                }
+                break;
 
-            case ACAMERA_LENS_FACING_EXTERNAL:
-            {
-                uint32_t size = width * height;
-                memcpy(encoderBuf, yPlane, size);
-                memcpy(encoderBuf + size, uvPlane, size / 2);
+                case ACAMERA_LENS_FACING_BACK:
+                {
+                    ImsMediaImageRotate::YUV420_SP_Rotate90(
+                            encoderBuf, yPlane, uvPlane, width, height);
+                }
+                break;
+
+                case ACAMERA_LENS_FACING_EXTERNAL:
+                {
+                    uint32_t size = width * height;
+                    memcpy(encoderBuf, yPlane, size);
+                    memcpy(encoderBuf + size, uvPlane, size / 2);
+                }
+                break;
             }
-            break;
         }
 
         IMLOGD_PACKET1(IM_PACKET_LOG_VIDEO, "[onCameraFrame] queue buffer size[%d]", ylen + uvlen);
@@ -440,6 +415,8 @@ void ImsMediaVideoSource::onCameraFrame(AImage* pImage)
     {
         IMLOGE1("[onCameraFrame] dequeueInputBuffer returned index[%d]", index);
     }
+
+    processOutputBuffer();
 }
 
 void ImsMediaVideoSource::changeBitrate(const uint32_t bitrate)
@@ -484,29 +461,8 @@ void ImsMediaVideoSource::requestIdrFrame()
 
 void ImsMediaVideoSource::EncodePauseImage()
 {
-    auto index = AMediaCodec_dequeueInputBuffer(mCodec, CODEC_TIMEOUT_NANO);
-    if (index >= 0)
-    {
-        size_t buffCapacity = 0;
-        uint8_t* encoderBuf = AMediaCodec_getInputBuffer(mCodec, index, &buffCapacity);
-        if (!encoderBuf || !buffCapacity)
-        {
-            IMLOGE1("[EncodePauseImage] returned null buffer pointer or buffCapacity[%d]",
-                    buffCapacity);
-            return;
-        }
-        size_t len = mPauseImageSource.GetYuvImage(encoderBuf, buffCapacity);
-        AMediaCodec_queueInputBuffer(
-                mCodec, index, 0, len, ImsMediaTimer::GetTimeInMicroSeconds(), 0);
-    }
-    else
-    {
-        IMLOGE1("[EncodePauseImage] dequeueInputBuffer returned index[%d]", index);
-    }
-}
+    IMLOGD0("[EncodePauseImage] start");
 
-void ImsMediaVideoSource::processOutputBuffer()
-{
     uint32_t nextTime = ImsMediaTimer::GetTimeInMilliSeconds();
     uint32_t timeInterval = 66;
 
@@ -515,74 +471,37 @@ void ImsMediaVideoSource::processOutputBuffer()
         timeInterval = 1000 / mFramerate;
     }
 
-    IMLOGD2("[processOutputBuffer] interval[%d] CameraId[%d]", timeInterval, mCameraId);
-
-    for (;;)
+    while (!IsStopped())
     {
-        if (IsStopped())
-        {
-            IMLOGD0("[processOutputBuffer] terminated");
-            break;
-        }
-
-        if (mVideoMode == kVideoModePauseImage)
-        {
-            EncodePauseImage();
-        }
-
-        AMediaCodecBufferInfo info;
-        auto index = AMediaCodec_dequeueOutputBuffer(mCodec, &info, CODEC_TIMEOUT_NANO);
+        mMutex.lock();
+        auto index = AMediaCodec_dequeueInputBuffer(mCodec, CODEC_TIMEOUT_NANO);
 
         if (index >= 0)
         {
-            IMLOGD_PACKET5(IM_PACKET_LOG_VIDEO,
-                    "[processOutputBuffer] index[%d], size[%d], offset[%d], time[%ld], flags[%d]",
-                    index, info.size, info.offset, info.presentationTimeUs, info.flags);
-
-            if (info.size > 0)
-            {
-                size_t buffCapacity;
-                uint8_t* buf = AMediaCodec_getOutputBuffer(mCodec, index, &buffCapacity);
-
-                if (IsStopped())
-                {
-                    break;
-                }
-
-                if (buf != nullptr && buffCapacity > 0)
-                {
-                    if (mListener != nullptr)
-                    {
-                        mListener->OnUplinkEvent(
-                                buf + info.offset, info.size, info.presentationTimeUs, info.flags);
-                    }
-                }
-
-                AMediaCodec_releaseOutputBuffer(mCodec, index, false);
-            }
-        }
-        else if (index == AMEDIACODEC_INFO_OUTPUT_BUFFERS_CHANGED)
-        {
-            IMLOGI0("[processOutputBuffer] Encoder output buffer changed");
-        }
-        else if (index == AMEDIACODEC_INFO_OUTPUT_FORMAT_CHANGED)
-        {
-            if (mFormat != nullptr)
+            size_t buffCapacity = 0;
+            uint8_t* encoderBuf = AMediaCodec_getInputBuffer(mCodec, index, &buffCapacity);
+            if (!encoderBuf || !buffCapacity)
             {
-                AMediaFormat_delete(mFormat);
+                IMLOGE1("[EncodePauseImage] returned null buffer pointer or buffCapacity[%d]",
+                        buffCapacity);
+                return;
             }
 
-            mFormat = AMediaCodec_getOutputFormat(mCodec);
-            IMLOGI1("[processOutputBuffer] Encoder format changed, format[%s]",
-                    AMediaFormat_toString(mFormat));
+            size_t len = mPauseImageSource.GetYuvImage(encoderBuf, buffCapacity);
+            AMediaCodec_queueInputBuffer(
+                    mCodec, index, 0, len, ImsMediaTimer::GetTimeInMicroSeconds(), 0);
         }
-        else if (index == AMEDIACODEC_INFO_TRY_AGAIN_LATER)
+        else
         {
-            IMLOGD_PACKET0(IM_PACKET_LOG_VIDEO, "[processOutputBuffer] no output buffer");
+            IMLOGE1("[EncodePauseImage] dequeueInputBuffer returned index[%d]", index);
         }
-        else
+
+        processOutputBuffer();
+        mMutex.unlock();
+
+        if (IsStopped())
         {
-            IMLOGI1("[processOutputBuffer] unexpected index[%d]", index);
+            break;
         }
 
         nextTime += timeInterval;
@@ -591,28 +510,77 @@ void ImsMediaVideoSource::processOutputBuffer()
         if (nextTime > nCurrTime)
         {
             uint32_t timeDiff = nextTime - nCurrTime;
-            IMLOGD_PACKET1(IM_PACKET_LOG_VIDEO, "[processOutputBuffer] timeDiff[%u]", timeDiff);
+            IMLOGD_PACKET1(IM_PACKET_LOG_VIDEO, "[EncodePauseImage] timeDiff[%u]", timeDiff);
             ImsMediaTimer::Sleep(timeDiff);
         }
     }
 
-    IMLOGD0("[processOutputBuffer] exit");
+    IMLOGD0("[EncodePauseImage] end");
+    mConditionExit.signal();
 }
 
-static void ImageCallback(void* context, AImageReader* reader)
+void ImsMediaVideoSource::processOutputBuffer()
 {
-    if (context == nullptr)
+    AMediaCodecBufferInfo info;
+    auto index = AMediaCodec_dequeueOutputBuffer(mCodec, &info, CODEC_TIMEOUT_NANO);
+
+    if (index >= 0)
     {
-        return;
+        IMLOGD_PACKET5(IM_PACKET_LOG_VIDEO,
+                "[processOutputBuffer] index[%d], size[%d], offset[%d], time[%ld], flags[%d]",
+                index, info.size, info.offset, info.presentationTimeUs, info.flags);
+
+        if (info.size > 0)
+        {
+            size_t buffCapacity;
+            uint8_t* buf = AMediaCodec_getOutputBuffer(mCodec, index, &buffCapacity);
+
+            if (buf != nullptr && buffCapacity > 0)
+            {
+                if (mListener != nullptr)
+                {
+                    mListener->OnUplinkEvent(
+                            buf + info.offset, info.size, info.presentationTimeUs, info.flags);
+                }
+            }
+
+            AMediaCodec_releaseOutputBuffer(mCodec, index, false);
+        }
+    }
+    else if (index == AMEDIACODEC_INFO_OUTPUT_BUFFERS_CHANGED)
+    {
+        IMLOGI0("[processOutputBuffer] Encoder output buffer changed");
     }
+    else if (index == AMEDIACODEC_INFO_OUTPUT_FORMAT_CHANGED)
+    {
+        if (mFormat != nullptr)
+        {
+            AMediaFormat_delete(mFormat);
+        }
 
-    ImsMediaVideoSource* pVideoSource = static_cast<ImsMediaVideoSource*>(context);
+        mFormat = AMediaCodec_getOutputFormat(mCodec);
+        IMLOGI1("[processOutputBuffer] Encoder format changed, format[%s]",
+                AMediaFormat_toString(mFormat));
+    }
+    else if (index == AMEDIACODEC_INFO_TRY_AGAIN_LATER)
+    {
+        IMLOGD_PACKET0(IM_PACKET_LOG_VIDEO, "[processOutputBuffer] no output buffer");
+    }
+    else
+    {
+        IMLOGI1("[processOutputBuffer] unexpected index[%d]", index);
+    }
+}
 
-    if (pVideoSource->IsStopped())
+static void ImageCallback(void* context, AImageReader* reader)
+{
+    if (context == nullptr)
     {
         return;
     }
 
+    ImsMediaVideoSource* pVideoSource = static_cast<ImsMediaVideoSource*>(context);
+
     AImage* image = nullptr;
     auto status = AImageReader_acquireNextImage(reader, &image);
 
diff --git a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/nodes/IVideoSourceNode.cpp b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/nodes/IVideoSourceNode.cpp
index 65b429e2..a54dc377 100644
--- a/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/nodes/IVideoSourceNode.cpp
+++ b/service/src/com/android/telephony/imsmedia/lib/libimsmedia/core/video/nodes/IVideoSourceNode.cpp
@@ -57,7 +57,6 @@ kBaseNodeId IVideoSourceNode::GetNodeId()
 ImsMediaResult IVideoSourceNode::Start()
 {
     IMLOGD3("[Start] codec[%d], mode[%d], cameraId[%d]", mCodecType, mVideoMode, mCameraId);
-    std::lock_guard<std::mutex> guard(mMutex);
 
     if (mVideoSource)
     {
@@ -99,7 +98,6 @@ ImsMediaResult IVideoSourceNode::Start()
 void IVideoSourceNode::Stop()
 {
     IMLOGD0("[Stop]");
-    std::lock_guard<std::mutex> guard(mMutex);
 
     if (mVideoSource)
     {
@@ -263,9 +261,9 @@ void IVideoSourceNode::UpdateSurface(ANativeWindow* window)
     mWindow = window;
 }
 
-void IVideoSourceNode::OnUplinkEvent(uint8_t* data, uint32_t size, int64_t timestamp, uint32_t flag)
+void IVideoSourceNode::OnUplinkEvent(
+        uint8_t* data, uint32_t size, int64_t timestamp, uint32_t /*flag*/)
 {
-    (void)flag;
     IMLOGD_PACKET2(
             IM_PACKET_LOG_VIDEO, "[OnUplinkEvent] size[%zu], timestamp[%ld]", size, timestamp);
     std::lock_guard<std::mutex> guard(mMutex);