commit c48ddcfc9113215b35e79c08ed77afb8998bd919
author Wonsik Kim <wonsik@google.com> Mon Feb 11 16:16:57 2019 -0800
committer Wonsik Kim <wonsik@google.com> Tue Feb 12 16:39:12 2019 -0800
tree 350e08797d7cb40d75a8432ec5390b01b2c19133
parent 0e2c6b80cbf498ce376cd356ea65d65ec570f335

CCodec: fix image data update handling

Formats should not be written after it's shared with the client. When
update is needed, duplicate the format to avoid race condition.

Bug: 122987730
Test: run MediaMetadataRetrieverTest#testGetFramesAtIndex for 1000 times
Test: atest CtsMediaTestCases -- --module-arg CtsMediaTestCases:size:small
Change-Id: I6abb5526d8df1e57b70c96f5b32d132e4a5de389

media/codec2/sfplugin/CCodecBufferChannel.cpp

diff --git a/media/codec2/sfplugin/CCodecBufferChannel.cpp b/media/codec2/sfplugin/CCodecBufferChannel.cpp
index 6842fa5..7a10e4c 100644
--- a/media/codec2/sfplugin/CCodecBufferChannel.cpp
+++ b/media/codec2/sfplugin/CCodecBufferChannel.cpp
@@ -255,6 +255,34 @@
         mSkipCutBuffer = scb;
     }
 
+    void handleImageData(const sp<Codec2Buffer> &buffer) {
+        sp<ABuffer> imageDataCandidate = buffer->getImageData();
+        if (imageDataCandidate == nullptr) {
+            return;
+        }
+        sp<ABuffer> imageData;
+        if (!mFormat->findBuffer("image-data", &imageData)
+                || imageDataCandidate->size() != imageData->size()
+                || memcmp(imageDataCandidate->data(), imageData->data(), imageData->size()) != 0) {
+            ALOGD("[%s] updating image-data", mName);
+            sp<AMessage> newFormat = dupFormat();
+            newFormat->setBuffer("image-data", imageDataCandidate);
+            MediaImage2 *img = (MediaImage2*)imageDataCandidate->data();
+            if (img->mNumPlanes > 0 && img->mType != img->MEDIA_IMAGE_TYPE_UNKNOWN) {
+                int32_t stride = img->mPlane[0].mRowInc;
+                newFormat->setInt32(KEY_STRIDE, stride);
+                ALOGD("[%s] updating stride = %d", mName, stride);
+                if (img->mNumPlanes > 1 && stride > 0) {
+                    int32_t vstride = (img->mPlane[1].mOffset - img->mPlane[0].mOffset) / stride;
+                    newFormat->setInt32(KEY_SLICE_HEIGHT, vstride);
+                    ALOGD("[%s] updating vstride = %d", mName, vstride);
+                }
+            }
+            setFormat(newFormat);
+            buffer->setFormat(newFormat);
+        }
+    }
+
 protected:
     sp<SkipCutBuffer> mSkipCutBuffer;
 
@@ -1152,6 +1180,7 @@
             return WOULD_BLOCK;
         }
         submit(c2Buffer);
+        handleImageData(c2Buffer);
         *clientBuffer = c2Buffer;
         ALOGV("[%s] grabbed buffer %zu", mName, *index);
         return OK;
@@ -1250,6 +1279,7 @@
         }
         newBuffer->setFormat(mFormat);
         *index = mImpl.assignSlot(newBuffer);
+        handleImageData(newBuffer);
         *clientBuffer = newBuffer;
         ALOGV("[%s] registered buffer %zu", mName, *index);
         return OK;

media/codec2/sfplugin/Codec2Buffer.cpp

diff --git a/media/codec2/sfplugin/Codec2Buffer.cpp b/media/codec2/sfplugin/Codec2Buffer.cpp
index 2dec42e..13b63c9 100644
--- a/media/codec2/sfplugin/Codec2Buffer.cpp
+++ b/media/codec2/sfplugin/Codec2Buffer.cpp
@@ -84,17 +84,7 @@
 }
 
 void Codec2Buffer::setImageData(const sp<ABuffer> &imageData) {
-    meta()->setBuffer("image-data", imageData);
-    format()->setBuffer("image-data", imageData);
-    MediaImage2 *img = (MediaImage2*)imageData->data();
-    if (img->mNumPlanes > 0 && img->mType != img->MEDIA_IMAGE_TYPE_UNKNOWN) {
-        int32_t stride = img->mPlane[0].mRowInc;
-        format()->setInt32(KEY_STRIDE, stride);
-        if (img->mNumPlanes > 1 && stride > 0) {
-            int32_t vstride = (img->mPlane[1].mOffset - img->mPlane[0].mOffset) / stride;
-            format()->setInt32(KEY_SLICE_HEIGHT, vstride);
-        }
-    }
+    mImageData = imageData;
 }
 
 // LocalLinearBuffer
@@ -546,7 +536,6 @@
     : Codec2Buffer(format, buffer),
       mView(view),
       mBlock(block),
-      mImageData(imageData),
       mWrapped(wrapped) {
     setImageData(imageData);
 }
@@ -683,9 +672,7 @@
       mView(std::move(view)),
       mBufferRef(buffer),
       mWrapped(wrapped) {
-    if (imageData != nullptr) {
-        setImageData(imageData);
-    }
+    setImageData(imageData);
 }
 
 std::shared_ptr<C2Buffer> ConstGraphicBlockBuffer::asC2Buffer() {

media/codec2/sfplugin/Codec2Buffer.h

diff --git a/media/codec2/sfplugin/Codec2Buffer.h b/media/codec2/sfplugin/Codec2Buffer.h
index 481975f..dd618aa 100644
--- a/media/codec2/sfplugin/Codec2Buffer.h
+++ b/media/codec2/sfplugin/Codec2Buffer.h
@@ -23,6 +23,7 @@
 #include <android/hardware/cas/native/1.0/types.h>
 #include <binder/IMemory.h>
 #include <media/hardware/VideoAPI.h>
+#include <media/stagefright/foundation/ABuffer.h>
 #include <media/MediaCodecBuffer.h>
 #include <media/ICrypto.h>
 
@@ -85,6 +86,8 @@
         return false;
     }
 
+    sp<ABuffer> getImageData() const { return mImageData; }
+
 protected:
     /**
      * canCopy() implementation for linear buffers.
@@ -100,6 +103,8 @@
      * sets MediaImage data for flexible graphic buffers
      */
     void setImageData(const sp<ABuffer> &imageData);
+
+    sp<ABuffer> mImageData;
 };
 
 /**
@@ -239,7 +244,6 @@
 
     C2GraphicView mView;
     std::shared_ptr<C2GraphicBlock> mBlock;
-    sp<ABuffer> mImageData;
     const bool mWrapped;
 };