transcoder: initial version of pause/resume
- Add pause/resume in TranscoderWrapper, save paused state on pause
and use it to create new transcoder on resume.
Misc fixes:
- TranscoderWrapper::stop should only cancel transcoder if the stop
is for the currently running job. Scheduler could call stop to
cancel a job any time.
- Don't hold TranscoderWrapper lock when running event runnable. If
the runnable calls back into scheduler, and scheduler may call
transcoder again and deadlock.
- Don't report abort as error if the transcoder is cancelled explicitly.
- Push decoder/encoder start as msgs, so that they could be skipped too
if the job is cancelled shortly after starts.
Tests:
Add tests for cancel/pause/resume with real transcoder.
bug: 154734285
bug: 154733948
test: unit testing
Change-Id: I2b7d3da69df53b92ab351db455310799ba0e0e8f
diff --git a/media/libmediatranscoding/transcoder/Android.bp b/media/libmediatranscoding/transcoder/Android.bp
index 59cd96d..c153a42 100644
--- a/media/libmediatranscoding/transcoder/Android.bp
+++ b/media/libmediatranscoding/transcoder/Android.bp
@@ -34,6 +34,8 @@
"libmediandk",
"libnativewindow",
"libutils",
+ // TODO: Use libbinder_ndk
+ "libbinder",
],
export_include_dirs: [
diff --git a/media/libmediatranscoding/transcoder/MediaTranscoder.cpp b/media/libmediatranscoding/transcoder/MediaTranscoder.cpp
index 69932f4..bde1cf6 100644
--- a/media/libmediatranscoding/transcoder/MediaTranscoder.cpp
+++ b/media/libmediatranscoding/transcoder/MediaTranscoder.cpp
@@ -18,6 +18,7 @@
#define LOG_TAG "MediaTranscoder"
#include <android-base/logging.h>
+#include <binder/Parcel.h>
#include <fcntl.h>
#include <media/MediaSampleReaderNDK.h>
#include <media/MediaSampleWriter.h>
@@ -84,6 +85,17 @@
}
void MediaTranscoder::sendCallback(media_status_t status) {
+ // If the transcoder is already cancelled explicitly, don't send any error callbacks.
+ // Tracks and sample writer will report errors for abort. However, currently we can't
+ // tell it apart from real errors. Ideally we still want to report real errors back
+ // to client, as there is a small chance that explicit abort and the real error come
+ // at around the same time, we should report that if abort has a specific error code.
+ // On the other hand, if the transcoder actually finished (status is AMEDIA_OK) at around
+ // the same time of the abort, we should still report the finish back to the client.
+ if (mCancelled && status != AMEDIA_OK) {
+ return;
+ }
+
bool expected = false;
if (mCallbackSent.compare_exchange_strong(expected, true)) {
if (status == AMEDIA_OK) {
@@ -149,11 +161,11 @@
std::shared_ptr<MediaTranscoder> MediaTranscoder::create(
const std::shared_ptr<CallbackInterface>& callbacks,
- const std::shared_ptr<Parcel>& pausedState) {
+ const std::shared_ptr<const Parcel>& pausedState) {
if (pausedState != nullptr) {
- LOG(ERROR) << "Initializing from paused state is currently not supported.";
- return nullptr;
- } else if (callbacks == nullptr) {
+ LOG(INFO) << "Initializing from paused state.";
+ }
+ if (callbacks == nullptr) {
LOG(ERROR) << "Callbacks cannot be null";
return nullptr;
}
@@ -309,15 +321,15 @@
return AMEDIA_OK;
}
-media_status_t MediaTranscoder::pause(std::shared_ptr<const Parcelable>* pausedState) {
- (void)pausedState;
- LOG(ERROR) << "Pause is not currently supported";
- return AMEDIA_ERROR_UNSUPPORTED;
+media_status_t MediaTranscoder::pause(std::shared_ptr<const Parcel>* pausedState) {
+ // TODO: write internal states to parcel.
+ *pausedState = std::make_shared<Parcel>();
+ return cancel();
}
media_status_t MediaTranscoder::resume() {
- LOG(ERROR) << "Resume is not currently supported";
- return AMEDIA_ERROR_UNSUPPORTED;
+ // TODO: restore internal states from parcel.
+ return start();
}
media_status_t MediaTranscoder::cancel() {
diff --git a/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp b/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp
index 4df3296..ca94b16 100644
--- a/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp
+++ b/media/libmediatranscoding/transcoder/VideoTrackTranscoder.cpp
@@ -380,20 +380,24 @@
}
media_status_t VideoTrackTranscoder::runTranscodeLoop() {
- media_status_t status = AMEDIA_OK;
+ // Push start decoder and encoder as two messages, so that these are subject to the
+ // stop request as well. If the job is cancelled (or paused) immediately after start,
+ // we don't need to waste time start then stop the codecs.
+ mCodecMessageQueue.push([this] {
+ media_status_t status = AMediaCodec_start(mDecoder);
+ if (status != AMEDIA_OK) {
+ LOG(ERROR) << "Unable to start video decoder: " << status;
+ mStatus = status;
+ }
+ });
- status = AMediaCodec_start(mDecoder);
- if (status != AMEDIA_OK) {
- LOG(ERROR) << "Unable to start video decoder: " << status;
- return status;
- }
-
- status = AMediaCodec_start(mEncoder.get());
- if (status != AMEDIA_OK) {
- LOG(ERROR) << "Unable to start video encoder: " << status;
- AMediaCodec_stop(mDecoder);
- return status;
- }
+ mCodecMessageQueue.push([this] {
+ media_status_t status = AMediaCodec_start(mEncoder.get());
+ if (status != AMEDIA_OK) {
+ LOG(ERROR) << "Unable to start video encoder: " << status;
+ mStatus = status;
+ }
+ });
// Process codec events until EOS is reached, transcoding is stopped or an error occurs.
while (!mStopRequested && !mEosFromEncoder && mStatus == AMEDIA_OK) {
diff --git a/media/libmediatranscoding/transcoder/include/media/MediaTranscoder.h b/media/libmediatranscoding/transcoder/include/media/MediaTranscoder.h
index e5a5d64..7a36c8c 100644
--- a/media/libmediatranscoding/transcoder/include/media/MediaTranscoder.h
+++ b/media/libmediatranscoding/transcoder/include/media/MediaTranscoder.h
@@ -17,8 +17,6 @@
#ifndef ANDROID_MEDIA_TRANSCODER_H
#define ANDROID_MEDIA_TRANSCODER_H
-#include <binder/Parcel.h>
-#include <binder/Parcelable.h>
#include <media/MediaTrackTranscoderCallback.h>
#include <media/NdkMediaError.h>
#include <media/NdkMediaFormat.h>
@@ -33,6 +31,7 @@
class MediaSampleReader;
class MediaSampleWriter;
+class Parcel;
class MediaTranscoder : public std::enable_shared_from_this<MediaTranscoder>,
public MediaTrackTranscoderCallback {
@@ -57,7 +56,7 @@
* resume.
*/
virtual void onCodecResourceLost(const MediaTranscoder* transcoder,
- const std::shared_ptr<const Parcelable>& pausedState) = 0;
+ const std::shared_ptr<const Parcel>& pausedState) = 0;
virtual ~CallbackInterface() = default;
};
@@ -69,7 +68,7 @@
*/
static std::shared_ptr<MediaTranscoder> create(
const std::shared_ptr<CallbackInterface>& callbacks,
- const std::shared_ptr<Parcel>& pausedState = nullptr);
+ const std::shared_ptr<const Parcel>& pausedState = nullptr);
/** Configures source from path fd. */
media_status_t configureSource(int fd);
@@ -102,8 +101,12 @@
* release the transcoder instance, clear the paused state and delete the partial destination
* file. The caller can optionally call cancel to let the transcoder clean up the partial
* destination file.
+ *
+ * TODO: use NDK AParcel instead
+ * libbinder shouldn't be used by mainline modules. When transcoding goes mainline
+ * it needs to be replaced by stable AParcel.
*/
- media_status_t pause(std::shared_ptr<const Parcelable>* pausedState);
+ media_status_t pause(std::shared_ptr<const Parcel>* pausedState);
/** Resumes a paused transcoding. */
media_status_t resume();
diff --git a/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp b/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp
index 67f1ca1..330261c 100644
--- a/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp
+++ b/media/libmediatranscoding/transcoder/tests/MediaTranscoderTests.cpp
@@ -20,6 +20,7 @@
#define LOG_TAG "MediaTranscoderTests"
#include <android-base/logging.h>
+#include <fcntl.h>
#include <gtest/gtest.h>
#include <media/MediaSampleReaderNDK.h>
#include <media/MediaTranscoder.h>
@@ -74,7 +75,7 @@
int32_t progress __unused) override {}
virtual void onCodecResourceLost(const MediaTranscoder* transcoder __unused,
- const std::shared_ptr<const Parcelable>& pausedState
+ const std::shared_ptr<const Parcel>& pausedState
__unused) override {}
void waitForTranscodingFinished() {