Add workaround for broken vendor camera on/off requests.
Scenario: A and B are in video call.
A goes to the background, pausing the video.
B attempts to turn off their camera.
The request fails to be interpreted by vendor code.
Reason: This SHOULD really be a request to go from/to:
Audio/TX/RX/Paused --> Audio/RX/Paused
However, it MUST be sent as:
Audio/TX/RX/Paused --> Audio/RX
The introduction of the VideoPauseTracker in N caused the request to be
sent in the former correct format rather than the latter incorrect format.
The VideoPauseTracker attempts to ensure that a request to pause sent by
the framework vs the incall ui are kept in sync (we use pause to disable
video on a call when the data limit is reached). In the process of ensuring
pause and resume requests were handled properly, this code was
fixing the malformed request.
Added a workaround to the code to ensure the requests remain in the same
broken format vendor code depends on.
Test: Manual, unit
Bug: 35304446
Merged-In: I9b974542234d3f567aba3f2996a815e39bc8963e
Change-Id: I9b974542234d3f567aba3f2996a815e39bc8963e
diff --git a/src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java b/src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java
index fcb9e09..1aae533 100644
--- a/src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java
+++ b/src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java
@@ -29,6 +29,7 @@
import android.telecom.VideoProfile;
import android.view.Surface;
+import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.os.SomeArgs;
import java.util.Collections;
@@ -67,6 +68,7 @@
private final Set<ImsVideoProviderWrapperCallback> mCallbacks = Collections.newSetFromMap(
new ConcurrentHashMap<ImsVideoProviderWrapperCallback, Boolean>(8, 0.9f, 1));
private VideoPauseTracker mVideoPauseTracker = new VideoPauseTracker();
+ private boolean mUseVideoPauseWorkaround = false;
private IBinder.DeathRecipient mDeathRecipient = new IBinder.DeathRecipient() {
@Override
@@ -207,13 +209,26 @@
*
* @param VideoProvider
*/
- public ImsVideoCallProviderWrapper(IImsVideoCallProvider VideoProvider)
+ public ImsVideoCallProviderWrapper(IImsVideoCallProvider videoProvider)
throws RemoteException {
- mVideoCallProvider = VideoProvider;
- mVideoCallProvider.asBinder().linkToDeath(mDeathRecipient, 0);
- mBinder = new ImsVideoCallCallback();
- mVideoCallProvider.setCallback(mBinder);
+ mVideoCallProvider = videoProvider;
+ if (videoProvider != null) {
+ mVideoCallProvider.asBinder().linkToDeath(mDeathRecipient, 0);
+
+ mBinder = new ImsVideoCallCallback();
+ mVideoCallProvider.setCallback(mBinder);
+ } else {
+ mBinder = null;
+ }
+ }
+
+ @VisibleForTesting
+ public ImsVideoCallProviderWrapper(IImsVideoCallProvider videoProvider,
+ VideoPauseTracker videoPauseTracker)
+ throws RemoteException {
+ this(videoProvider);
+ mVideoPauseTracker = videoPauseTracker;
}
/** @inheritDoc */
@@ -323,7 +338,8 @@
* @param to The to video state.
* @return {@code true} if a pause was requested.
*/
- private static boolean isPauseRequest(int from, int to) {
+ @VisibleForTesting
+ public static boolean isPauseRequest(int from, int to) {
boolean fromPaused = VideoProfile.isPaused(from);
boolean toPaused = VideoProfile.isPaused(to);
@@ -337,7 +353,8 @@
* @param to The to video state.
* @return {@code true} if a resume was requested.
*/
- private static boolean isResumeRequest(int from, int to) {
+ @VisibleForTesting
+ public static boolean isResumeRequest(int from, int to) {
boolean fromPaused = VideoProfile.isPaused(from);
boolean toPaused = VideoProfile.isPaused(to);
@@ -345,6 +362,30 @@
}
/**
+ * Determines if this request includes turning the camera off (ie turning off transmission).
+ * @param from the from video state.
+ * @param to the to video state.
+ * @return true if the state change disables the user's camera.
+ */
+ @VisibleForTesting
+ public static boolean isTurnOffCameraRequest(int from, int to) {
+ return VideoProfile.isTransmissionEnabled(from)
+ && !VideoProfile.isTransmissionEnabled(to);
+ }
+
+ /**
+ * Determines if this request includes turning the camera on (ie turning on transmission).
+ * @param from the from video state.
+ * @param to the to video state.
+ * @return true if the state change enables the user's camera.
+ */
+ @VisibleForTesting
+ public static boolean isTurnOnCameraRequest(int from, int to) {
+ return !VideoProfile.isTransmissionEnabled(from)
+ && VideoProfile.isTransmissionEnabled(to);
+ }
+
+ /**
* Filters incoming pause and resume requests based on whether there are other active pause or
* resume requests at the current time.
*
@@ -361,7 +402,8 @@
* @return The new toProfile, with the pause bit set or unset based on whether we should
* actually pause or resume the video at the current time.
*/
- private VideoProfile maybeFilterPauseResume(VideoProfile fromProfile, VideoProfile toProfile,
+ @VisibleForTesting
+ public VideoProfile maybeFilterPauseResume(VideoProfile fromProfile, VideoProfile toProfile,
int source) {
int fromVideoState = fromProfile.getVideoState();
int toVideoState = toProfile.getVideoState();
@@ -381,7 +423,9 @@
boolean isPauseRequest = isPauseRequest(fromVideoState, toVideoState) || isPauseSpecialCase;
boolean isResumeRequest = isResumeRequest(fromVideoState, toVideoState);
if (isPauseRequest) {
- Log.i(this, "maybeFilterPauseResume: isPauseRequest");
+ Log.i(this, "maybeFilterPauseResume: isPauseRequest (from=%s, to=%s)",
+ VideoProfile.videoStateToString(fromVideoState),
+ VideoProfile.videoStateToString(toVideoState));
// Check if we have already paused the video in the past.
if (!mVideoPauseTracker.shouldPauseVideoFor(source) && !isPauseSpecialCase) {
// Note: We don't want to remove the "pause" in the "special case" scenario. If we
@@ -393,7 +437,29 @@
toProfile = new VideoProfile(toVideoState, toProfile.getQuality());
}
} else if (isResumeRequest) {
- Log.i(this, "maybeFilterPauseResume: isResumeRequest");
+ boolean isTurnOffCameraRequest = isTurnOffCameraRequest(fromVideoState, toVideoState);
+ boolean isTurnOnCameraRequest = isTurnOnCameraRequest(fromVideoState, toVideoState);
+ // TODO: Fix vendor code so that this isn't required.
+ // Some vendors do not properly handle turning the camera on/off when the video is
+ // in paused state.
+ // If the request is to turn on/off the camera, it might be in the unfortunate format:
+ // FROM: Audio Tx Rx Pause TO: Audio Rx
+ // FROM: Audio Rx Pause TO: Audio Rx Tx
+ // If this is the case, we should not treat this request as a resume request as well.
+ // Ideally the IMS stack should treat a turn off camera request as:
+ // FROM: Audio Tx Rx Pause TO: Audio Rx Pause
+ // FROM: Audio Rx Pause TO: Audio Rx Tx Pause
+ // Unfortunately, it does not. ¯\_(ツ)_/¯
+ if (mUseVideoPauseWorkaround && (isTurnOffCameraRequest || isTurnOnCameraRequest)) {
+ Log.i(this, "maybeFilterPauseResume: isResumeRequest, but camera turning on/off so "
+ + "skipping (from=%s, to=%s)",
+ VideoProfile.videoStateToString(fromVideoState),
+ VideoProfile.videoStateToString(toVideoState));
+ return toProfile;
+ }
+ Log.i(this, "maybeFilterPauseResume: isResumeRequest (from=%s, to=%s)",
+ VideoProfile.videoStateToString(fromVideoState),
+ VideoProfile.videoStateToString(toVideoState));
// Check if we should remain paused (other pause requests pending).
if (!mVideoPauseTracker.shouldResumeVideoFor(source)) {
// There are other pause requests from other sources which are still active, so we
@@ -465,4 +531,8 @@
public boolean wasVideoPausedFromSource(int source) {
return mVideoPauseTracker.wasVideoPausedFromSource(source);
}
+
+ public void setUseVideoPauseWorkaround(boolean useVideoPauseWorkaround) {
+ mUseVideoPauseWorkaround = useVideoPauseWorkaround;
+ }
}
