stagefright: Fix race condition between MediaCodec and SoftwareRenderer

* Set the buffer's format info when it's returned from OMX component.
* Move frame format meta from SoftwareRenderer's ctor to the render call.
  I.e. each frame sent to the renderer carries the format info.
* Reset renderer with the new format instead of re-creating
  SoftwareRenderer when incoming frame's format is changed.

Bug: 13842676
Change-Id: Ibab46f109200bcbdeab13a4cc1bcd0870f2a99fb

diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp
index cd05c54..ab8ac79 100644 (file)
--- a/media/libstagefright/AwesomePlayer.cpp
+++ b/media/libstagefright/AwesomePlayer.cpp
@@ -103,8 +103,9 @@ private:
 
 struct AwesomeLocalRenderer : public AwesomeRenderer {
     AwesomeLocalRenderer(
-            const sp<ANativeWindow> &nativeWindow, const sp<MetaData> &meta)
-        : mTarget(new SoftwareRenderer(nativeWindow, meta)) {
+            const sp<ANativeWindow> &nativeWindow, const sp<AMessage> &format)
+        : mFormat(format),
+          mTarget(new SoftwareRenderer(nativeWindow)) {
     }
 
     virtual void render(MediaBuffer *buffer) {
@@ -116,7 +117,7 @@ struct AwesomeLocalRenderer : public AwesomeRenderer {
     }
 
     void render(const void *data, size_t size, int64_t timestampNs) {
-        mTarget->render(data, size, timestampNs, NULL);
+        mTarget->render(data, size, timestampNs, NULL, mFormat);
     }
 
 protected:
@@ -126,6 +127,7 @@ protected:
     }
 
 private:
+    sp<AMessage> mFormat;
     SoftwareRenderer *mTarget;
 
     AwesomeLocalRenderer(const AwesomeLocalRenderer &);
@@ -1236,7 +1238,9 @@ void AwesomePlayer::initRenderer_l() {
         // allocate their buffers in local address space.  This renderer
         // then performs a color conversion and copy to get the data
         // into the ANativeBuffer.
-        mVideoRenderer = new AwesomeLocalRenderer(mNativeWindow, meta);
+        sp<AMessage> format;
+        convertMetaDataToMessage(meta, &format);
+        mVideoRenderer = new AwesomeLocalRenderer(mNativeWindow, format);
     }
 }
 

diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp
index 42691b9..7bb7ed9 100644 (file)
--- a/media/libstagefright/MediaCodec.cpp
+++ b/media/libstagefright/MediaCodec.cpp
@@ -958,36 +958,14 @@ void MediaCodec::onMessageReceived(const sp<AMessage> &msg) {
                 {
                     ALOGV("codec output format changed");
 
-                    if ((mFlags & kFlagIsSoftwareCodec)
-                            && mNativeWindow != NULL) {
+                    if (mSoftRenderer == NULL &&
+                            mNativeWindow != NULL &&
+                            (mFlags & kFlagIsSoftwareCodec)) {
                         AString mime;
                         CHECK(msg->findString("mime", &mime));
 
-                        if (!strncasecmp("video/", mime.c_str(), 6)) {
-                            delete mSoftRenderer;
-                            mSoftRenderer = NULL;
-
-                            int32_t width, height;
-                            CHECK(msg->findInt32("width", &width));
-                            CHECK(msg->findInt32("height", &height));
-
-                            int32_t cropLeft, cropTop, cropRight, cropBottom;
-                            CHECK(msg->findRect("crop",
-                                &cropLeft, &cropTop, &cropRight, &cropBottom));
-
-                            int32_t colorFormat;
-                            CHECK(msg->findInt32(
-                                        "color-format", &colorFormat));
-
-                            sp<MetaData> meta = new MetaData;
-                            meta->setInt32(kKeyWidth, width);
-                            meta->setInt32(kKeyHeight, height);
-                            meta->setRect(kKeyCropRect,
-                                cropLeft, cropTop, cropRight, cropBottom);
-                            meta->setInt32(kKeyColorFormat, colorFormat);
-
-                            mSoftRenderer =
-                                new SoftwareRenderer(mNativeWindow, meta);
+                        if (mime.startsWithIgnoreCase("video/")) {
+                            mSoftRenderer = new SoftwareRenderer(mNativeWindow);
                         }
                     }
 
@@ -1799,6 +1777,8 @@ size_t MediaCodec::updateBuffers(
             CHECK(info->mNotify == NULL);
             CHECK(msg->findMessage("reply", &info->mNotify));
 
+            info->mFormat =
+                (portIndex == kPortIndexInput) ? mInputFormat : mOutputFormat;
             mAvailPortBuffers[portIndex].push_back(i);
 
             return i;
@@ -1978,7 +1958,8 @@ status_t MediaCodec::onReleaseOutputBuffer(const sp<AMessage> &msg) {
 
         if (mSoftRenderer != NULL) {
             mSoftRenderer->render(
-                    info->mData->data(), info->mData->size(), timestampNs, NULL);
+                    info->mData->data(), info->mData->size(),
+                    timestampNs, NULL, info->mFormat);
         }
     }
 
@@ -2004,7 +1985,6 @@ ssize_t MediaCodec::dequeuePortBuffer(int32_t portIndex) {
     CHECK(!info->mOwnedByClient);
     {
         Mutex::Autolock al(mBufferLock);
-        info->mFormat = portIndex == kPortIndexInput ? mInputFormat : mOutputFormat;
         info->mOwnedByClient = true;
 
         // set image-data

diff --git a/media/libstagefright/Utils.cpp b/media/libstagefright/Utils.cpp
index 5f1d1c6..25afc5b 100644 (file)
--- a/media/libstagefright/Utils.cpp
+++ b/media/libstagefright/Utils.cpp
@@ -109,6 +109,25 @@ status_t convertMetaDataToMessage(
             msg->setInt32("sar-width", sarWidth);
             msg->setInt32("sar-height", sarHeight);
         }
+
+        int32_t colorFormat;
+        if (meta->findInt32(kKeyColorFormat, &colorFormat)) {
+            msg->setInt32("color-format", colorFormat);
+        }
+
+        int32_t cropLeft, cropTop, cropRight, cropBottom;
+        if (meta->findRect(kKeyCropRect,
+                           &cropLeft,
+                           &cropTop,
+                           &cropRight,
+                           &cropBottom)) {
+            msg->setRect("crop", cropLeft, cropTop, cropRight, cropBottom);
+        }
+
+        int32_t rotationDegrees;
+        if (meta->findInt32(kKeyRotation, &rotationDegrees)) {
+            msg->setInt32("rotation-degrees", rotationDegrees);
+        }
     } else if (!strncasecmp("audio/", mime, 6)) {
         int32_t numChannels, sampleRate;
         CHECK(meta->findInt32(kKeyChannelCount, &numChannels));
@@ -475,6 +494,25 @@ void convertMessageToMetaData(const sp<AMessage> &msg, sp<MetaData> &meta) {
             meta->setInt32(kKeySARWidth, sarWidth);
             meta->setInt32(kKeySARHeight, sarHeight);
         }
+
+        int32_t colorFormat;
+        if (msg->findInt32("color-format", &colorFormat)) {
+            meta->setInt32(kKeyColorFormat, colorFormat);
+        }
+
+        int32_t cropLeft, cropTop, cropRight, cropBottom;
+        if (msg->findRect("crop",
+                          &cropLeft,
+                          &cropTop,
+                          &cropRight,
+                          &cropBottom)) {
+            meta->setRect(kKeyCropRect, cropLeft, cropTop, cropRight, cropBottom);
+        }
+
+        int32_t rotationDegrees;
+        if (msg->findInt32("rotation-degrees", &rotationDegrees)) {
+            meta->setInt32(kKeyRotation, rotationDegrees);
+        }
     } else if (mime.startsWith("audio/")) {
         int32_t numChannels;
         if (msg->findInt32("channel-count", &numChannels)) {

diff --git a/media/libstagefright/colorconversion/SoftwareRenderer.cpp b/media/libstagefright/colorconversion/SoftwareRenderer.cpp
index 0c5527a..cc98da0 100644 (file)
--- a/media/libstagefright/colorconversion/SoftwareRenderer.cpp
+++ b/media/libstagefright/colorconversion/SoftwareRenderer.cpp
@@ -21,7 +21,7 @@
 
 #include <cutils/properties.h> // for property_get
 #include <media/stagefright/foundation/ADebug.h>
-#include <media/stagefright/MetaData.h>
+#include <media/stagefright/foundation/AMessage.h>
 #include <system/window.h>
 #include <ui/GraphicBufferMapper.h>
 #include <gui/IGraphicBufferProducer.h>
@@ -33,34 +33,71 @@ static bool runningInEmulator() {
     return (property_get("ro.kernel.qemu", prop, NULL) > 0);
 }
 
-SoftwareRenderer::SoftwareRenderer(
-        const sp<ANativeWindow> &nativeWindow, const sp<MetaData> &meta)
-    : mConverter(NULL),
+static int ALIGN(int x, int y) {
+    // y must be a power of 2.
+    return (x + y - 1) & ~(y - 1);
+}
+
+SoftwareRenderer::SoftwareRenderer(const sp<ANativeWindow> &nativeWindow)
+    : mColorFormat(OMX_COLOR_FormatUnused),
+      mConverter(NULL),
       mYUVMode(None),
-      mNativeWindow(nativeWindow) {
-    int32_t tmp;
-    CHECK(meta->findInt32(kKeyColorFormat, &tmp));
-    mColorFormat = (OMX_COLOR_FORMATTYPE)tmp;
-
-    CHECK(meta->findInt32(kKeyWidth, &mWidth));
-    CHECK(meta->findInt32(kKeyHeight, &mHeight));
-
-    if (!meta->findRect(
-                kKeyCropRect,
-                &mCropLeft, &mCropTop, &mCropRight, &mCropBottom)) {
-        mCropLeft = mCropTop = 0;
-        mCropRight = mWidth - 1;
-        mCropBottom = mHeight - 1;
+      mNativeWindow(nativeWindow),
+      mWidth(0),
+      mHeight(0),
+      mCropLeft(0),
+      mCropTop(0),
+      mCropRight(0),
+      mCropBottom(0),
+      mCropWidth(0),
+      mCropHeight(0) {
+}
+
+SoftwareRenderer::~SoftwareRenderer() {
+    delete mConverter;
+    mConverter = NULL;
+}
+
+void SoftwareRenderer::resetFormatIfChanged(const sp<AMessage> &format) {
+    CHECK(format != NULL);
+
+    int32_t colorFormatNew;
+    CHECK(format->findInt32("color-format", &colorFormatNew));
+
+    int32_t widthNew, heightNew;
+    CHECK(format->findInt32("width", &widthNew));
+    CHECK(format->findInt32("height", &heightNew));
+
+    int32_t cropLeftNew, cropTopNew, cropRightNew, cropBottomNew;
+    if (!format->findRect(
+            "crop", &cropLeftNew, &cropTopNew, &cropRightNew, &cropBottomNew)) {
+        cropLeftNew = cropTopNew = 0;
+        cropRightNew = widthNew - 1;
+        cropBottomNew = heightNew - 1;
     }
 
+    if (static_cast<int32_t>(mColorFormat) == colorFormatNew &&
+        mWidth == widthNew &&
+        mHeight == heightNew &&
+        mCropLeft == cropLeftNew &&
+        mCropTop == cropTopNew &&
+        mCropRight == cropRightNew &&
+        mCropBottom == cropBottomNew) {
+        // Nothing changed, no need to reset renderer.
+        return;
+    }
+
+    mColorFormat = static_cast<OMX_COLOR_FORMATTYPE>(colorFormatNew);
+    mWidth = widthNew;
+    mHeight = heightNew;
+    mCropLeft = cropLeftNew;
+    mCropTop = cropTopNew;
+    mCropRight = cropRightNew;
+    mCropBottom = cropBottomNew;
+
     mCropWidth = mCropRight - mCropLeft + 1;
     mCropHeight = mCropBottom - mCropTop + 1;
 
-    int32_t rotationDegrees;
-    if (!meta->findInt32(kKeyRotation, &rotationDegrees)) {
-        rotationDegrees = 0;
-    }
-
     int halFormat;
     size_t bufWidth, bufHeight;
 
@@ -106,10 +143,12 @@ SoftwareRenderer::SoftwareRenderer(
             NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW));
 
     // Width must be multiple of 32???
-    CHECK_EQ(0, native_window_set_buffers_geometry(
+    CHECK_EQ(0, native_window_set_buffers_dimensions(
                 mNativeWindow.get(),
                 bufWidth,
-                bufHeight,
+                bufHeight));
+    CHECK_EQ(0, native_window_set_buffers_format(
+                mNativeWindow.get(),
                 halFormat));
 
     // NOTE: native window uses extended right-bottom coordinate
@@ -123,6 +162,10 @@ SoftwareRenderer::SoftwareRenderer(
 
     CHECK_EQ(0, native_window_set_crop(mNativeWindow.get(), &crop));
 
+    int32_t rotationDegrees;
+    if (!format->findInt32("rotation-degrees", &rotationDegrees)) {
+        rotationDegrees = 0;
+    }
     uint32_t transform;
     switch (rotationDegrees) {
         case 0: transform = 0; break;
@@ -132,24 +175,15 @@ SoftwareRenderer::SoftwareRenderer(
         default: transform = 0; break;
     }
 
-    if (transform) {
-        CHECK_EQ(0, native_window_set_buffers_transform(
-                    mNativeWindow.get(), transform));
-    }
-}
-
-SoftwareRenderer::~SoftwareRenderer() {
-    delete mConverter;
-    mConverter = NULL;
-}
-
-static int ALIGN(int x, int y) {
-    // y must be a power of 2.
-    return (x + y - 1) & ~(y - 1);
+    CHECK_EQ(0, native_window_set_buffers_transform(
+                mNativeWindow.get(), transform));
 }
 
 void SoftwareRenderer::render(
-        const void *data, size_t size, int64_t timestampNs, void *platformPrivate) {
+        const void *data, size_t /*size*/, int64_t timestampNs,
+        void* /*platformPrivate*/, const sp<AMessage>& format) {
+    resetFormatIfChanged(format);
+
     ANativeWindowBuffer *buf;
     int err;
     if ((err = native_window_dequeue_buffer_and_wait(mNativeWindow.get(),

diff --git a/media/libstagefright/include/SoftwareRenderer.h b/media/libstagefright/include/SoftwareRenderer.h
index 0ba670c..fa3ea89 100644 (file)
--- a/media/libstagefright/include/SoftwareRenderer.h
+++ b/media/libstagefright/include/SoftwareRenderer.h
@@ -24,17 +24,17 @@
 
 namespace android {
 
-struct MetaData;
+struct AMessage;
 
 class SoftwareRenderer {
 public:
-    SoftwareRenderer(
-            const sp<ANativeWindow> &nativeWindow, const sp<MetaData> &meta);
+    explicit SoftwareRenderer(const sp<ANativeWindow> &nativeWindow);
 
     ~SoftwareRenderer();
 
     void render(
-            const void *data, size_t size, int64_t timestampNs, void *platformPrivate);
+            const void *data, size_t size, int64_t timestampNs,
+            void *platformPrivate, const sp<AMessage> &format);
 
 private:
     enum YUVMode {
@@ -51,6 +51,8 @@ private:
 
     SoftwareRenderer(const SoftwareRenderer &);
     SoftwareRenderer &operator=(const SoftwareRenderer &);
+
+    void resetFormatIfChanged(const sp<AMessage> &format);
 };
 
 }  // namespace android