diff options
author | Tyler Gunn <tgunn@google.com> | 2017-05-10 17:46:06 -0700 |
---|---|---|
committer | Tyler Gunn <tgunn@google.com> | 2017-08-14 08:09:37 -0700 |
commit | ea812e490bc5585b08f6ef5d32c814a0b03bd115 (patch) | |
tree | e211d3e2ca1de22b1eeb7143b87d1c0510788eb7 | |
parent | ff0979256d1a67a62c726d6fe15264c496990605 (diff) | |
download | ims-ea812e490bc5585b08f6ef5d32c814a0b03bd115.tar.gz |
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
-rw-r--r-- | src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java | 90 |
1 files changed, 80 insertions, 10 deletions
diff --git a/src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java b/src/java/com/android/ims/internal/ImsVideoCallProviderWrapper.java index fcb9e09b..1aae5338 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.Log; 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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { 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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { * * @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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { * @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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { * @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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { } /** + * 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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { * @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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { 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 @@ public class ImsVideoCallProviderWrapper extends Connection.VideoProvider { 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 class ImsVideoCallProviderWrapper extends Connection.VideoProvider { public boolean wasVideoPausedFromSource(int source) { return mVideoPauseTracker.wasVideoPausedFromSource(source); } + + public void setUseVideoPauseWorkaround(boolean useVideoPauseWorkaround) { + mUseVideoPauseWorkaround = useVideoPauseWorkaround; + } } |