Offer a type-safe album art interface.
Bug: 15514223
Change-Id: Iddfc33a00e6cd3779ca09c01a55f62b151f6ec95
diff --git a/include/media/MediaMetadataRetrieverInterface.h b/include/media/MediaMetadataRetrieverInterface.h
index ecc3b65..cd5bf88 100644
--- a/include/media/MediaMetadataRetrieverInterface.h
+++ b/include/media/MediaMetadataRetrieverInterface.h
@@ -20,6 +20,7 @@
#include <utils/RefBase.h>
#include <media/mediametadataretriever.h>
+#include <media/mediascanner.h>
#include <private/media/VideoFrame.h>
namespace android {
diff --git a/include/media/mediascanner.h b/include/media/mediascanner.h
index a73403b..a7bb6c1 100644
--- a/include/media/mediascanner.h
+++ b/include/media/mediascanner.h
@@ -41,6 +41,31 @@
MEDIA_SCAN_RESULT_ERROR,
};
+struct MediaAlbumArt {
+public:
+ static MediaAlbumArt *fromData(int32_t size, const void* data);
+
+ static void init(MediaAlbumArt* instance, int32_t size, const void* data);
+
+ MediaAlbumArt *clone();
+
+ const char *data() {
+ return &mData[0];
+ }
+
+ int32_t size() {
+ return mSize;
+ }
+
+private:
+ int32_t mSize;
+ char mData[0];
+
+ // You can't construct instances of this class directly because this is a
+ // variable-sized object passed through the binder.
+ MediaAlbumArt();
+} __packed;
+
struct MediaScanner {
MediaScanner();
virtual ~MediaScanner();
@@ -53,8 +78,7 @@
void setLocale(const char *locale);
- // extracts album art as a block of data
- virtual char *extractAlbumArt(int fd) = 0;
+ virtual MediaAlbumArt *extractAlbumArt(int fd) = 0;
protected:
const char *locale() const;
diff --git a/include/media/stagefright/StagefrightMediaScanner.h b/include/media/stagefright/StagefrightMediaScanner.h
index 6510a59..eb3accc 100644
--- a/include/media/stagefright/StagefrightMediaScanner.h
+++ b/include/media/stagefright/StagefrightMediaScanner.h
@@ -30,7 +30,7 @@
const char *path, const char *mimeType,
MediaScannerClient &client);
- virtual char *extractAlbumArt(int fd);
+ virtual MediaAlbumArt *extractAlbumArt(int fd);
private:
StagefrightMediaScanner(const StagefrightMediaScanner &);
diff --git a/include/private/media/VideoFrame.h b/include/private/media/VideoFrame.h
index a211ed9..5dd425b 100644
--- a/include/private/media/VideoFrame.h
+++ b/include/private/media/VideoFrame.h
@@ -25,64 +25,6 @@
namespace android {
-// A simple buffer to hold binary data
-class MediaAlbumArt
-{
-public:
- MediaAlbumArt(): mSize(0), mData(0) {}
-
- explicit MediaAlbumArt(const char* url) {
- mSize = 0;
- mData = NULL;
- FILE *in = fopen(url, "r");
- if (!in) {
- return;
- }
- fseek(in, 0, SEEK_END);
- mSize = ftell(in); // Allocating buffer of size equals to the external file size.
- if (mSize == 0 || (mData = new uint8_t[mSize]) == NULL) {
- fclose(in);
- if (mSize != 0) {
- mSize = 0;
- }
- return;
- }
- rewind(in);
- if (fread(mData, 1, mSize, in) != mSize) { // Read failed.
- delete[] mData;
- mData = NULL;
- mSize = 0;
- return;
- }
- fclose(in);
- }
-
- MediaAlbumArt(const MediaAlbumArt& copy) {
- mSize = copy.mSize;
- mData = NULL; // initialize it first
- if (mSize > 0 && copy.mData != NULL) {
- mData = new uint8_t[copy.mSize];
- if (mData != NULL) {
- memcpy(mData, copy.mData, mSize);
- } else {
- mSize = 0;
- }
- }
- }
-
- ~MediaAlbumArt() {
- if (mData != 0) {
- delete[] mData;
- }
- }
-
- // Intentional public access modifier:
- // We have to know the internal structure in order to share it between
- // processes?
- uint32_t mSize; // Number of bytes in mData
- uint8_t* mData; // Actual binary data
-};
-
// Represents a color converted (RGB-based) video frame
// with bitmap pixels stored in FrameBuffer
class VideoFrame
diff --git a/media/libmedia/MediaScanner.cpp b/media/libmedia/MediaScanner.cpp
index 28b5aa7..dcbb769 100644
--- a/media/libmedia/MediaScanner.cpp
+++ b/media/libmedia/MediaScanner.cpp
@@ -237,4 +237,24 @@
return MEDIA_SCAN_RESULT_OK;
}
+MediaAlbumArt *MediaAlbumArt::clone() {
+ size_t byte_size = this->size() + sizeof(MediaAlbumArt);
+ MediaAlbumArt *result = reinterpret_cast<MediaAlbumArt *>(malloc(byte_size));
+ result->mSize = this->size();
+ memcpy(&result->mData[0], &this->mData[0], this->size());
+ return result;
+}
+
+void MediaAlbumArt::init(MediaAlbumArt *instance, int32_t dataSize, const void *data) {
+ instance->mSize = dataSize;
+ memcpy(&instance->mData[0], data, dataSize);
+}
+
+MediaAlbumArt *MediaAlbumArt::fromData(int32_t dataSize, const void* data) {
+ size_t byte_size = sizeof(MediaAlbumArt) + dataSize;
+ MediaAlbumArt *result = reinterpret_cast<MediaAlbumArt *>(malloc(byte_size));
+ init(result, dataSize, data);
+ return result;
+}
+
} // namespace android
diff --git a/media/libmediaplayerservice/MetadataRetrieverClient.cpp b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
index 348957f..5c9bd8f 100644
--- a/media/libmediaplayerservice/MetadataRetrieverClient.cpp
+++ b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
@@ -230,7 +230,7 @@
ALOGE("failed to extract an album art");
return NULL;
}
- size_t size = sizeof(MediaAlbumArt) + albumArt->mSize;
+ size_t size = sizeof(MediaAlbumArt) + albumArt->size();
sp<MemoryHeapBase> heap = new MemoryHeapBase(size, 0, "MetadataRetrieverClient");
if (heap == NULL) {
ALOGE("failed to create MemoryDealer object");
@@ -243,11 +243,9 @@
delete albumArt;
return NULL;
}
- MediaAlbumArt *albumArtCopy = static_cast<MediaAlbumArt *>(mAlbumArt->pointer());
- albumArtCopy->mSize = albumArt->mSize;
- albumArtCopy->mData = (uint8_t *)albumArtCopy + sizeof(MediaAlbumArt);
- memcpy(albumArtCopy->mData, albumArt->mData, albumArt->mSize);
- delete albumArt; // Fix memory leakage
+ MediaAlbumArt::init((MediaAlbumArt *) mAlbumArt->pointer(),
+ albumArt->size(), albumArt->data());
+ delete albumArt; // We've taken our copy.
return mAlbumArt;
}
diff --git a/media/libstagefright/StagefrightMediaScanner.cpp b/media/libstagefright/StagefrightMediaScanner.cpp
index 2b51a29..f3fc3f6 100644
--- a/media/libstagefright/StagefrightMediaScanner.cpp
+++ b/media/libstagefright/StagefrightMediaScanner.cpp
@@ -202,7 +202,7 @@
return MEDIA_SCAN_RESULT_OK;
}
-char *StagefrightMediaScanner::extractAlbumArt(int fd) {
+MediaAlbumArt *StagefrightMediaScanner::extractAlbumArt(int fd) {
ALOGV("extractAlbumArt %d", fd);
off64_t size = lseek64(fd, 0, SEEK_END);
@@ -214,15 +214,9 @@
sp<MediaMetadataRetriever> mRetriever(new MediaMetadataRetriever);
if (mRetriever->setDataSource(fd, 0, size) == OK) {
sp<IMemory> mem = mRetriever->extractAlbumArt();
-
if (mem != NULL) {
MediaAlbumArt *art = static_cast<MediaAlbumArt *>(mem->pointer());
-
- char *data = (char *)malloc(art->mSize + 4);
- *(int32_t *)data = art->mSize;
- memcpy(&data[4], &art[1], art->mSize);
-
- return data;
+ return art->clone();
}
}
diff --git a/media/libstagefright/StagefrightMetadataRetriever.cpp b/media/libstagefright/StagefrightMetadataRetriever.cpp
index fcd9a85..e4d3c79 100644
--- a/media/libstagefright/StagefrightMetadataRetriever.cpp
+++ b/media/libstagefright/StagefrightMetadataRetriever.cpp
@@ -375,10 +375,7 @@
size_t dataSize;
if (fileMeta->findData(kKeyAlbumArt, &type, &data, &dataSize)
&& mAlbumArt == NULL) {
- mAlbumArt = new MediaAlbumArt;
- mAlbumArt->mSize = dataSize;
- mAlbumArt->mData = new uint8_t[dataSize];
- memcpy(mAlbumArt->mData, data, dataSize);
+ mAlbumArt = MediaAlbumArt::fromData(dataSize, data);
}
VideoFrame *frame =
@@ -411,7 +408,7 @@
}
if (mAlbumArt) {
- return new MediaAlbumArt(*mAlbumArt);
+ return mAlbumArt->clone();
}
return NULL;
@@ -480,10 +477,7 @@
size_t dataSize;
if (meta->findData(kKeyAlbumArt, &type, &data, &dataSize)
&& mAlbumArt == NULL) {
- mAlbumArt = new MediaAlbumArt;
- mAlbumArt->mSize = dataSize;
- mAlbumArt->mData = new uint8_t[dataSize];
- memcpy(mAlbumArt->mData, data, dataSize);
+ mAlbumArt = MediaAlbumArt::fromData(dataSize, data);
}
size_t numTracks = mExtractor->countTracks();