MediaPlayer2: use protobuf instead of parcel in invoke()
Also fix MEDIA_INFO handling code to send the event to the client
after handling internal update. (This fixes several cts test cases.)
Test: pass MediaPlayer2Test
Bug: 63934228
Change-Id: I5d4884353057a195b1f587694bfbf66cdf1fd23c
diff --git a/Android.bp b/Android.bp
index 07b3018..eeede0f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -679,6 +679,7 @@
static_libs: [
"framework-protos",
+ "mediaplayer2-protos",
"android.hidl.base-V1.0-java",
"android.hardware.cas-V1.0-java",
"android.hardware.contexthub-V1.0-java",
diff --git a/media/java/android/media/MediaPlayer2.java b/media/java/android/media/MediaPlayer2.java
index aa78b0d..89827bc 100644
--- a/media/java/android/media/MediaPlayer2.java
+++ b/media/java/android/media/MediaPlayer2.java
@@ -773,22 +773,6 @@
}
/**
- * Invoke a generic method on the native player using opaque
- * parcels for the request and reply. Both payloads' format is a
- * convention between the java caller and the native player.
- * Must be called after setDataSource to make sure a native player
- * exists. On failure, a RuntimeException is thrown.
- *
- * @param request Parcel with the data for the extension. The
- * caller must use {@link #newRequest()} to get one.
- *
- * @param reply Output parcel with the data returned by the
- * native player.
- * {@hide}
- */
- public void invoke(Parcel request, Parcel reply) { }
-
- /**
* Insert a task in the command queue to help the client to identify whether a batch
* of commands has been finished. When this command is processed, a notification
* {@code EventCallback.onCommandLabelReached} will be fired with the
diff --git a/media/java/android/media/MediaPlayer2Impl.java b/media/java/android/media/MediaPlayer2Impl.java
index c06b97b..42d3d22 100644
--- a/media/java/android/media/MediaPlayer2Impl.java
+++ b/media/java/android/media/MediaPlayer2Impl.java
@@ -25,6 +25,9 @@
import android.content.Context;
import android.content.res.AssetFileDescriptor;
import android.graphics.SurfaceTexture;
+import android.media.MediaPlayer2Proto;
+import android.media.MediaPlayer2Proto.PlayerMessage;
+import android.media.MediaPlayer2Proto.Value;
import android.media.SubtitleController.Anchor;
import android.media.SubtitleTrack.RenderingWidget;
import android.net.Uri;
@@ -49,6 +52,7 @@
import android.view.SurfaceHolder;
import android.widget.VideoView;
+import com.android.framework.protobuf.InvalidProtocolBufferException;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.Preconditions;
@@ -72,6 +76,7 @@
import java.util.Arrays;
import java.util.BitSet;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -555,28 +560,30 @@
}
/**
- * Invoke a generic method on the native player using opaque
- * parcels for the request and reply. Both payloads' format is a
+ * Invoke a generic method on the native player using opaque protocol
+ * buffer message for the request and reply. Both payloads' format is a
* convention between the java caller and the native player.
- * Must be called after setDataSource or setPlaylist to make sure a native player
- * exists. On failure, a RuntimeException is thrown.
*
- * @param request Parcel with the data for the extension. The
+ * @param request PlayerMessage for the extension. The
* caller must use {@link #newRequest()} to get one.
*
- * @param reply Output parcel with the data returned by the
+ * @return PlayerMessage with the data returned by the
* native player.
- * {@hide}
*/
- @Override
- public void invoke(Parcel request, Parcel reply) {
- int retcode = native_invoke(request, reply);
- reply.setDataPosition(0);
- if (retcode != 0) {
- throw new RuntimeException("failure code: " + retcode);
+ private PlayerMessage invoke(PlayerMessage msg) {
+ byte[] ret = _invoke(msg.toByteArray());
+ if (ret == null) {
+ return null;
+ }
+ try {
+ return PlayerMessage.parseFrom(ret);
+ } catch (InvalidProtocolBufferException e) {
+ return null;
}
}
+ private native byte[] _invoke(byte[] request);
+
@Override
public void notifyWhenCommandLabelReached(Object label) {
addTask(new Task(CALL_COMPLETED_NOTIFY_WHEN_COMMAND_LABEL_REACHED, false) {
@@ -683,16 +690,12 @@
final String msg = "Scaling mode " + mode + " is not supported";
throw new IllegalArgumentException(msg);
}
- Parcel request = Parcel.obtain();
- Parcel reply = Parcel.obtain();
- try {
- request.writeInt(INVOKE_ID_SET_VIDEO_SCALE_MODE);
- request.writeInt(mode);
- invoke(request, reply);
- } finally {
- request.recycle();
- reply.recycle();
- }
+ PlayerMessage request = PlayerMessage.newBuilder()
+ .addValues(Value.newBuilder()
+ .setInt32Value(INVOKE_ID_SET_VIDEO_SCALE_MODE))
+ .addValues(Value.newBuilder().setInt32Value(mode))
+ .build();
+ invoke(request);
}
});
}
@@ -1799,14 +1802,6 @@
private native void _setAuxEffectSendLevel(float level);
/*
- * @param request Parcel destinated to the media player.
- * @param reply[out] Parcel that will contain the reply.
- * @return The status code.
- */
- private native final int native_invoke(Parcel request, Parcel reply);
-
-
- /*
* @param update_only If true fetch only the set of metadata that have
* changed since the last invocation of getMetadata.
* The set is built using the unfiltered
@@ -1886,18 +1881,18 @@
final int mTrackType;
final MediaFormat mFormat;
- TrackInfoImpl(Parcel in) {
- mTrackType = in.readInt();
- // TODO: parcel in the full MediaFormat; currently we are using createSubtitleFormat
+ TrackInfoImpl(Iterator<Value> in) {
+ mTrackType = in.next().getInt32Value();
+ // TODO: build the full MediaFormat; currently we are using createSubtitleFormat
// even for audio/video tracks, meaning we only set the mime and language.
- String mime = in.readString();
- String language = in.readString();
+ String mime = in.next().getStringValue();
+ String language = in.next().getStringValue();
mFormat = MediaFormat.createSubtitleFormat(mime, language);
if (mTrackType == MEDIA_TRACK_TYPE_SUBTITLE) {
- mFormat.setInteger(MediaFormat.KEY_IS_AUTOSELECT, in.readInt());
- mFormat.setInteger(MediaFormat.KEY_IS_DEFAULT, in.readInt());
- mFormat.setInteger(MediaFormat.KEY_IS_FORCED_SUBTITLE, in.readInt());
+ mFormat.setInteger(MediaFormat.KEY_IS_AUTOSELECT, in.next().getInt32Value());
+ mFormat.setInteger(MediaFormat.KEY_IS_DEFAULT, in.next().getInt32Value());
+ mFormat.setInteger(MediaFormat.KEY_IS_FORCED_SUBTITLE, in.next().getInt32Value());
}
}
@@ -1952,23 +1947,6 @@
out.append("}");
return out.toString();
}
-
- /**
- * Used to read a TrackInfoImpl from a Parcel.
- */
- /* package private */ static final Parcelable.Creator<TrackInfoImpl> CREATOR
- = new Parcelable.Creator<TrackInfoImpl>() {
- @Override
- public TrackInfoImpl createFromParcel(Parcel in) {
- return new TrackInfoImpl(in);
- }
-
- @Override
- public TrackInfoImpl[] newArray(int size) {
- return new TrackInfoImpl[size];
- }
- };
-
};
// We would like domain specific classes with more informative names than the `first` and `second`
@@ -2010,17 +1988,23 @@
}
private TrackInfoImpl[] getInbandTrackInfoImpl() throws IllegalStateException {
- Parcel request = Parcel.obtain();
- Parcel reply = Parcel.obtain();
- try {
- request.writeInt(INVOKE_ID_GET_TRACK_INFO);
- invoke(request, reply);
- TrackInfoImpl trackInfo[] = reply.createTypedArray(TrackInfoImpl.CREATOR);
- return trackInfo;
- } finally {
- request.recycle();
- reply.recycle();
+ PlayerMessage request = PlayerMessage.newBuilder()
+ .addValues(Value.newBuilder().setInt32Value(INVOKE_ID_GET_TRACK_INFO))
+ .build();
+ PlayerMessage response = invoke(request);
+ if (response == null) {
+ return null;
}
+ Iterator<Value> in = response.getValuesList().iterator();
+ int size = in.next().getInt32Value();
+ if (size == 0) {
+ return null;
+ }
+ TrackInfoImpl trackInfo[] = new TrackInfoImpl[size];
+ for (int i = 0; i < size; ++i) {
+ trackInfo[i] = new TrackInfoImpl(in);
+ }
+ return trackInfo;
}
/*
@@ -2481,26 +2465,24 @@
}
}
- Parcel request = Parcel.obtain();
- Parcel reply = Parcel.obtain();
- try {
- request.writeInt(INVOKE_ID_GET_SELECTED_TRACK);
- request.writeInt(trackType);
- invoke(request, reply);
- int inbandTrackIndex = reply.readInt();
- synchronized (mIndexTrackPairs) {
- for (int i = 0; i < mIndexTrackPairs.size(); i++) {
- Pair<Integer, SubtitleTrack> p = mIndexTrackPairs.get(i);
- if (p.first != null && p.first == inbandTrackIndex) {
- return i;
- }
+ PlayerMessage request = PlayerMessage.newBuilder()
+ .addValues(Value.newBuilder().setInt32Value(INVOKE_ID_GET_SELECTED_TRACK))
+ .addValues(Value.newBuilder().setInt32Value(trackType))
+ .build();
+ PlayerMessage response = invoke(request);
+ if (response == null) {
+ return -1;
+ }
+ int inbandTrackIndex = response.getValues(0).getInt32Value();
+ synchronized (mIndexTrackPairs) {
+ for (int i = 0; i < mIndexTrackPairs.size(); i++) {
+ Pair<Integer, SubtitleTrack> p = mIndexTrackPairs.get(i);
+ if (p.first != null && p.first == inbandTrackIndex) {
+ return i;
}
}
- return -1;
- } finally {
- request.recycle();
- reply.recycle();
}
+ return -1;
}
/**
@@ -2617,16 +2599,12 @@
private void selectOrDeselectInbandTrack(int index, boolean select)
throws IllegalStateException {
- Parcel request = Parcel.obtain();
- Parcel reply = Parcel.obtain();
- try {
- request.writeInt(select? INVOKE_ID_SELECT_TRACK: INVOKE_ID_DESELECT_TRACK);
- request.writeInt(index);
- invoke(request, reply);
- } finally {
- request.recycle();
- reply.recycle();
- }
+ PlayerMessage request = PlayerMessage.newBuilder()
+ .addValues(Value.newBuilder().setInt32Value(
+ select? INVOKE_ID_SELECT_TRACK: INVOKE_ID_DESELECT_TRACK))
+ .addValues(Value.newBuilder().setInt32Value(index))
+ .build();
+ invoke(request);
}
// Have to declare protected for finalize() since it is protected
@@ -2935,20 +2913,7 @@
case MEDIA_INFO:
{
- synchronized (mEventCbLock) {
- for (Pair<Executor, EventCallback> cb : mEventCallbackRecords) {
- cb.first.execute(() -> cb.second.onInfo(
- mMediaPlayer, dsd, what, extra));
- }
- }
-
switch (msg.arg1) {
- case MEDIA_INFO_DATA_SOURCE_START:
- if (isCurrentSrcId) {
- prepareNextDataSource();
- }
- break;
-
case MEDIA_INFO_VIDEO_TRACK_LAGGING:
Log.i(TAG, "Info (" + msg.arg1 + "," + msg.arg2 + ")");
break;
@@ -2980,6 +2945,20 @@
}
break;
}
+
+ synchronized (mEventCbLock) {
+ for (Pair<Executor, EventCallback> cb : mEventCallbackRecords) {
+ cb.first.execute(() -> cb.second.onInfo(
+ mMediaPlayer, dsd, what, extra));
+ }
+ }
+
+ if (msg.arg1 == MEDIA_INFO_DATA_SOURCE_START) {
+ if (isCurrentSrcId) {
+ prepareNextDataSource();
+ }
+ }
+
// No real default action so far.
return;
}
diff --git a/media/jni/Android.bp b/media/jni/Android.bp
index de9a24b..b468164 100644
--- a/media/jni/Android.bp
+++ b/media/jni/Android.bp
@@ -133,8 +133,10 @@
"libmediaextractor",
"libmediametrics",
"libmediaplayer2",
+ "libmediaplayer2-protos",
"libmediautils",
"libnetd_client",
+ "libprotobuf-cpp-lite",
"libstagefright_esds",
"libstagefright_foundation",
"libstagefright_httplive",
diff --git a/media/jni/android_media_MediaPlayer2.cpp b/media/jni/android_media_MediaPlayer2.cpp
index 4265987..d924b38 100644
--- a/media/jni/android_media_MediaPlayer2.cpp
+++ b/media/jni/android_media_MediaPlayer2.cpp
@@ -56,6 +56,10 @@
#include "android_util_Binder.h"
#include <binder/Parcel.h>
+#include "mediaplayer2.pb.h"
+
+using android::media::MediaPlayer2Proto::PlayerMessage;
+
// Modular DRM begin
#define FIND_CLASS(var, className) \
var = env->FindClass(className); \
@@ -196,6 +200,11 @@
if (obj && obj->dataSize() > 0) {
jobject jParcel = createJavaParcelObject(env);
if (jParcel != NULL) {
+ // TODO: replace the following Parcel usages with proto.
+ // 1. MEDIA_DRM_INFO : DrmInfo
+ // 2. MEDIA_TIMED_TEXT : TimedText
+ // 2. MEDIA_SUBTITLE_DATA : SubtitleData
+ // 4. MEDIA_META_DATA : TimedMetaData
Parcel* nativeParcel = parcelForJavaObject(env, jParcel);
nativeParcel->setData(obj->data(), obj->dataSize());
env->CallStaticVoidMethod(mClass, fields.post_event, mObject,
@@ -911,6 +920,9 @@
return false;
}
+ // TODO: parcelForJavaObject() shouldn't be used since it's dependent on
+ // framework's Parcel implementation. This setParameter() is used
+ // only with AudioAttribute. Can this be used as jobject with JAudioTrack?
Parcel *request = parcelForJavaObject(env, java_request);
status_t err = mp->setParameter(key, *request);
if (err == OK) {
@@ -978,26 +990,35 @@
process_media_player_call( env, thiz, mp->setVolume((float) leftVolume, (float) rightVolume), NULL, NULL );
}
-// Sends the request and reply parcels to the media player via the
-// binder interface.
-static jint
-android_media_MediaPlayer2_invoke(JNIEnv *env, jobject thiz,
- jobject java_request, jobject java_reply)
-{
+static jbyteArray
+android_media_MediaPlayer2_invoke(JNIEnv *env, jobject thiz, jbyteArray requestData) {
sp<MediaPlayer2> media_player = getMediaPlayer(env, thiz);
- if (media_player == NULL ) {
+ if (media_player == NULL) {
jniThrowException(env, "java/lang/IllegalStateException", NULL);
- return UNKNOWN_ERROR;
+ return NULL;
}
- Parcel *request = parcelForJavaObject(env, java_request);
- Parcel *reply = parcelForJavaObject(env, java_reply);
+ // Get the byte[] pointer and data length.
+ jbyte* pData = env->GetByteArrayElements(requestData, NULL);
+ jsize pDataLen = env->GetArrayLength(requestData);
- request->setDataPosition(0);
+ // Deserialize from the byte stream.
+ PlayerMessage request;
+ PlayerMessage response;
+ request.ParseFromArray(pData, pDataLen);
- // Don't use process_media_player_call which use the async loop to
- // report errors, instead returns the status.
- return (jint) media_player->invoke(*request, reply);
+ media_player->invoke(request, &response);
+
+ int size = response.ByteSize();
+ jbyte* temp = new jbyte[size];
+ response.SerializeToArray(temp, size);
+
+ // return the response as a byte array.
+ jbyteArray out = env->NewByteArray(size);
+ env->SetByteArrayRegion(out, 0, size, temp);
+ delete[] temp;
+
+ return out;
}
// Sends the new filter to the client.
@@ -1512,7 +1533,7 @@
{"setLooping", "(Z)V", (void *)android_media_MediaPlayer2_setLooping},
{"isLooping", "()Z", (void *)android_media_MediaPlayer2_isLooping},
{"_setVolume", "(FF)V", (void *)android_media_MediaPlayer2_setVolume},
- {"native_invoke", "(Landroid/os/Parcel;Landroid/os/Parcel;)I",(void *)android_media_MediaPlayer2_invoke},
+ {"_invoke", "([B)[B", (void *)android_media_MediaPlayer2_invoke},
{"native_setMetadataFilter", "(Landroid/os/Parcel;)I", (void *)android_media_MediaPlayer2_setMetadataFilter},
{"native_getMetadata", "(ZZLandroid/os/Parcel;)Z", (void *)android_media_MediaPlayer2_getMetadata},
{"native_init", "()V", (void *)android_media_MediaPlayer2_native_init},
diff --git a/media/proto/Android.bp b/media/proto/Android.bp
new file mode 100644
index 0000000..50d44c3
--- /dev/null
+++ b/media/proto/Android.bp
@@ -0,0 +1,20 @@
+java_library_static {
+ name: "mediaplayer2-protos",
+ host_supported: true,
+ proto: {
+ type: "lite",
+ },
+ srcs: ["mediaplayer2.proto"],
+ no_framework_libs: true,
+ jarjar_rules: "jarjar-rules.txt",
+}
+
+cc_library_static {
+ name: "libmediaplayer2-protos",
+ host_supported: true,
+ proto: {
+ export_proto_headers: true,
+ type: "lite",
+ },
+ srcs: ["mediaplayer2.proto"],
+}
diff --git a/media/proto/jarjar-rules.txt b/media/proto/jarjar-rules.txt
new file mode 100644
index 0000000..7be6e73
--- /dev/null
+++ b/media/proto/jarjar-rules.txt
@@ -0,0 +1,2 @@
+rule com.google.protobuf.** com.android.framework.protobuf.@1
+
diff --git a/media/proto/mediaplayer2.proto b/media/proto/mediaplayer2.proto
new file mode 100644
index 0000000..6287d6cd
--- /dev/null
+++ b/media/proto/mediaplayer2.proto
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+// C++ namespace: android::media:MediaPlayer2Proto:
+package android.media.MediaPlayer2Proto;
+
+option java_package = "android.media";
+option java_outer_classname = "MediaPlayer2Proto";
+
+message Value {
+ // The kind of value.
+ oneof kind {
+ // Represents a boolean value.
+ bool bool_value = 1;
+ // Represents an int32 value.
+ int32 int32_value = 2;
+ // Represents an uint32 value.
+ uint32 uint32_value = 3;
+ // Represents an int64 value.
+ int64 int64_value = 4;
+ // Represents an uint64 value.
+ uint64 uint64_value = 5;
+ // Represents a float value.
+ double float_value = 6;
+ // Represents a double value.
+ double double_value = 7;
+ // Represents a string value.
+ string string_value = 8;
+ // Represents a bytes value.
+ bytes bytes_value = 9;
+ }
+}
+
+message PlayerMessage {
+ repeated Value values = 1;
+}