From 4ab2de10aa7abc9dea1099e99cf07e4f9fb77328 Mon Sep 17 00:00:00 2001 From: "daren.liao" Date: Tue, 14 Jun 2022 21:18:43 +0800 Subject: [PATCH] Fix SAD validation condition This CL fixes a bug in the condition for validating a Short Audio Descriptor. Without this CL, valid SADs get dropped, while invalid SADs get forwarded to the audio framework. With this CL, only valid SADs get forwarded to the audio framework. Test: atest and manual Bug: 215186955 Change-Id: Idfdff2546cf2188e428f4e7b925d402ed9e0e46e --- .../com/android/server/hdmi/Constants.java | 2 + .../android/server/hdmi/RequestSadAction.java | 10 +- .../server/hdmi/RequestSadActionTest.java | 111 +++++++++--------- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/services/core/java/com/android/server/hdmi/Constants.java b/services/core/java/com/android/server/hdmi/Constants.java index 157057d0e4b72..aaa9ee5f59267 100644 --- a/services/core/java/com/android/server/hdmi/Constants.java +++ b/services/core/java/com/android/server/hdmi/Constants.java @@ -337,6 +337,8 @@ final class Constants { static final int AUDIO_CODEC_WMAPRO = 0xE; // Support WMA-Pro static final int AUDIO_CODEC_MAX = 0xF; + static final int AUDIO_FORMAT_MASK = 0b0111_1000; + @StringDef({ AUDIO_DEVICE_ARC_IN, AUDIO_DEVICE_SPDIF, diff --git a/services/core/java/com/android/server/hdmi/RequestSadAction.java b/services/core/java/com/android/server/hdmi/RequestSadAction.java index 23aaf3260bac4..0188e963140eb 100644 --- a/services/core/java/com/android/server/hdmi/RequestSadAction.java +++ b/services/core/java/com/android/server/hdmi/RequestSadAction.java @@ -200,8 +200,14 @@ final class RequestSadAction extends HdmiCecFeatureAction { } private boolean isValidCodec(byte codec) { - return Constants.AUDIO_CODEC_NONE < (codec & 0xFF) - && (codec & 0xFF) <= Constants.AUDIO_CODEC_MAX; + // Bit 7 needs to be 0. + if ((codec & (1 << 7)) != 0) { + return false; + } + // Bit [6, 3] is the audio format code. + int audioFormatCode = (codec & Constants.AUDIO_FORMAT_MASK) >> 3; + return Constants.AUDIO_CODEC_NONE < audioFormatCode + && audioFormatCode <= Constants.AUDIO_CODEC_MAX; } private void updateResult(byte[] sad) { 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 8b314cd888788..c2519caabfee8 100644 --- a/services/tests/servicestests/src/com/android/server/hdmi/RequestSadActionTest.java +++ b/services/tests/servicestests/src/com/android/server/hdmi/RequestSadActionTest.java @@ -295,10 +295,10 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_1.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_1 = new byte[]{ - 0x01, 0x18, 0x4A, - 0x02, 0x64, 0x5A, - 0x03, 0x4B, 0x00, - 0x04, 0x20, 0x0A}; + 0x0F, 0x18, 0x4A, + 0x17, 0x64, 0x5A, + 0x1F, 0x4B, 0x00, + 0x27, 0x20, 0x0A}; HdmiCecMessage response1 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_1); assertThat(mNativeWrapper.getResultMessages()).contains(expected1); @@ -310,10 +310,10 @@ public class RequestSadActionTest { 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, 0x4B, 0x00, - 0x08, 0x20, 0x0A}; + 0x2F, 0x18, 0x4A, + 0x37, 0x64, 0x5A, + 0x3F, 0x4B, 0x00, + 0x47, 0x20, 0x0A}; HdmiCecMessage response2 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_2); assertThat(mNativeWrapper.getResultMessages()).contains(expected2); @@ -325,10 +325,10 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_3.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_3 = new byte[]{ - 0x09, 0x18, 0x4A, - 0x0A, 0x64, 0x5A, - 0x0B, 0x4B, 0x00, - 0x0C, 0x20, 0x0A}; + 0x4F, 0x18, 0x4A, + 0x57, 0x64, 0x5A, + 0x5F, 0x4B, 0x00, + 0x67, 0x20, 0x0A}; HdmiCecMessage response3 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_3); assertThat(mNativeWrapper.getResultMessages()).contains(expected3); @@ -340,9 +340,9 @@ public class RequestSadActionTest { 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, 0x00}; + 0x6F, 0x18, 0x4A, + 0x77, 0x64, 0x5A, + 0x7F, 0x4B, 0x00}; HdmiCecMessage response4 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_4); assertThat(mNativeWrapper.getResultMessages()).contains(expected4); @@ -371,9 +371,9 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_1.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_1 = new byte[]{ - 0x01, 0x18, 0x4A, - 0x03, 0x4B, 0x00, - 0x04, 0x20, 0x0A}; + 0x0F, 0x18, 0x4A, + 0x1F, 0x4B, 0x00, + 0x27, 0x20, 0x0A}; HdmiCecMessage response1 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_1); assertThat(mNativeWrapper.getResultMessages()).contains(expected1); @@ -385,7 +385,7 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_2.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_2 = new byte[]{ - 0x08, 0x20, 0x0A}; + 0x2F, 0x20, 0x0A}; HdmiCecMessage response2 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_2); assertThat(mNativeWrapper.getResultMessages()).contains(expected2); @@ -397,10 +397,10 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_3.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_3 = new byte[]{ - 0x09, 0x18, 0x4A, - 0x0A, 0x64, 0x5A, - 0x0B, 0x4B, 0x00, - 0x0C, 0x20, 0x0A}; + 0x4F, 0x18, 0x4A, + 0x57, 0x64, 0x5A, + 0x5F, 0x4B, 0x00, + 0x67, 0x20, 0x0A}; HdmiCecMessage response3 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_3); assertThat(mNativeWrapper.getResultMessages()).contains(expected3); @@ -412,7 +412,7 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_4.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_4 = new byte[]{ - 0x0F, 0x4B, 0x00}; + 0x7F, 0x4B, 0x00}; HdmiCecMessage response4 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_4); assertThat(mNativeWrapper.getResultMessages()).contains(expected4); @@ -440,11 +440,12 @@ public class RequestSadActionTest { HdmiCecMessage expected1 = HdmiCecMessageBuilder.buildRequestShortAudioDescriptor( mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_1.stream().mapToInt(i -> i).toArray()); + // Negative values require explicit casting byte[] sadsToRespond_1 = new byte[]{ - 0x20, 0x18, 0x4A, - 0x21, 0x64, 0x5A, - 0x22, 0x4B, 0x00, - 0x23, 0x20, 0x0A}; + (byte) 0x80, 0x18, 0x4A, + (byte) 0x82, 0x64, 0x5A, + (byte) 0x87, 0x4B, 0x00, + (byte) 0x8F, 0x20, 0x0A}; HdmiCecMessage response1 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_1); assertThat(mNativeWrapper.getResultMessages()).contains(expected1); @@ -456,10 +457,10 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_2.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_2 = new byte[]{ - 0x24, 0x18, 0x4A, - 0x25, 0x64, 0x5A, - 0x26, 0x4B, 0x00, - 0x27, 0x20, 0x0A}; + (byte) 0x92, 0x18, 0x4A, + (byte) 0x98, 0x64, 0x5A, + (byte) 0xA1, 0x4B, 0x00, + (byte) 0xA4, 0x20, 0x0A}; HdmiCecMessage response2 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_2); assertThat(mNativeWrapper.getResultMessages()).contains(expected2); @@ -471,10 +472,10 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_3.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_3 = new byte[]{ - 0x28, 0x18, 0x4A, - 0x29, 0x64, 0x5A, - 0x2A, 0x4B, 0x00, - 0x2B, 0x20, 0x0A}; + (byte) 0xB3, 0x18, 0x4A, + (byte) 0xBA, 0x64, 0x5A, + (byte) 0xCB, 0x4B, 0x00, + (byte) 0xCF, 0x20, 0x0A}; HdmiCecMessage response3 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_3); assertThat(mNativeWrapper.getResultMessages()).contains(expected3); @@ -486,9 +487,9 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_4.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_4 = new byte[]{ - 0x2C, 0x18, 0x4A, - 0x2D, 0x64, 0x5A, - 0x2E, 0x4B, 0x00}; + (byte) 0xF0, 0x18, 0x4A, + (byte) 0xF1, 0x64, 0x5A, + (byte) 0xF3, 0x4B, 0x00}; HdmiCecMessage response4 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_4); assertThat(mNativeWrapper.getResultMessages()).contains(expected4); @@ -510,10 +511,10 @@ public class RequestSadActionTest { mTvLogicalAddress, Constants.ADDR_AUDIO_SYSTEM, CODECS_TO_QUERY_1.stream().mapToInt(i -> i).toArray()); byte[] sadsToRespond_1 = new byte[]{ - 0x01, 0x18, - 0x02, 0x64, 0x5A, - 0x03, 0x4B, 0x00, - 0x04, 0x20, 0x0A}; + 0x0F, 0x18, + 0x17, 0x64, 0x5A, + 0x1F, 0x4B, 0x00, + 0x27, 0x20, 0x0A}; HdmiCecMessage response1 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_1); assertThat(mNativeWrapper.getResultMessages()).contains(expected1); @@ -580,10 +581,10 @@ public class RequestSadActionTest { new int[]{Constants.AUDIO_CODEC_LPCM, Constants.AUDIO_CODEC_DD, Constants.AUDIO_CODEC_MP3, Constants.AUDIO_CODEC_MPEG2}); byte[] sadsToRespond_1 = new byte[]{ - 0x01, 0x18, 0x4A, - 0x02, 0x64, 0x5A, - 0x04, 0x20, 0x0A, - 0x05, 0x18, 0x4A}; + 0x0F, 0x18, 0x4A, + 0x17, 0x64, 0x5A, + 0x27, 0x20, 0x0A, + 0x2F, 0x18, 0x4A}; HdmiCecMessage response1 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_1); assertThat(mNativeWrapper.getResultMessages()).contains(expected1); @@ -596,10 +597,10 @@ public class RequestSadActionTest { new int[]{Constants.AUDIO_CODEC_AAC, Constants.AUDIO_CODEC_DTS, Constants.AUDIO_CODEC_ATRAC, Constants.AUDIO_CODEC_DDP}); byte[] sadsToRespond_2 = new byte[]{ - 0x06, 0x64, 0x5A, - 0x07, 0x4B, 0x00, - 0x08, 0x20, 0x0A, - 0x09, 0x18, 0x4A}; + 0x37, 0x64, 0x5A, + 0x3F, 0x4B, 0x00, + 0x47, 0x20, 0x0A, + 0x4F, 0x18, 0x4A}; HdmiCecMessage response2 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_2); assertThat(mNativeWrapper.getResultMessages()).contains(expected2); @@ -612,10 +613,10 @@ public class RequestSadActionTest { new int[]{Constants.AUDIO_CODEC_DTSHD, Constants.AUDIO_CODEC_TRUEHD, Constants.AUDIO_CODEC_DST, Constants.AUDIO_CODEC_MAX}); byte[] sadsToRespond_3 = new byte[]{ - 0x0B, 0x4B, 0x00, - 0x0C, 0x20, 0x0A, - 0x0D, 0x18, 0x4A, - 0x0F, 0x4B, 0x00}; + 0x5F, 0x4B, 0x00, + 0x67, 0x20, 0x0A, + 0x6F, 0x18, 0x4A, + 0x7F, 0x4B, 0x00}; HdmiCecMessage response3 = HdmiCecMessageBuilder.buildReportShortAudioDescriptor( Constants.ADDR_AUDIO_SYSTEM, mTvLogicalAddress, sadsToRespond_3); assertThat(mNativeWrapper.getResultMessages()).contains(expected3);