From 48b98602b28275dfdcab27e7f83a5b79b34b4798 Mon Sep 17 00:00:00 2001 From: Yan Han Date: Thu, 12 May 2022 15:14:37 +0200 Subject: [PATCH] Don't send until ARC is enabled Send only when ARC is physically enabled. Currently, this can be sent several seconds early, which violates the CEC spec. Currently, when initiating ARC, we do the following in parallel: 1. Query SADs -> Enable ARC 2. Send This CL queries SADs first, to better align the other two tasks: Query SADs -> | 1. Enable ARC | 2. Send Bug: 232241939 Test: manual; atest HdmiCecLocalDeviceTvTest Change-Id: Ifebc283a8143a7bb48e4cf14ef09090eb1f852fa --- .../server/hdmi/HdmiCecLocalDeviceTv.java | 45 ++++++-------- .../android/server/hdmi/RequestArcAction.java | 2 +- .../hdmi/RequestArcInitiationAction.java | 2 +- .../hdmi/SetArcTransmissionStateAction.java | 58 +++++++++++-------- .../server/hdmi/HdmiCecLocalDeviceTvTest.java | 13 ++++- 5 files changed, 64 insertions(+), 56 deletions(-) diff --git a/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java b/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java index 1ea1457439ecf..9bce471fd0cb6 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecLocalDeviceTv.java @@ -810,35 +810,24 @@ final class HdmiCecLocalDeviceTv extends HdmiCecLocalDevice { } } - /** - * Change ARC status into the given {@code enabled} status. - * - * @return {@code true} if ARC was in "Enabled" status - */ @ServiceThreadOnly - boolean setArcStatus(boolean enabled) { + void enableArc(List supportedSads) { assertRunOnServiceThread(); + HdmiLogger.debug("Set Arc Status[old:%b new:true]", mArcEstablished); - HdmiLogger.debug("Set Arc Status[old:%b new:%b]", mArcEstablished, enabled); - boolean oldStatus = mArcEstablished; - if (enabled) { - RequestSadAction action = new RequestSadAction( - this, Constants.ADDR_AUDIO_SYSTEM, - new RequestSadAction.RequestSadCallback() { - @Override - public void onRequestSadDone(List supportedSads) { - enableAudioReturnChannel(enabled); - notifyArcStatusToAudioService(enabled, supportedSads); - mArcEstablished = enabled; - } - }); - addAndStartAction(action); - } else { - enableAudioReturnChannel(enabled); - notifyArcStatusToAudioService(enabled, new ArrayList<>()); - mArcEstablished = enabled; - } - return oldStatus; + enableAudioReturnChannel(true); + notifyArcStatusToAudioService(true, supportedSads); + mArcEstablished = true; + } + + @ServiceThreadOnly + void disableArc() { + assertRunOnServiceThread(); + HdmiLogger.debug("Set Arc Status[old:%b new:false]", mArcEstablished); + + enableAudioReturnChannel(false); + notifyArcStatusToAudioService(false, new ArrayList<>()); + mArcEstablished = false; } /** @@ -1066,7 +1055,7 @@ final class HdmiCecLocalDeviceTv extends HdmiCecLocalDevice { protected int handleTerminateArc(HdmiCecMessage message) { assertRunOnServiceThread(); if (mService .isPowerStandbyOrTransient()) { - setArcStatus(false); + disableArc(); return Constants.HANDLED; } // Do not check ARC configuration since the AVR might have been already removed. @@ -1353,7 +1342,7 @@ final class HdmiCecLocalDeviceTv extends HdmiCecLocalDevice { if (avr == null) { return; } - setArcStatus(false); + disableArc(); // Seq #44. removeAllRunningArcAction(); diff --git a/services/core/java/com/android/server/hdmi/RequestArcAction.java b/services/core/java/com/android/server/hdmi/RequestArcAction.java index c70101c43d794..3d9a2905f4fbc 100644 --- a/services/core/java/com/android/server/hdmi/RequestArcAction.java +++ b/services/core/java/com/android/server/hdmi/RequestArcAction.java @@ -63,7 +63,7 @@ abstract class RequestArcAction extends HdmiCecFeatureAction { finish(); return true; } else if (originalOpcode == Constants.MESSAGE_REQUEST_ARC_INITIATION) { - tv().setArcStatus(false); + tv().disableArc(); finish(); return true; } diff --git a/services/core/java/com/android/server/hdmi/RequestArcInitiationAction.java b/services/core/java/com/android/server/hdmi/RequestArcInitiationAction.java index 4eb220fd65ee7..3b7f1dd0d9715 100644 --- a/services/core/java/com/android/server/hdmi/RequestArcInitiationAction.java +++ b/services/core/java/com/android/server/hdmi/RequestArcInitiationAction.java @@ -48,7 +48,7 @@ final class RequestArcInitiationAction extends RequestArcAction { public void onSendCompleted(int error) { if (error != SendMessageResult.SUCCESS) { // Turn off ARC status if fails. - tv().setArcStatus(false); + tv().disableArc(); finish(); } } diff --git a/services/core/java/com/android/server/hdmi/SetArcTransmissionStateAction.java b/services/core/java/com/android/server/hdmi/SetArcTransmissionStateAction.java index db93ad0617ff3..32e274ece9ab1 100644 --- a/services/core/java/com/android/server/hdmi/SetArcTransmissionStateAction.java +++ b/services/core/java/com/android/server/hdmi/SetArcTransmissionStateAction.java @@ -20,6 +20,8 @@ import android.hardware.hdmi.HdmiDeviceInfo; import android.hardware.tv.cec.V1_0.SendMessageResult; import android.util.Slog; +import java.util.List; + /** * Feature action that handles enabling/disabling of ARC transmission channel. * Once TV gets <Initiate ARC>, TV sends <Report ARC Initiated> to AV Receiver. @@ -55,21 +57,31 @@ final class SetArcTransmissionStateAction extends HdmiCecFeatureAction { boolean start() { // Seq #37. if (mEnabled) { - // Enable ARC status immediately before sending . - // If AVR responds with , disable ARC status again. - // This is different from spec that says that turns ARC status to - // "Enabled" if is acknowledged and no - // is received. - // But implemented this way to save the time having to wait for - // . - setArcStatus(true); - // If succeeds to send , wait general timeout - // to check whether there is no for . - mState = STATE_WAITING_TIMEOUT; - addTimer(mState, HdmiConfig.TIMEOUT_MS); - sendReportArcInitiated(); + // Request SADs before enabling ARC + RequestSadAction action = new RequestSadAction( + localDevice(), Constants.ADDR_AUDIO_SYSTEM, + new RequestSadAction.RequestSadCallback() { + @Override + public void onRequestSadDone(List supportedSads) { + // Enable ARC status immediately before sending . + // If AVR responds with , disable ARC status again. + // This is different from spec that says that turns ARC status to + // "Enabled" if is acknowledged and no + // is received. + // But implemented this way to save the time having to wait for + // . + Slog.i(TAG, "Enabling ARC"); + tv().enableArc(supportedSads); + // If succeeds to send , wait general timeout to + // check whether there is no for . + mState = STATE_WAITING_TIMEOUT; + addTimer(mState, HdmiConfig.TIMEOUT_MS); + sendReportArcInitiated(); + } + }); + addAndStartAction(action); } else { - setArcStatus(false); + disableArc(); finish(); } return true; @@ -92,7 +104,7 @@ final class SetArcTransmissionStateAction extends HdmiCecFeatureAction { case SendMessageResult.NACK: // If is negatively ack'ed, disable ARC and // send directly. - setArcStatus(false); + disableArc(); HdmiLogger.debug("Failed to send ."); finish(); break; @@ -101,16 +113,12 @@ final class SetArcTransmissionStateAction extends HdmiCecFeatureAction { }); } - private void setArcStatus(boolean enabled) { - tv().setArcStatus(enabled); - Slog.i(TAG, "Change arc status to " + enabled); + private void disableArc() { + Slog.i(TAG, "Disabling ARC"); - // If enabled before and set to "disabled" and send to - // av reciever. - if (!enabled) { - sendCommand(HdmiCecMessageBuilder.buildReportArcTerminated(getSourceAddress(), - mAvrAddress)); - } + tv().disableArc(); + sendCommand(HdmiCecMessageBuilder.buildReportArcTerminated(getSourceAddress(), + mAvrAddress)); } @Override @@ -124,7 +132,7 @@ final class SetArcTransmissionStateAction extends HdmiCecFeatureAction { int originalOpcode = cmd.getParams()[0] & 0xFF; if (originalOpcode == Constants.MESSAGE_REPORT_ARC_INITIATED) { HdmiLogger.debug("Feature aborted for "); - setArcStatus(false); + disableArc(); finish(); return true; } diff --git a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java index f27b8c2f4b3a3..8112ca8fbb142 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDeviceTvTest.java @@ -560,8 +560,19 @@ public class HdmiCecLocalDeviceTvTest { HdmiCecMessage reportArcInitiated = HdmiCecMessageBuilder.buildReportArcInitiated( ADDR_TV, ADDR_AUDIO_SYSTEM); - assertThat(mNativeWrapper.getResultMessages()).contains(reportArcInitiated); + // should only be sent after SAD querying is done + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(reportArcInitiated); + + // Finish querying SADs assertThat(mNativeWrapper.getResultMessages()).contains(SAD_QUERY); + mNativeWrapper.clearResultMessages(); + mTestLooper.moveTimeForward(HdmiConfig.TIMEOUT_MS); + mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).contains(SAD_QUERY); + mTestLooper.moveTimeForward(HdmiConfig.TIMEOUT_MS); + mTestLooper.dispatchAll(); + + assertThat(mNativeWrapper.getResultMessages()).contains(reportArcInitiated); } @Test