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/RequestSadAction.java b/services/core/java/com/android/server/hdmi/RequestSadAction.java index 702c0004cb91a..23aaf3260bac4 100644 --- a/services/core/java/com/android/server/hdmi/RequestSadAction.java +++ b/services/core/java/com/android/server/hdmi/RequestSadAction.java @@ -181,13 +181,20 @@ final class RequestSadAction extends HdmiCecFeatureAction { return true; } if (cmd.getOpcode() == Constants.MESSAGE_FEATURE_ABORT - && (cmd.getParams()[0] & 0xFF) == Constants.MESSAGE_REQUEST_SHORT_AUDIO_DESCRIPTOR - && (cmd.getParams()[1] & 0xFF) == Constants.ABORT_INVALID_OPERAND) { - // Queried SADs are not supported - mQueriedSadCount += MAX_SAD_PER_REQUEST; - mTimeoutRetry = 0; - querySad(); - return true; + && (cmd.getParams()[0] & 0xFF) + == Constants.MESSAGE_REQUEST_SHORT_AUDIO_DESCRIPTOR) { + if ((cmd.getParams()[1] & 0xFF) == Constants.ABORT_UNRECOGNIZED_OPCODE) { + // SAD feature is not supported + wrapUpAndFinish(); + return true; + } + if ((cmd.getParams()[1] & 0xFF) == Constants.ABORT_INVALID_OPERAND) { + // Queried SADs are not supported + mQueriedSadCount += MAX_SAD_PER_REQUEST; + mTimeoutRetry = 0; + querySad(); + return true; + } } return false; } @@ -211,9 +218,9 @@ final class RequestSadAction extends HdmiCecFeatureAction { querySad(); return; } - mQueriedSadCount += MAX_SAD_PER_REQUEST; - mTimeoutRetry = 0; - querySad(); + // Don't query any other SADs if one of the SAD queries ran into the maximum amount of + // retries. + wrapUpAndFinish(); } } 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 diff --git a/services/tests/servicestests/src/com/android/server/hdmi/RequestSadActionTest.java b/services/tests/servicestests/src/com/android/server/hdmi/RequestSadActionTest.java index f7983ca218162..3228e82b566b1 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/RequestSadActionTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/RequestSadActionTest.java @@ -139,11 +139,12 @@ public class RequestSadActionTest { } @Test - public void noResponse_queryAgain_emptyResult() { + public void noResponse_queryAgainOnce_emptyResult() { RequestSadAction action = new RequestSadAction(mHdmiCecLocalDeviceTv, ADDR_AUDIO_SYSTEM, mCallback); action.start(); mTestLooper.dispatchAll(); + assertThat(mSupportedSads).isNull(); HdmiCecMessage expected1 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, @@ -153,45 +154,90 @@ public class RequestSadActionTest { mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); assertThat(mNativeWrapper.getResultMessages()).contains(expected1); - mNativeWrapper.clearResultMessages(); mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); + assertThat(mSupportedSads).isNotNull(); + assertThat(mSupportedSads.size()).isEqualTo(0); + HdmiCecMessage expected2 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_2.stream().mapToInt(i -> i).toArray()); - assertThat(mNativeWrapper.getResultMessages()).contains(expected2); - mNativeWrapper.clearResultMessages(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - assertThat(mNativeWrapper.getResultMessages()).contains(expected2); - mNativeWrapper.clearResultMessages(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - HdmiCecMessage expected3 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_3.stream().mapToInt(i -> i).toArray()); - assertThat(mNativeWrapper.getResultMessages()).contains(expected3); - mNativeWrapper.clearResultMessages(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - assertThat(mNativeWrapper.getResultMessages()).contains(expected3); - mNativeWrapper.clearResultMessages(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - HdmiCecMessage expected4 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_4.stream().mapToInt(i -> i).toArray()); - assertThat(mNativeWrapper.getResultMessages()).contains(expected4); - mNativeWrapper.clearResultMessages(); + + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); - assertThat(mNativeWrapper.getResultMessages()).contains(expected4); mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected2); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected3); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected4); + assertThat(mSupportedSads.size()).isEqualTo(0); + } + + @Test + public void unrecognizedOpcode_dontQueryAgain_emptyResult() { + RequestSadAction action = new RequestSadAction(mHdmiCecLocalDeviceTv, ADDR_AUDIO_SYSTEM, + mCallback); + action.start(); + mTestLooper.dispatchAll(); + assertThat(mSupportedSads).isNull(); + + HdmiCecMessage unrecognizedOpcode = HdmiCecMessageBuilder.buildFeatureAbortCommand( + Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, + Constants.MESSAGE_REQUEST_SHORT_AUDIO_DESCRIPTOR, + Constants.ABORT_UNRECOGNIZED_OPCODE); + + HdmiCecMessage expected1 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( + mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, + CODECS_TO_QUERY_1.stream().mapToInt(i -> i).toArray()); + assertThat(mNativeWrapper.getResultMessages()).contains(expected1); + action.processCommand(unrecognizedOpcode); + mTestLooper.dispatchAll(); + + assertThat(mSupportedSads).isNotNull(); + assertThat(mSupportedSads.size()).isEqualTo(0); + + HdmiCecMessage expected2 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( + mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, + CODECS_TO_QUERY_2.stream().mapToInt(i -> i).toArray()); + HdmiCecMessage expected3 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( + mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, + CODECS_TO_QUERY_3.stream().mapToInt(i -> i).toArray()); + HdmiCecMessage expected4 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( + mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, + CODECS_TO_QUERY_4.stream().mapToInt(i -> i).toArray()); + + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected2); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected3); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected4); assertThat(mSupportedSads.size()).isEqualTo(0); } @@ -455,11 +501,12 @@ public class RequestSadActionTest { } @Test - public void invalidMessageLength_queryAgain() { + public void invalidMessageLength_queryAgainOnce() { RequestSadAction action = new RequestSadAction(mHdmiCecLocalDeviceTv, ADDR_AUDIO_SYSTEM, mCallback); action.start(); mTestLooper.dispatchAll(); + assertThat(mSupportedSads).isNull(); HdmiCecMessage expected1 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, @@ -482,63 +529,35 @@ public class RequestSadActionTest { mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); + assertThat(mSupportedSads).isNotNull(); + assertThat(mSupportedSads.size()).isEqualTo(0); + HdmiCecMessage expected2 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_2.stream().mapToInt(i -> i).toArray()); - byte[] sadsToRespond_2 = new byte[]{ - 0x05, 0x18, 0x4A, - 0x06, 0x64, 0x5A, - 0x07, - 0x08, 0x20, 0x0A}; - HdmiCecMessage response2 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( - Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_2); - assertThat(mNativeWrapper.getResultMessages()).contains(expected2); - mNativeWrapper.clearResultMessages(); - action.processCommand(response2); - mTestLooper.dispatchAll(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - assertThat(mNativeWrapper.getResultMessages()).contains(expected2); - mNativeWrapper.clearResultMessages(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - HdmiCecMessage expected3 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_3.stream().mapToInt(i -> i).toArray()); - byte[] sadsToRespond_3 = new byte[0]; - HdmiCecMessage response3 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( - Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_3); - assertThat(mNativeWrapper.getResultMessages()).contains(expected3); - mNativeWrapper.clearResultMessages(); - action.processCommand(response3); - mTestLooper.dispatchAll(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - assertThat(mNativeWrapper.getResultMessages()).contains(expected3); - mNativeWrapper.clearResultMessages(); - mTestLooper.moveTimeForward(TIMEOUT_MS); - mTestLooper.dispatchAll(); - HdmiCecMessage expected4 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_4.stream().mapToInt(i -> i).toArray()); - byte[] sadsToRespond_4 = new byte[]{ - 0x0D, 0x18, 0x4A, - 0x0E, 0x64, 0x5A, - 0x0F, 0x4B}; - HdmiCecMessage response4 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( - Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_4); - assertThat(mNativeWrapper.getResultMessages()).contains(expected4); - mNativeWrapper.clearResultMessages(); - action.processCommand(response4); + + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); + mTestLooper.dispatchAll(); + mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); - assertThat(mNativeWrapper.getResultMessages()).contains(expected4); mTestLooper.moveTimeForward(TIMEOUT_MS); mTestLooper.dispatchAll(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected2); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected3); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(expected4); assertThat(mSupportedSads.size()).isEqualTo(0); }