From 140ad1bc10e678801d8084ce729bf56eeef38015 Mon Sep 17 00:00:00 2001 From: Paul Colta Date: Tue, 16 Aug 2022 09:55:27 +0000 Subject: HDMICEC: Avoid sending undesired messages The bug occurs if a playback device wakes up from the standby state, trigerring CEC initialization, when a message of type or is requesting it to become the active source. Because the playback device hasn't finished allocating LAs to local devices, the ActiveSourceAction triggered by the previous message is deferred. Before it has a LA allocated, the playback device ignores incoming messages of type or , not being aware of any active source changes. This fix buffers the and messages. After LAs allocation is done, the playback device can cancel any queued ActiveSourceAction if there are any messages of these types or in the buffer. Bug: 235434112 Test: make && atest HdmiCecLocalDevicePlaybackTest Change-Id: I318e090dcdb57ab023a1b401ec2503a21955007b --- .../com/android/server/hdmi/CecMessageBuffer.java | 22 ++++ .../com/android/server/hdmi/HdmiCecController.java | 15 ++- .../android/server/hdmi/HdmiCecLocalDevice.java | 11 +- .../server/hdmi/HdmiCecLocalDevicePlayback.java | 19 +++ .../android/server/hdmi/HdmiControlService.java | 11 +- .../hdmi/HdmiCecLocalDevicePlaybackTest.java | 127 +++++++++++++++++++-- 6 files changed, 186 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/hdmi/CecMessageBuffer.java b/services/core/java/com/android/server/hdmi/CecMessageBuffer.java index 8f971fd7db07..0c73582e66cb 100644 --- a/services/core/java/com/android/server/hdmi/CecMessageBuffer.java +++ b/services/core/java/com/android/server/hdmi/CecMessageBuffer.java @@ -48,6 +48,12 @@ final class CecMessageBuffer { case Constants.MESSAGE_SYSTEM_AUDIO_MODE_REQUEST: bufferSystemAudioModeRequest(message); return true; + case Constants.MESSAGE_ROUTING_CHANGE: + bufferRoutingChange(message); + return true; + case Constants.MESSAGE_SET_STREAM_PATH: + bufferSetStreamPath(message); + return true; // Add here if new message that needs to buffer default: // Do not need to buffer messages other than above @@ -89,6 +95,22 @@ final class CecMessageBuffer { } } + private void bufferRoutingChange(HdmiCecMessage message) { + if (!replaceMessageIfBuffered(message, Constants.MESSAGE_ROUTING_CHANGE)) { + mBuffer.add(message); + } + } + + private void bufferSetStreamPath(HdmiCecMessage message) { + if (!replaceMessageIfBuffered(message, Constants.MESSAGE_SET_STREAM_PATH)) { + mBuffer.add(message); + } + } + + public List getBuffer() { + return new ArrayList<>(mBuffer); + } + // Returns true if the message is replaced private boolean replaceMessageIfBuffered(HdmiCecMessage message, int opcode) { for (int i = 0; i < mBuffer.size(); i++) { diff --git a/services/core/java/com/android/server/hdmi/HdmiCecController.java b/services/core/java/com/android/server/hdmi/HdmiCecController.java index 97e9c6458376..5aa3fa4db4f1 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecController.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecController.java @@ -147,6 +147,9 @@ final class HdmiCecController { private final HdmiCecAtomWriter mHdmiCecAtomWriter; + // This variable is used for testing, in order to delay the logical address allocation. + private long mLogicalAddressAllocationDelay = 0; + // Private constructor. Use HdmiCecController.create(). private HdmiCecController( HdmiControlService service, NativeWrapper nativeWrapper, HdmiCecAtomWriter atomWriter) { @@ -215,12 +218,12 @@ final class HdmiCecController { final AllocateAddressCallback callback) { assertRunOnServiceThread(); - runOnIoThread(new Runnable() { + mIoHandler.postDelayed(new Runnable() { @Override public void run() { handleAllocateLogicalAddress(deviceType, preferredAddress, callback); } - }); + }, mLogicalAddressAllocationDelay); } /** @@ -385,6 +388,14 @@ final class HdmiCecController { mNativeWrapperImpl.nativeSetLanguage(language); } + /** + * This method is used for testing, in order to delay the logical address allocation. + */ + @VisibleForTesting + void setLogicalAddressAllocationDelay(long delay) { + mLogicalAddressAllocationDelay = delay; + } + /** * Returns true if the language code is well-formed. */ diff --git a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java index 1de1a7a5e1e5..2622cef32241 100755 --- a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevice.java @@ -649,6 +649,13 @@ abstract class HdmiCecLocalDevice { return Constants.NOT_HANDLED; } + /** + * Called after logical address allocation is finished, allowing a local device to react to + * messages in the buffer before they are processed. This method may be used to cancel deferred + * actions. + */ + protected void preprocessBufferedMessages(List bufferedMessages) {} + @Constants.RcProfile protected abstract int getRcProfile(); @@ -963,8 +970,10 @@ abstract class HdmiCecLocalDevice { } @ServiceThreadOnly - final void handleAddressAllocated(int logicalAddress, int reason) { + final void handleAddressAllocated( + int logicalAddress, List bufferedMessages, int reason) { assertRunOnServiceThread(); + preprocessBufferedMessages(bufferedMessages); mPreferredAddress = logicalAddress; updateDeviceFeatures(); if (mService.getCecVersion() >= HdmiControlManager.HDMI_CEC_VERSION_2_0) { diff --git a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java index 5cfe27a62e1b..ea54b3001163 100644 --- a/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java +++ b/services/core/java/com/android/server/hdmi/HdmiCecLocalDevicePlayback.java @@ -530,6 +530,25 @@ public class HdmiCecLocalDevicePlayback extends HdmiCecLocalDeviceSource { } } + /** + * Called after logical address allocation is finished, allowing a local device to react to + * messages in the buffer before they are processed. This method may be used to cancel deferred + * actions. + */ + @Override + protected void preprocessBufferedMessages(List bufferedMessages) { + for (HdmiCecMessage message: bufferedMessages) { + // Prevent the device from broadcasting message if the active path + // changed during address allocation. + if (message.getOpcode() == Constants.MESSAGE_ROUTING_CHANGE + || message.getOpcode() == Constants.MESSAGE_SET_STREAM_PATH + || message.getOpcode() == Constants.MESSAGE_ACTIVE_SOURCE) { + removeAction(ActiveSourceAction.class); + return; + } + } + } + @Override protected int findKeyReceiverAddress() { return Constants.ADDR_TV; diff --git a/services/core/java/com/android/server/hdmi/HdmiControlService.java b/services/core/java/com/android/server/hdmi/HdmiControlService.java index 6c37bf1bb9e2..fa8b5c1b8086 100644 --- a/services/core/java/com/android/server/hdmi/HdmiControlService.java +++ b/services/core/java/com/android/server/hdmi/HdmiControlService.java @@ -1079,9 +1079,10 @@ public class HdmiControlService extends SystemService { @ServiceThreadOnly private void notifyAddressAllocated(ArrayList devices, int initiatedBy) { assertRunOnServiceThread(); + List bufferedMessages = mCecMessageBuffer.getBuffer(); for (HdmiCecLocalDevice device : devices) { int address = device.getDeviceInfo().getLogicalAddress(); - device.handleAddressAllocated(address, initiatedBy); + device.handleAddressAllocated(address, bufferedMessages, initiatedBy); } if (isTvDeviceEnabled()) { tv().setSelectRequestBuffer(mSelectRequestBuffer); @@ -1361,8 +1362,9 @@ public class HdmiControlService extends SystemService { @Constants.HandleMessageResult int handleMessageResult = dispatchMessageToLocalDevice(message); - if (handleMessageResult == Constants.NOT_HANDLED - && !mAddressAllocated + // mAddressAllocated is false during address allocation, meaning there is no device to + // handle the message, so it should be buffered, if possible. + if (!mAddressAllocated && mCecMessageBuffer.bufferMessage(message)) { return Constants.HANDLED; } @@ -3286,7 +3288,8 @@ public class HdmiControlService extends SystemService { } @ServiceThreadOnly - private void onWakeUp(@WakeReason final int wakeUpAction) { + @VisibleForTesting + protected void onWakeUp(@WakeReason final int wakeUpAction) { assertRunOnServiceThread(); mPowerStatusController.setPowerStatus(HdmiControlManager.POWER_STATUS_TRANSIENT_TO_ON, false); diff --git a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java index 0f6addb452a1..fe9e0b6a2d07 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/HdmiCecLocalDevicePlaybackTest.java @@ -47,7 +47,6 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import java.util.ArrayList; -import java.util.Collections; import java.util.concurrent.TimeUnit; @SmallTest @@ -79,6 +78,7 @@ public class HdmiCecLocalDevicePlaybackTest { private TestLooper mTestLooper = new TestLooper(); private FakePowerManagerWrapper mPowerManager; private ArrayList mLocalDevices = new ArrayList<>(); + private ArrayList mLocalDeviceTypes = new ArrayList<>(); private int mPlaybackPhysicalAddress; private int mPlaybackLogicalAddress; private boolean mWokenUp; @@ -91,9 +91,11 @@ public class HdmiCecLocalDevicePlaybackTest { Context context = InstrumentationRegistry.getTargetContext(); mMyLooper = mTestLooper.getLooper(); + mLocalDeviceTypes.add(HdmiDeviceInfo.DEVICE_PLAYBACK); mHdmiControlService = new HdmiControlService(InstrumentationRegistry.getTargetContext(), - Collections.emptyList(), new FakeAudioDeviceVolumeManagerWrapper()) { + mLocalDeviceTypes, new FakeAudioDeviceVolumeManagerWrapper()) { + @Override void wakeUp() { mWokenUp = true; @@ -120,16 +122,6 @@ public class HdmiCecLocalDevicePlaybackTest { // do nothing } - @Override - boolean isPowerStandby() { - return false; - } - - @Override - boolean isPowerStandbyOrTransient() { - return false; - } - @Override boolean canGoToStandby() { return true; @@ -165,6 +157,7 @@ public class HdmiCecLocalDevicePlaybackTest { mNativeWrapper.clearResultMessages(); mHdmiCecLocalDevicePlayback.mPlaybackDeviceActionOnRoutingControl = HdmiProperties.playback_device_action_on_routing_control_values.NONE; + mHdmiControlService.setPowerStatus(HdmiControlManager.POWER_STATUS_ON); } @Test @@ -1199,6 +1192,9 @@ public class HdmiCecLocalDevicePlaybackTest { HdmiCecMessage setStreamPath = HdmiCecMessageBuilder.buildSetStreamPath(ADDR_TV, mPlaybackPhysicalAddress); mHdmiCecLocalDevicePlayback.dispatchMessage(setStreamPath); + // The ActiveSourceAction created from the message above is deferred until the device wakes + // up. + mHdmiControlService.onWakeUp(HdmiControlService.WAKE_UP_SCREEN_ON); mTestLooper.dispatchAll(); HdmiCecMessage activeSource = HdmiCecMessageBuilder.buildActiveSource( @@ -2088,4 +2084,111 @@ public class HdmiCecLocalDevicePlaybackTest { assertThat(mHdmiControlService.getHdmiCecNetwork().getDeviceInfoList(false)) .isEmpty(); } + + @Test + public void handleRoutingChange_addressNotAllocated_removeActiveSourceAction() { + long allocationDelay = TimeUnit.SECONDS.toMillis(60); + mHdmiCecLocalDevicePlayback.mPlaybackDeviceActionOnRoutingControl = + HdmiProperties + .playback_device_action_on_routing_control_values + .WAKE_UP_AND_SEND_ACTIVE_SOURCE; + mHdmiCecLocalDevicePlayback.setActiveSource(ADDR_TV, 0x0000, + "HdmiCecLocalDevicePlaybackTest"); + HdmiCecMessage routingChangeToPlayback = + HdmiCecMessageBuilder.buildRoutingChange(ADDR_TV, 0x0000, + mPlaybackPhysicalAddress); + HdmiCecMessage routingChangeToTv = + HdmiCecMessageBuilder.buildRoutingChange(ADDR_TV, mPlaybackPhysicalAddress, + 0x0000); + HdmiCecMessage unexpectedMessage = + HdmiCecMessageBuilder.buildActiveSource(mPlaybackLogicalAddress, + mPlaybackPhysicalAddress); + // 1. DUT goes to sleep. + mHdmiControlService.onStandby(HdmiControlService.STANDBY_SCREEN_OFF); + // Delay allocate logical address in order to trigger message buffering. + mHdmiCecController.setLogicalAddressAllocationDelay(allocationDelay); + mNativeWrapper.onCecMessage(routingChangeToPlayback); + mTestLooper.dispatchAll(); + // 2. DUT wakes up and defer ActiveSourceAction. + mHdmiControlService.onWakeUp(HdmiControlService.WAKE_UP_SCREEN_ON); + mTestLooper.dispatchAll(); + assertThat(mHdmiCecLocalDevicePlayback.getActions(ActiveSourceAction.class)).isNotEmpty(); + // 3. DUT buffers message to TV. + mNativeWrapper.onCecMessage(routingChangeToTv); + mTestLooper.dispatchAll(); + // 4. Allocation is finished and the ActiveSourceAction is removed from the queue. + // No message is sent by the DUT. + mTestLooper.moveTimeForward(allocationDelay); + mTestLooper.dispatchAll(); + assertThat(mHdmiCecLocalDevicePlayback.getActions(ActiveSourceAction.class)).isEmpty(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(unexpectedMessage); + } + + @Test + public void handleSetStreamPath_addressNotAllocated_removeActiveSourceAction() { + long allocationDelay = TimeUnit.SECONDS.toMillis(60); + mHdmiCecLocalDevicePlayback.setActiveSource(ADDR_TV, 0x0000, + "HdmiCecLocalDevicePlaybackTest"); + HdmiCecMessage setStreamPathToPlayback = + HdmiCecMessageBuilder.buildSetStreamPath(ADDR_TV, mPlaybackPhysicalAddress); + HdmiCecMessage setStreamPathToTv = + HdmiCecMessageBuilder.buildSetStreamPath(ADDR_TV, 0x0000); + HdmiCecMessage unexpectedMessage = + HdmiCecMessageBuilder.buildActiveSource(mPlaybackLogicalAddress, + mPlaybackPhysicalAddress); + // 1. DUT goes to sleep. + mHdmiControlService.onStandby(HdmiControlService.STANDBY_SCREEN_OFF); + // Delay allocate logical address in order to trigger message buffering. + mHdmiCecController.setLogicalAddressAllocationDelay(allocationDelay); + mNativeWrapper.onCecMessage(setStreamPathToPlayback); + mTestLooper.dispatchAll(); + // 2. DUT wakes up and defer ActiveSourceAction. + mHdmiControlService.onWakeUp(HdmiControlService.WAKE_UP_SCREEN_ON); + mTestLooper.dispatchAll(); + assertThat(mHdmiCecLocalDevicePlayback.getActions(ActiveSourceAction.class)).isNotEmpty(); + // 3. DUT buffers message to TV. + mNativeWrapper.onCecMessage(setStreamPathToTv); + mTestLooper.dispatchAll(); + // 4. Allocation is finished and the ActiveSourceAction is removed from the queue. + // No message is sent by the DUT. + mTestLooper.moveTimeForward(allocationDelay); + mTestLooper.dispatchAll(); + assertThat(mHdmiCecLocalDevicePlayback.getActions(ActiveSourceAction.class)).isEmpty(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(unexpectedMessage); + } + + @Test + public void handleActiveSource_addressNotAllocated_removeActiveSourceAction() { + long allocationDelay = TimeUnit.SECONDS.toMillis(60); + + mHdmiCecLocalDevicePlayback.setActiveSource(ADDR_TV, 0x0000, + "HdmiCecLocalDevicePlaybackTest"); + + HdmiCecMessage setStreamPathToPlayback = + HdmiCecMessageBuilder.buildSetStreamPath(ADDR_TV, mPlaybackPhysicalAddress); + HdmiCecMessage activeSourceFromTv = + HdmiCecMessageBuilder.buildActiveSource(ADDR_TV, 0x0000); + HdmiCecMessage unexpectedMessage = + HdmiCecMessageBuilder.buildActiveSource(mPlaybackLogicalAddress, + mPlaybackPhysicalAddress); + // 1. DUT goes to sleep. + mHdmiControlService.onStandby(HdmiControlService.STANDBY_SCREEN_OFF); + // Delay allocate logical address in order to trigger message buffering. + mHdmiCecController.setLogicalAddressAllocationDelay(allocationDelay); + mNativeWrapper.onCecMessage(setStreamPathToPlayback); + mTestLooper.dispatchAll(); + // 2. DUT wakes up and defer ActiveSourceAction. + mHdmiControlService.onWakeUp(HdmiControlService.WAKE_UP_SCREEN_ON); + mTestLooper.dispatchAll(); + assertThat(mHdmiCecLocalDevicePlayback.getActions(ActiveSourceAction.class)).isNotEmpty(); + // 3. DUT buffers message from TV. + mNativeWrapper.onCecMessage(activeSourceFromTv); + mTestLooper.dispatchAll(); + // 4. Allocation is finished and the ActiveSourceAction is removed from the queue. + // No message is sent by the DUT. + mTestLooper.moveTimeForward(allocationDelay); + mTestLooper.dispatchAll(); + assertThat(mHdmiCecLocalDevicePlayback.getActions(ActiveSourceAction.class)).isEmpty(); + assertThat(mNativeWrapper.getResultMessages()).doesNotContain(unexpectedMessage); + } } -- cgit v1.2.3-59-g8ed1b