Remove one redundant frame copy in MetadataRetriever
When extracting bitmaps, MediaMetadataRetriever does a copy from
StagefrightMetadataRetriever's VideoFrame to IMemory. We could
just allocate IMemory directly and return that to the client.
bug: 78475896
Change-Id: Ibe07e7d2c68f031261470308476089c2fa9298ea
diff --git a/cmds/stagefright/stagefright.cpp b/cmds/stagefright/stagefright.cpp
index 936733d..61fc897 100644
--- a/cmds/stagefright/stagefright.cpp
+++ b/cmds/stagefright/stagefright.cpp
@@ -893,7 +893,7 @@
VideoFrame *frame = (VideoFrame *)mem->pointer();
CHECK_EQ(writeJpegFile("/sdcard/out.jpg",
- (uint8_t *)frame + sizeof(VideoFrame),
+ frame->getFlattenedData(),
frame->mWidth, frame->mHeight), 0);
}
diff --git a/include/private/media/VideoFrame.h b/include/private/media/VideoFrame.h
index a9d4dd1..8b8824f 100644
--- a/include/private/media/VideoFrame.h
+++ b/include/private/media/VideoFrame.h
@@ -25,94 +25,32 @@
namespace android {
-// Represents a color converted (RGB-based) video frame
-// with bitmap pixels stored in FrameBuffer
+// Represents a color converted (RGB-based) video frame with bitmap
+// pixels stored in FrameBuffer.
+// In a VideoFrame struct stored in IMemory, frame data and ICC data
+// come after the VideoFrame structure. Their locations can be retrieved
+// by getFlattenedData() and getFlattenedIccData();
class VideoFrame
{
public:
// Construct a VideoFrame object with the specified parameters,
- // will allocate frame buffer if |allocate| is set to true, will
- // allocate buffer to hold ICC data if |iccData| and |iccSize|
- // indicate its presence.
+ // will calculate frame buffer size if |hasData| is set to true.
VideoFrame(uint32_t width, uint32_t height,
uint32_t displayWidth, uint32_t displayHeight,
- uint32_t angle, uint32_t bpp, bool allocate,
- const void *iccData, size_t iccSize):
+ uint32_t angle, uint32_t bpp, bool hasData, size_t iccSize):
mWidth(width), mHeight(height),
mDisplayWidth(displayWidth), mDisplayHeight(displayHeight),
mRotationAngle(angle), mBytesPerPixel(bpp), mRowBytes(bpp * width),
- mSize(0), mIccSize(0), mReserved(0), mData(0), mIccData(0) {
- if (allocate) {
- mSize = mRowBytes * mHeight;
- mData = new uint8_t[mSize];
- if (mData == NULL) {
- mSize = 0;
- }
- }
-
- if (iccData != NULL && iccSize > 0) {
- mIccSize = iccSize;
- mIccData = new uint8_t[iccSize];
- if (mIccData != NULL) {
- memcpy(mIccData, iccData, iccSize);
- } else {
- mIccSize = 0;
- }
- }
+ mSize(hasData ? (bpp * width * height) : 0),
+ mIccSize(iccSize), mReserved(0) {
}
- // Deep copy of both the information fields and the frame data
- VideoFrame(const VideoFrame& copy) {
- copyInfoOnly(copy);
-
- mSize = copy.mSize;
- mData = NULL; // initialize it first
- if (mSize > 0 && copy.mData != NULL) {
- mData = new uint8_t[mSize];
- if (mData != NULL) {
- memcpy(mData, copy.mData, mSize);
- } else {
- mSize = 0;
- }
- }
-
- mIccSize = copy.mIccSize;
- mIccData = NULL; // initialize it first
- if (mIccSize > 0 && copy.mIccData != NULL) {
- mIccData = new uint8_t[mIccSize];
- if (mIccData != NULL) {
- memcpy(mIccData, copy.mIccData, mIccSize);
- } else {
- mIccSize = 0;
- }
- }
- }
-
- ~VideoFrame() {
- if (mData != 0) {
- delete[] mData;
- }
- if (mIccData != 0) {
- delete[] mIccData;
- }
- }
-
- // Copy |copy| to a flattened VideoFrame in IMemory, 'this' must point to
- // a chunk of memory back by IMemory of size at least getFlattenedSize()
- // of |copy|.
- void copyFlattened(const VideoFrame& copy) {
- copyInfoOnly(copy);
-
- mSize = copy.mSize;
- mData = NULL; // initialize it first
- if (copy.mSize > 0 && copy.mData != NULL) {
- memcpy(getFlattenedData(), copy.mData, copy.mSize);
- }
-
- mIccSize = copy.mIccSize;
- mIccData = NULL; // initialize it first
- if (copy.mIccSize > 0 && copy.mIccData != NULL) {
- memcpy(getFlattenedIccData(), copy.mIccData, copy.mIccSize);
+ void init(const VideoFrame& copy, const void* iccData, size_t iccSize) {
+ *this = copy;
+ if (mIccSize == iccSize && iccSize > 0 && iccData != NULL) {
+ memcpy(getFlattenedIccData(), iccData, iccSize);
+ } else {
+ mIccSize = 0;
}
}
@@ -139,35 +77,9 @@
int32_t mRotationAngle; // Rotation angle, clockwise, should be multiple of 90
uint32_t mBytesPerPixel; // Number of bytes per pixel
uint32_t mRowBytes; // Number of bytes per row before rotation
- uint32_t mSize; // Number of bytes in mData
- uint32_t mIccSize; // Number of bytes in mIccData
+ uint32_t mSize; // Number of bytes of frame data
+ uint32_t mIccSize; // Number of bytes of ICC data
uint32_t mReserved; // (padding to make mData 64-bit aligned)
-
- // mData should be 64-bit aligned to prevent additional padding
- uint8_t* mData; // Actual binary data
- // pad structure so it's the same size on 64-bit and 32-bit
- char mPadding[8 - sizeof(mData)];
-
- // mIccData should be 64-bit aligned to prevent additional padding
- uint8_t* mIccData; // Actual binary data
- // pad structure so it's the same size on 64-bit and 32-bit
- char mIccPadding[8 - sizeof(mIccData)];
-
-private:
- //
- // Utility methods used only within VideoFrame struct
- //
-
- // Copy the information fields only
- void copyInfoOnly(const VideoFrame& copy) {
- mWidth = copy.mWidth;
- mHeight = copy.mHeight;
- mDisplayWidth = copy.mDisplayWidth;
- mDisplayHeight = copy.mDisplayHeight;
- mRotationAngle = copy.mRotationAngle;
- mBytesPerPixel = copy.mBytesPerPixel;
- mRowBytes = copy.mRowBytes;
- }
};
}; // namespace android
diff --git a/media/libmedia/include/media/MediaMetadataRetrieverInterface.h b/media/libmedia/include/media/MediaMetadataRetrieverInterface.h
index c45a964..992e230 100644
--- a/media/libmedia/include/media/MediaMetadataRetrieverInterface.h
+++ b/media/libmedia/include/media/MediaMetadataRetrieverInterface.h
@@ -43,12 +43,12 @@
virtual status_t setDataSource(int fd, int64_t offset, int64_t length) = 0;
virtual status_t setDataSource(const sp<DataSource>& source, const char *mime) = 0;
- virtual VideoFrame* getFrameAtTime(
+ virtual sp<IMemory> getFrameAtTime(
int64_t timeUs, int option, int colorFormat, bool metaOnly) = 0;
- virtual VideoFrame* getImageAtIndex(
+ virtual sp<IMemory> getImageAtIndex(
int index, int colorFormat, bool metaOnly, bool thumbnail) = 0;
virtual status_t getFrameAtIndex(
- std::vector<VideoFrame*>* frames,
+ std::vector<sp<IMemory> >* frames,
int frameIndex, int numFrames, int colorFormat, bool metaOnly) = 0;
virtual MediaAlbumArt* extractAlbumArt() = 0;
virtual const char* extractMetadata(int keyCode) = 0;
@@ -61,14 +61,14 @@
MediaMetadataRetrieverInterface() {}
virtual ~MediaMetadataRetrieverInterface() {}
- virtual VideoFrame* getFrameAtTime(
+ virtual sp<IMemory> getFrameAtTime(
int64_t /*timeUs*/, int /*option*/, int /*colorFormat*/, bool /*metaOnly*/)
{ return NULL; }
- virtual VideoFrame* getImageAtIndex(
+ virtual sp<IMemory> getImageAtIndex(
int /*index*/, int /*colorFormat*/, bool /*metaOnly*/, bool /*thumbnail*/)
{ return NULL; }
virtual status_t getFrameAtIndex(
- std::vector<VideoFrame*>* /*frames*/,
+ std::vector<sp<IMemory> >* /*frames*/,
int /*frameIndex*/, int /*numFrames*/, int /*colorFormat*/, bool /*metaOnly*/)
{ return ERROR_UNSUPPORTED; }
virtual MediaAlbumArt* extractAlbumArt() { return NULL; }
diff --git a/media/libmediaplayerservice/MetadataRetrieverClient.cpp b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
index 3b3ac29..672b832 100644
--- a/media/libmediaplayerservice/MetadataRetrieverClient.cpp
+++ b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
@@ -194,25 +194,6 @@
Mutex MetadataRetrieverClient::sLock;
-static sp<IMemory> getThumbnail(VideoFrame* frame) {
- std::unique_ptr<VideoFrame> frameDeleter(frame);
-
- size_t size = frame->getFlattenedSize();
- sp<MemoryHeapBase> heap = new MemoryHeapBase(size, 0, "MetadataRetrieverClient");
- if (heap == NULL) {
- ALOGE("failed to create MemoryDealer");
- return NULL;
- }
- sp<IMemory> thrumbnail = new MemoryBase(heap, 0, size);
- if (thrumbnail == NULL) {
- ALOGE("not enough memory for VideoFrame size=%zu", size);
- return NULL;
- }
- VideoFrame *frameCopy = static_cast<VideoFrame *>(thrumbnail->pointer());
- frameCopy->copyFlattened(*frame);
- return thrumbnail;
-}
-
sp<IMemory> MetadataRetrieverClient::getFrameAtTime(
int64_t timeUs, int option, int colorFormat, bool metaOnly)
{
@@ -225,12 +206,12 @@
ALOGE("retriever is not initialized");
return NULL;
}
- VideoFrame *frame = mRetriever->getFrameAtTime(timeUs, option, colorFormat, metaOnly);
+ sp<IMemory> frame = mRetriever->getFrameAtTime(timeUs, option, colorFormat, metaOnly);
if (frame == NULL) {
ALOGE("failed to capture a video frame");
return NULL;
}
- return getThumbnail(frame);
+ return frame;
}
sp<IMemory> MetadataRetrieverClient::getImageAtIndex(
@@ -244,12 +225,12 @@
ALOGE("retriever is not initialized");
return NULL;
}
- VideoFrame *frame = mRetriever->getImageAtIndex(index, colorFormat, metaOnly, thumbnail);
+ sp<IMemory> frame = mRetriever->getImageAtIndex(index, colorFormat, metaOnly, thumbnail);
if (frame == NULL) {
ALOGE("failed to extract image");
return NULL;
}
- return getThumbnail(frame);
+ return frame;
}
status_t MetadataRetrieverClient::getFrameAtIndex(
@@ -264,15 +245,12 @@
return INVALID_OPERATION;
}
- std::vector<VideoFrame*> videoFrames;
status_t err = mRetriever->getFrameAtIndex(
- &videoFrames, frameIndex, numFrames, colorFormat, metaOnly);
+ frames, frameIndex, numFrames, colorFormat, metaOnly);
if (err != OK) {
+ frames->clear();
return err;
}
- for (size_t i = 0; i < videoFrames.size(); i++) {
- frames->push_back(getThumbnail(videoFrames[i]));
- }
return OK;
}
diff --git a/media/libstagefright/FrameDecoder.cpp b/media/libstagefright/FrameDecoder.cpp
index a00d13a..b17fbc9 100644
--- a/media/libstagefright/FrameDecoder.cpp
+++ b/media/libstagefright/FrameDecoder.cpp
@@ -17,12 +17,11 @@
//#define LOG_NDEBUG 0
#define LOG_TAG "FrameDecoder"
-#include <inttypes.h>
-
-#include <utils/Log.h>
-#include <gui/Surface.h>
-
#include "include/FrameDecoder.h"
+#include <binder/MemoryBase.h>
+#include <binder/MemoryHeapBase.h>
+#include <gui/Surface.h>
+#include <inttypes.h>
#include <media/ICrypto.h>
#include <media/IMediaSource.h>
#include <media/MediaCodecBuffer.h>
@@ -36,6 +35,7 @@
#include <media/stagefright/MediaErrors.h>
#include <media/stagefright/Utils.h>
#include <private/media/VideoFrame.h>
+#include <utils/Log.h>
namespace android {
@@ -43,7 +43,7 @@
static const size_t kRetryCount = 20; // must be >0
//static
-VideoFrame *allocVideoFrame(const sp<MetaData> &trackMeta,
+sp<IMemory> allocVideoFrame(const sp<MetaData>& trackMeta,
int32_t width, int32_t height, int32_t dstBpp, bool metaOnly = false) {
int32_t rotationAngle;
if (!trackMeta->findInt32(kKeyRotation, &rotationAngle)) {
@@ -74,8 +74,24 @@
displayHeight = height;
}
- return new VideoFrame(width, height, displayWidth, displayHeight,
- rotationAngle, dstBpp, !metaOnly, iccData, iccSize);
+ VideoFrame frame(width, height, displayWidth, displayHeight,
+ rotationAngle, dstBpp, !metaOnly, iccSize);
+
+ size_t size = frame.getFlattenedSize();
+ sp<MemoryHeapBase> heap = new MemoryHeapBase(size, 0, "MetadataRetrieverClient");
+ if (heap == NULL) {
+ ALOGE("failed to create MemoryDealer");
+ return NULL;
+ }
+ sp<IMemory> frameMem = new MemoryBase(heap, 0, size);
+ if (frameMem == NULL) {
+ ALOGE("not enough memory for VideoFrame size=%zu", size);
+ return NULL;
+ }
+ VideoFrame* frameCopy = static_cast<VideoFrame*>(frameMem->pointer());
+ frameCopy->init(frame, iccData, iccSize);
+
+ return frameMem;
}
//static
@@ -92,7 +108,7 @@
}
//static
-VideoFrame* FrameDecoder::getMetadataOnly(
+sp<IMemory> FrameDecoder::getMetadataOnly(
const sp<MetaData> &trackMeta, int colorFormat, bool thumbnail) {
OMX_COLOR_FORMATTYPE dstFormat;
int32_t dstBpp;
@@ -146,7 +162,7 @@
return false;
}
-VideoFrame* FrameDecoder::extractFrame(
+sp<IMemory> FrameDecoder::extractFrame(
int64_t frameTimeUs, int option, int colorFormat) {
if (!getDstColorFormat(
(android_pixel_format_t)colorFormat, &mDstFormat, &mDstBpp)) {
@@ -158,12 +174,12 @@
return NULL;
}
- return mFrames.size() > 0 ? mFrames[0].release() : NULL;
+ return mFrames.size() > 0 ? mFrames[0] : NULL;
}
status_t FrameDecoder::extractFrames(
int64_t frameTimeUs, size_t numFrames, int option, int colorFormat,
- std::vector<VideoFrame*>* frames) {
+ std::vector<sp<IMemory> >* frames) {
if (!getDstColorFormat(
(android_pixel_format_t)colorFormat, &mDstFormat, &mDstBpp)) {
return ERROR_UNSUPPORTED;
@@ -175,7 +191,7 @@
}
for (size_t i = 0; i < mFrames.size(); i++) {
- frames->push_back(mFrames[i].release());
+ frames->push_back(mFrames[i]);
}
return OK;
}
@@ -468,12 +484,13 @@
crop_bottom = height - 1;
}
- VideoFrame *frame = allocVideoFrame(
+ sp<IMemory> frameMem = allocVideoFrame(
trackMeta(),
(crop_right - crop_left + 1),
(crop_bottom - crop_top + 1),
dstBpp());
- addFrame(frame);
+ addFrame(frameMem);
+ VideoFrame* frame = static_cast<VideoFrame*>(frameMem->pointer());
int32_t srcFormat;
CHECK(outputFormat->findInt32("color-format", &srcFormat));
@@ -485,7 +502,7 @@
(const uint8_t *)videoFrameBuffer->data(),
width, height,
crop_left, crop_top, crop_right, crop_bottom,
- frame->mData,
+ frame->getFlattenedData(),
frame->mWidth,
frame->mHeight,
crop_left, crop_top, crop_right, crop_bottom);
@@ -596,9 +613,10 @@
}
if (mFrame == NULL) {
- mFrame = allocVideoFrame(trackMeta(), imageWidth, imageHeight, dstBpp());
+ sp<IMemory> frameMem = allocVideoFrame(trackMeta(), imageWidth, imageHeight, dstBpp());
+ mFrame = static_cast<VideoFrame*>(frameMem->pointer());
- addFrame(mFrame);
+ addFrame(frameMem);
}
int32_t srcFormat;
@@ -639,7 +657,7 @@
(const uint8_t *)videoFrameBuffer->data(),
width, height,
crop_left, crop_top, crop_right, crop_bottom,
- mFrame->mData,
+ mFrame->getFlattenedData(),
mFrame->mWidth,
mFrame->mHeight,
dstLeft, dstTop, dstRight, dstBottom);
diff --git a/media/libstagefright/StagefrightMetadataRetriever.cpp b/media/libstagefright/StagefrightMetadataRetriever.cpp
index 6ad6004..5417fef 100644
--- a/media/libstagefright/StagefrightMetadataRetriever.cpp
+++ b/media/libstagefright/StagefrightMetadataRetriever.cpp
@@ -124,7 +124,7 @@
return OK;
}
-VideoFrame* StagefrightMetadataRetriever::getImageAtIndex(
+sp<IMemory> StagefrightMetadataRetriever::getImageAtIndex(
int index, int colorFormat, bool metaOnly, bool thumbnail) {
ALOGV("getImageAtIndex: index(%d) colorFormat(%d) metaOnly(%d) thumbnail(%d)",
@@ -193,7 +193,7 @@
for (size_t i = 0; i < matchingCodecs.size(); ++i) {
const AString &componentName = matchingCodecs[i];
ImageDecoder decoder(componentName, trackMeta, source);
- VideoFrame* frame = decoder.extractFrame(
+ sp<IMemory> frame = decoder.extractFrame(
thumbnail ? -1 : 0 /*frameTimeUs*/, 0 /*seekMode*/, colorFormat);
if (frame != NULL) {
@@ -205,19 +205,19 @@
return NULL;
}
-VideoFrame* StagefrightMetadataRetriever::getFrameAtTime(
+sp<IMemory> StagefrightMetadataRetriever::getFrameAtTime(
int64_t timeUs, int option, int colorFormat, bool metaOnly) {
ALOGV("getFrameAtTime: %" PRId64 " us option: %d colorFormat: %d, metaOnly: %d",
timeUs, option, colorFormat, metaOnly);
- VideoFrame *frame;
+ sp<IMemory> frame;
status_t err = getFrameInternal(
timeUs, 1, option, colorFormat, metaOnly, &frame, NULL /*outFrames*/);
return (err == OK) ? frame : NULL;
}
status_t StagefrightMetadataRetriever::getFrameAtIndex(
- std::vector<VideoFrame*>* frames,
+ std::vector<sp<IMemory> >* frames,
int frameIndex, int numFrames, int colorFormat, bool metaOnly) {
ALOGV("getFrameAtIndex: frameIndex %d, numFrames %d, colorFormat: %d, metaOnly: %d",
frameIndex, numFrames, colorFormat, metaOnly);
@@ -229,7 +229,7 @@
status_t StagefrightMetadataRetriever::getFrameInternal(
int64_t timeUs, int numFrames, int option, int colorFormat, bool metaOnly,
- VideoFrame **outFrame, std::vector<VideoFrame*>* outFrames) {
+ sp<IMemory>* outFrame, std::vector<sp<IMemory> >* outFrames) {
if (mExtractor.get() == NULL) {
ALOGE("no extractor.");
return NO_INIT;
diff --git a/media/libstagefright/include/FrameDecoder.h b/media/libstagefright/include/FrameDecoder.h
index b67e928..f6d4727 100644
--- a/media/libstagefright/include/FrameDecoder.h
+++ b/media/libstagefright/include/FrameDecoder.h
@@ -44,16 +44,16 @@
mDstFormat(OMX_COLOR_Format16bitRGB565),
mDstBpp(2) {}
- VideoFrame* extractFrame(int64_t frameTimeUs, int option, int colorFormat);
+ sp<IMemory> extractFrame(int64_t frameTimeUs, int option, int colorFormat);
status_t extractFrames(
int64_t frameTimeUs,
size_t numFrames,
int option,
int colorFormat,
- std::vector<VideoFrame*>* frames);
+ std::vector<sp<IMemory> >* frames);
- static VideoFrame* getMetadataOnly(
+ static sp<IMemory> getMetadataOnly(
const sp<MetaData> &trackMeta, int colorFormat, bool thumbnail = false);
protected:
@@ -81,8 +81,8 @@
OMX_COLOR_FORMATTYPE dstFormat() const { return mDstFormat; }
int32_t dstBpp() const { return mDstBpp; }
- void addFrame(VideoFrame *frame) {
- mFrames.push_back(std::unique_ptr<VideoFrame>(frame));
+ void addFrame(const sp<IMemory> &frame) {
+ mFrames.push_back(frame);
}
private:
@@ -91,7 +91,7 @@
sp<IMediaSource> mSource;
OMX_COLOR_FORMATTYPE mDstFormat;
int32_t mDstBpp;
- std::vector<std::unique_ptr<VideoFrame> > mFrames;
+ std::vector<sp<IMemory> > mFrames;
static bool getDstColorFormat(
android_pixel_format_t colorFormat,
diff --git a/media/libstagefright/include/StagefrightMetadataRetriever.h b/media/libstagefright/include/StagefrightMetadataRetriever.h
index 8443fbe..209f850 100644
--- a/media/libstagefright/include/StagefrightMetadataRetriever.h
+++ b/media/libstagefright/include/StagefrightMetadataRetriever.h
@@ -40,12 +40,12 @@
virtual status_t setDataSource(int fd, int64_t offset, int64_t length);
virtual status_t setDataSource(const sp<DataSource>& source, const char *mime);
- virtual VideoFrame* getFrameAtTime(
+ virtual sp<IMemory> getFrameAtTime(
int64_t timeUs, int option, int colorFormat, bool metaOnly);
- virtual VideoFrame* getImageAtIndex(
+ virtual sp<IMemory> getImageAtIndex(
int index, int colorFormat, bool metaOnly, bool thumbnail);
virtual status_t getFrameAtIndex(
- std::vector<VideoFrame*>* frames,
+ std::vector<sp<IMemory> >* frames,
int frameIndex, int numFrames, int colorFormat, bool metaOnly);
virtual MediaAlbumArt *extractAlbumArt();
@@ -65,7 +65,7 @@
status_t getFrameInternal(
int64_t timeUs, int numFrames, int option, int colorFormat, bool metaOnly,
- VideoFrame **outFrame, std::vector<VideoFrame*>* outFrames);
+ sp<IMemory>* outFrame, std::vector<sp<IMemory> >* outFrames);
StagefrightMetadataRetriever(const StagefrightMetadataRetriever &);