MediaMuxer: add wrapper for MediaMuxer constructor
Added MediaMuxer::create(), that validates the input arguments.
All the calls to MediaMuxer's default constructor are now replaced
by MediaMuxer::create().
MediaMuxer's default constructor is still retained as public for the
time being and will be made private in a subsequent CL.
Bug: 146417874
Test: atest CtsMediaV2TestCases:MuxerTest
Test: atest CtsMediaV2TestCases:MuxerUnitTest
Test: CtsMediaMuxerTestCases
Change-Id: I166caa65708cd1ba2076bbbda4c425f534ba2887
diff --git a/cmds/stagefright/muxer.cpp b/cmds/stagefright/muxer.cpp
index bc7e41e..185491f 100644
--- a/cmds/stagefright/muxer.cpp
+++ b/cmds/stagefright/muxer.cpp
@@ -78,10 +78,14 @@
int fd = open(outputFileName, O_CREAT | O_LARGEFILE | O_TRUNC | O_RDWR, S_IRUSR | S_IWUSR);
if (fd < 0) {
- ALOGE("couldn't open file");
- return fd;
+ ALOGE("couldn't open output file %s", outputFileName);
+ return 1;
}
- sp<MediaMuxer> muxer = new MediaMuxer(fd, container);
+ sp<MediaMuxer> muxer = MediaMuxer::create(fd, container);
+ if (muxer == nullptr) {
+ fprintf(stderr, "unable to instantiate muxer for format %d\n", container);
+ return 1;
+ }
close(fd);
size_t trackCount = extractor->countTracks();
diff --git a/media/libstagefright/MediaAppender.cpp b/media/libstagefright/MediaAppender.cpp
index 21dcfa1..2d9c651 100644
--- a/media/libstagefright/MediaAppender.cpp
+++ b/media/libstagefright/MediaAppender.cpp
@@ -308,7 +308,11 @@
ALOGE("MediaAppender::start() is called in invalid state %d", mState);
return INVALID_OPERATION;
}
- mMuxer = new (std::nothrow) MediaMuxer(mFd, mFormat);
+ mMuxer = MediaMuxer::create(mFd, mFormat);
+ if (mMuxer == nullptr) {
+ ALOGE("MediaMuxer::create failed");
+ return INVALID_OPERATION;
+ }
for (const auto& n : mFmtIndexMap) {
ssize_t muxIndex = mMuxer->addTrack(n.second);
if (muxIndex < 0) {
diff --git a/media/libstagefright/MediaMuxer.cpp b/media/libstagefright/MediaMuxer.cpp
index a946f71..9f590e5 100644
--- a/media/libstagefright/MediaMuxer.cpp
+++ b/media/libstagefright/MediaMuxer.cpp
@@ -46,6 +46,30 @@
format == MediaMuxer::OUTPUT_FORMAT_HEIF;
}
+MediaMuxer* MediaMuxer::create(int fd, OutputFormat format) {
+ bool isInputValid = true;
+ if (isMp4Format(format)) {
+ isInputValid = MPEG4Writer::isFdOpenModeValid(fd);
+ } else if (format == OUTPUT_FORMAT_WEBM) {
+ isInputValid = WebmWriter::isFdOpenModeValid(fd);
+ } else if (format == OUTPUT_FORMAT_OGG) {
+ isInputValid = OggWriter::isFdOpenModeValid(fd);
+ } else {
+ ALOGE("MediaMuxer does not support output format %d", format);
+ return nullptr;
+ }
+ if (!isInputValid) {
+ ALOGE("File descriptor is not suitable for format %d", format);
+ return nullptr;
+ }
+
+ MediaMuxer *muxer = new (std::nothrow) MediaMuxer(fd, (MediaMuxer::OutputFormat)format);
+ if (muxer == nullptr) {
+ ALOGE("Failed to create writer object");
+ }
+ return muxer;
+}
+
MediaMuxer::MediaMuxer(int fd, OutputFormat format)
: mFormat(format),
mState(UNINITIALIZED) {
diff --git a/media/libstagefright/include/media/stagefright/MediaMuxer.h b/media/libstagefright/include/media/stagefright/MediaMuxer.h
index e97a65e..6dc70bf 100644
--- a/media/libstagefright/include/media/stagefright/MediaMuxer.h
+++ b/media/libstagefright/include/media/stagefright/MediaMuxer.h
@@ -48,6 +48,14 @@
// deleting the output file after stop.
struct MediaMuxer : public MediaMuxerBase {
public:
+ /**
+ * Creates the muxer for a given output format.
+ * @param fd : file descriptor of the output file.
+ * @param format : output format of the muxer. e.g.: webm/mp4/ogg
+ * @return writer's object or nullptr if error.
+ */
+ static MediaMuxer* create(int fd, OutputFormat format);
+
// Construct the muxer with the file descriptor. Note that the MediaMuxer
// will close this file at stop().
MediaMuxer(int fd, OutputFormat format);
diff --git a/media/libstagefright/include/media/stagefright/MediaWriter.h b/media/libstagefright/include/media/stagefright/MediaWriter.h
index 9f20185..2b14811 100644
--- a/media/libstagefright/include/media/stagefright/MediaWriter.h
+++ b/media/libstagefright/include/media/stagefright/MediaWriter.h
@@ -31,6 +31,29 @@
mMaxFileDurationLimitUs(0) {
}
+ // Returns true if the file descriptor is opened using a mode
+ // which meets minimum writer/muxer requirements.
+ static bool isFdOpenModeValid(int fd) {
+ // check for invalid file descriptor.
+ int flags = fcntl(fd, F_GETFL);
+ if (flags == -1) {
+ ALOGE("Invalid File Status Flags and/or mode : %d", flags);
+ return false;
+ }
+ // fd must be in read-write mode or write-only mode.
+ if ((flags & (O_RDWR | O_WRONLY)) == 0) {
+ ALOGE("File must be writable");
+ return false;
+ }
+ // Verify fd is seekable
+ off64_t off = lseek64(fd, 0, SEEK_SET);
+ if (off < 0) {
+ ALOGE("File descriptor is not seekable");
+ return false;
+ }
+ return true;
+ }
+
virtual status_t addSource(const sp<MediaSource> &source) = 0;
virtual bool reachedEOS() = 0;
virtual status_t start(MetaData *params = NULL) = 0;
diff --git a/media/libstagefright/tests/fuzzers/MediaMuxerFuzzer.cpp b/media/libstagefright/tests/fuzzers/MediaMuxerFuzzer.cpp
index 5df3267..70d73c8 100644
--- a/media/libstagefright/tests/fuzzers/MediaMuxerFuzzer.cpp
+++ b/media/libstagefright/tests/fuzzers/MediaMuxerFuzzer.cpp
@@ -52,7 +52,10 @@
MediaMuxer::OutputFormat format =
(MediaMuxer::OutputFormat)fdp.ConsumeIntegralInRange<int32_t>(0, 4);
- sp<MediaMuxer> mMuxer(new MediaMuxer(fd, format));
+ sp<MediaMuxer> mMuxer = MediaMuxer::create(fd, format);
+ if (mMuxer == nullptr) {
+ return 0;
+ }
while (fdp.remaining_bytes() > 1) {
switch (fdp.ConsumeIntegralInRange<uint8_t>(0, 4)) {
diff --git a/media/libstagefright/webm/WebmWriter.cpp b/media/libstagefright/webm/WebmWriter.cpp
index 5eaadbd..3823c36 100644
--- a/media/libstagefright/webm/WebmWriter.cpp
+++ b/media/libstagefright/webm/WebmWriter.cpp
@@ -54,6 +54,19 @@
static const int64_t kMinStreamableFileSizeInBytes = 5 * 1024 * 1024;
+bool WebmWriter::isFdOpenModeValid(int fd) {
+ // check for invalid file descriptor.
+ if (!MediaWriter::isFdOpenModeValid(fd)) {
+ return false;
+ }
+ int flags = fcntl(fd, F_GETFL);
+ if ((flags & O_RDWR) == 0) {
+ ALOGE("File must be in read-write mode for webm writer");
+ return false;
+ }
+ return true;
+}
+
WebmWriter::WebmWriter(int fd)
: mFd(dup(fd)),
mInitCheck(mFd < 0 ? NO_INIT : OK),
diff --git a/media/libstagefright/webm/include/webm/WebmWriter.h b/media/libstagefright/webm/include/webm/WebmWriter.h
index ed5bc4c..e339add 100644
--- a/media/libstagefright/webm/include/webm/WebmWriter.h
+++ b/media/libstagefright/webm/include/webm/WebmWriter.h
@@ -36,6 +36,10 @@
class WebmWriter : public MediaWriter {
public:
+ // Returns true if the file descriptor is opened using a mode
+ // which is compatible with WebmWriter.
+ // Note that this overloads that method in the base class.
+ static bool isFdOpenModeValid(int fd);
explicit WebmWriter(int fd);
~WebmWriter() { reset(); }
diff --git a/media/ndk/NdkMediaMuxer.cpp b/media/ndk/NdkMediaMuxer.cpp
index 1965e62..9d62884 100644
--- a/media/ndk/NdkMediaMuxer.cpp
+++ b/media/ndk/NdkMediaMuxer.cpp
@@ -46,7 +46,7 @@
if (mData == nullptr) {
return nullptr;
}
- mData->mImpl = new (std::nothrow) MediaMuxer(fd, (android::MediaMuxer::OutputFormat)format);
+ mData->mImpl = MediaMuxer::create(fd, (MediaMuxer::OutputFormat)format);
if (mData->mImpl == nullptr) {
delete mData;
return nullptr;
diff --git a/services/camera/libcameraservice/api2/HeicCompositeStream.cpp b/services/camera/libcameraservice/api2/HeicCompositeStream.cpp
index 54cc27a..71965f2 100644
--- a/services/camera/libcameraservice/api2/HeicCompositeStream.cpp
+++ b/services/camera/libcameraservice/api2/HeicCompositeStream.cpp
@@ -931,7 +931,7 @@
tempOutputFile.str().c_str(), errno);
return NO_INIT;
}
- inputFrame.muxer = new MediaMuxer(inputFrame.fileFd, MediaMuxer::OUTPUT_FORMAT_HEIF);
+ inputFrame.muxer = MediaMuxer::create(inputFrame.fileFd, MediaMuxer::OUTPUT_FORMAT_HEIF);
if (inputFrame.muxer == nullptr) {
ALOGE("%s: Failed to create MediaMuxer for file fd %d",
__FUNCTION__, inputFrame.fileFd);