[issess]

Sometimes, VirtualDisplay DisplayInfo object returns null, So, I fix getting displayInfo from "dumpsys display".

[rom1v]

Thank you.

Sometimes, VirtualDisplay DisplayInfo object returns null

Do you know why?

Are there another getDisplayInfo method with different parameter types on your device (if you list all the methods available on manager.getClass())?

I would prefer that over parsing dumpsys display if possible. Otherwise, using it as a fallback looks good to me in principle.

[issess]

@rom1v Thank you for quick response.

Accessing VirtualDisplay restriced by UID or FLAG_PRIVATE, So It can return null object.

getDisplayInfoInternal(int displayId, int callingUid)
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/services/core/java/com/android/server/display/DisplayManagerService.java;drc=f6247e3d3067fea526382ae32d83f6c650449589;l=633

hasAccess(int uid, int flags, int ownerUid, int displayId)
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/view/Display.java;drc=52be024bc503a12de9baad55140d9a5385e818a3;l=1537

isUidPresentOnDisplayInternal(int uid, int displayId)
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/services/core/java/com/android/server/display/DisplayManagerService.java;drc=f6247e3d3067fea526382ae32d83f6c650449589;l=1362

I already tried list all methods in IDisplayManager, there are no way to get virtualdisplay displayInfo. I use dumpsys display , And then It works!

[rom1v]

@issess Thank you.

Just for info, does #3448 also fix the problem?

[issess]

No, It doesn't fixed yet. It can cause conflict my code. So I will modify my commit and apply dev branch again.

[issess]

I changed base branch from 'master' to 'dev'. please review again.

[rom1v]

Hi,

Sorry for the delay.

I reviewed, and took your branch and applied a few changes:

• added execReadOutput() to keep execReadLine() as is, that is returning the fist line without any trailing \r and/or \n (this is important for reading and writing settings);
• removed a few unnecessary capturing blocks from the regex;
• started the regex with ^ so that it only checks from the beginning of the lines (perfs);
• extracted the fallback code to a separate function.

Here is a branch: pr3416

One remaining thing to do is parsing flags. For example, on my device, here is an extract of dumpsys display:

    mBaseDisplayInfo=DisplayInfo{"Built-in Screen", displayId 0, FLAG_SECURE, FLAG_SUPPORTS_PROTECTED_BUFFERS, FLAG_TRUSTED, real 1440 x 3120, …

The declared flags must be set in the flags variable.

[issess]

Hi,

Sorry for the delay.

I reviewed, and took your branch and applied a few changes:

• added execReadOutput() to keep execReadLine() as is, that is returning the fist line without any trailing \r and/or \n (this is important for reading and writing settings);
• removed a few unnecessary capturing blocks from the regex;
• started the regex with ^ so that it only checks from the beginning of the lines (perfs);
• extracted the fallback code to a separate function.

Here is a branch: pr3416

One remaining thing to do is parsing flags. For example, on my device, here is an extract of dumpsys display:

    mBaseDisplayInfo=DisplayInfo{"Built-in Screen", displayId 0, FLAG_SECURE, FLAG_SUPPORTS_PROTECTED_BUFFERS, FLAG_TRUSTED, real 1440 x 3120, …

The declared flags must be set in the flags variable.

Great work.
I applied your commit and I implemented stringToFlags method to get display flags.
also, regex ^ doesn't work with multi line text. I tested it. So, I remove the ^
please review again!

[rom1v]

Thanks, I will try to review soon.

regex ^ doesn't work with multi line text. I tested it. So, I remove the ^

Even without multi-line, this avoids the regex engine to try to match the regex everywhere in the line (it forces to only match at the beginning of the line, so in theory the perfs are better).

[issess]

I added Pattern.MULTILINE option. It works.

[rom1v]

Thank you.

About the flags capture, since the captured text might include non-flags (fields not beginning with FLAG_), I replaced your String.split() by a separate regex to only capture real flags (and avoid testing reading fields with random names).

I also extracted to a separate function to call the parsing function from unit tests, and I added a unit test: pr3416.2

[issess]

Thank you.

There is more something to check after Android API 31.

https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/view/DisplayInfo.java;drc=db08ffe5f18f9335c07b367b740a94fa97206a80;l=729

In API31 emulator dumpsys display

    mBaseDisplayInfo=DisplayInfo{"Built-in Screen", displayId 0", displayGroupId 0, FLAG_SECURE, FLAG_SUPPORTS_PROTECTED_BUFFERS, FLAG_TRUSTED, real 1080 x 2280, largest app 1080 x 2280, smallest app 1080 x 2280, appVsyncOff 1000000, presDeadline 16666666, mode 1, defaultMode 1, modes [{id=1, width=1080, height=2280, fps=60.000004, alternativeRefreshRates=[]}], hdrCapabilities HdrCapabilities{mSupportedHdrTypes=[], mMaxLuminance=500.0, mMaxAverageLuminance=500.0, mMinLuminance=0.0}, userDisabledHdrTypes [], minimalPostProcessingSupported false, rotation 0, state ON, type INTERNAL, uniqueId "local:4619827259835644672", app 1080 x 2280, density 440 (440.0 x 440.0) dpi, layerStack 0, colorMode 0, supportedColorModes [0], address {port=0, model=0x401cec6a7a2b7b}, deviceProductInfo DeviceProductInfo{name=EMU_display_0, manufacturerPnpId=GGL, productId=1, modelYear=null, manufactureDate=ManufactureDate{week=27, year=2006}, connectionToSinkType=0}, removeMode 0, refreshRateOverride 0.0, brightnessMinimum 0.0, brightnessMaximum 1.0, brightnessDefault 0.39763778}
emulator64_x86_64_arm64:/ $

So, I added test case for API 31, And I change regex from (, FLAG_.*)? to .*?(, FLAG_.*)?

[issess]

And I added testcase for No FLAG display.

[rom1v]

👍

I rebased on dev, and applied the following minor changes:

• removed useless ? after .*
• test == null first to avoid Pattern.compile() if text == null (and to avoid additional indentation)
• fix typos

diff

diff --git a/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java b/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
index 9967c74e..4a2a65be 100644
--- a/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
+++ b/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
@@ -21,8 +21,8 @@ public final class DisplayManager {
     // public to call it from unit tests
     public static DisplayInfo parseDisplayInfo(String dumpsysDisplayOutput, int displayId) {
         Pattern regex = Pattern.compile(
-                "^    mBaseDisplayInfo=DisplayInfo\\{\".*\", displayId " + displayId + ".*?(, FLAG_.*)?, real ([0-9]+) x ([0-9]+).*, rotation ([0-9]+)"
-                        + ".*, layerStack ([0-9]+)",
+                "^    mBaseDisplayInfo=DisplayInfo\\{\".*\", displayId " + displayId + ".*(, FLAG_.*)?, real ([0-9]+) x ([0-9]+).*, rotation " +
+                        "([0-9]+).*, layerStack ([0-9]+)",
                 Pattern.MULTILINE);
         Matcher m = regex.matcher(dumpsysDisplayOutput);
         if (!m.find()) {
@@ -49,17 +49,19 @@ public final class DisplayManager {
 
     private static int parseDisplayFlags(String text) {
         Pattern regex = Pattern.compile("FLAG_[A-Z_]+");
+        if (text == null) {
+            return 0;
+        }
+
         int flags = 0;
-        if (text != null) {
-            Matcher m = regex.matcher(text);
-            while (m.find()) {
-                String flagString = m.group();
-                try {
-                    Field filed = Display.class.getDeclaredField(flagString);
-                    flags |= filed.getInt(null);
-                } catch (NoSuchFieldException | IllegalAccessException e) {
-                    // Silently ignore, some flags reported by "dumpsys display" are @TestApi
-                }
+        Matcher m = regex.matcher(text);
+        while (m.find()) {
+            String flagString = m.group();
+            try {
+                Field filed = Display.class.getDeclaredField(flagString);
+                flags |= filed.getInt(null);
+            } catch (NoSuchFieldException | IllegalAccessException e) {
+                // Silently ignore, some flags reported by "dumpsys display" are @TestApi
             }
         }
         return flags;
diff --git a/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java b/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java
index 460835dd..33810265 100644
--- a/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java
+++ b/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java
@@ -9,7 +9,7 @@ import org.junit.Test;
 
 public class CommandParserTest {
     @Test
-    public void testParseDisplayInfoFrqomDumpsysDisplay() {
+    public void testParseDisplayInfoFromDumpsysDisplay() {
         /* @formatter:off */
         String partialOutput = "Logical Displays: size=1\n"
                 + "  Display 0:\n"
@@ -51,7 +51,7 @@ public class CommandParserTest {
     }
 
     @Test
-    public void testParseDisplayInfoFrqomDumpsysDisplayAPI31() {
+    public void testParseDisplayInfoFromDumpsysDisplayAPI31() {
         /* @formatter:off */
         String partialOutput = "Logical Displays: size=1\n"
                 + "  Display 0:\n"
@@ -97,7 +97,7 @@ public class CommandParserTest {
     }
 
     @Test
-    public void testParseDisplayInfoFrqomDumpsysDisplayAPI31NoFlags() {
+    public void testParseDisplayInfoFromDumpsysDisplayAPI31NoFlags() {
         /* @formatter:off */
         String partialOutput = "Logical Displays: size=1\n"
                 + "  Display 0:\n"
@@ -136,7 +136,6 @@ public class CommandParserTest {
         Assert.assertEquals(0, displayInfo.getDisplayId());
         Assert.assertEquals(0, displayInfo.getRotation());
         Assert.assertEquals(0, displayInfo.getLayerStack());
-        // FLAG_TRUSTED does not exist in Display (@TestApi), so it won't be reported
         Assert.assertEquals(0, displayInfo.getFlags());
         Assert.assertEquals(1080, displayInfo.getSize().getWidth());
         Assert.assertEquals(2280, displayInfo.getSize().getHeight());

pr3416.3

[issess]

+1

I rebased on dev, and applied the following minor changes:

• removed useless ? after .*
• test == null first to avoid Pattern.compile() if text == null (and to avoid additional indentation)
• fix typos

diff

diff --git a/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java b/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
index 9967c74e..4a2a65be 100644
--- a/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
+++ b/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
@@ -21,8 +21,8 @@ public final class DisplayManager {
     // public to call it from unit tests
     public static DisplayInfo parseDisplayInfo(String dumpsysDisplayOutput, int displayId) {
         Pattern regex = Pattern.compile(
-                "^    mBaseDisplayInfo=DisplayInfo\\{\".*\", displayId " + displayId + ".*?(, FLAG_.*)?, real ([0-9]+) x ([0-9]+).*, rotation ([0-9]+)"
-                        + ".*, layerStack ([0-9]+)",
+                "^    mBaseDisplayInfo=DisplayInfo\\{\".*\", displayId " + displayId + ".*(, FLAG_.*)?, real ([0-9]+) x ([0-9]+).*, rotation " +
+                        "([0-9]+).*, layerStack ([0-9]+)",
                 Pattern.MULTILINE);
         Matcher m = regex.matcher(dumpsysDisplayOutput);
         if (!m.find()) {
@@ -49,17 +49,19 @@ public final class DisplayManager {
 
     private static int parseDisplayFlags(String text) {
         Pattern regex = Pattern.compile("FLAG_[A-Z_]+");
+        if (text == null) {
+            return 0;
+        }
+
         int flags = 0;
-        if (text != null) {
-            Matcher m = regex.matcher(text);
-            while (m.find()) {
-                String flagString = m.group();
-                try {
-                    Field filed = Display.class.getDeclaredField(flagString);
-                    flags |= filed.getInt(null);
-                } catch (NoSuchFieldException | IllegalAccessException e) {
-                    // Silently ignore, some flags reported by "dumpsys display" are @TestApi
-                }
+        Matcher m = regex.matcher(text);
+        while (m.find()) {
+            String flagString = m.group();
+            try {
+                Field filed = Display.class.getDeclaredField(flagString);
+                flags |= filed.getInt(null);
+            } catch (NoSuchFieldException | IllegalAccessException e) {
+                // Silently ignore, some flags reported by "dumpsys display" are @TestApi
             }
         }
         return flags;
diff --git a/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java b/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java
index 460835dd..33810265 100644
--- a/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java
+++ b/server/src/test/java/com/genymobile/scrcpy/CommandParserTest.java
@@ -9,7 +9,7 @@ import org.junit.Test;
 
 public class CommandParserTest {
     @Test
-    public void testParseDisplayInfoFrqomDumpsysDisplay() {
+    public void testParseDisplayInfoFromDumpsysDisplay() {
         /* @formatter:off */
         String partialOutput = "Logical Displays: size=1\n"
                 + "  Display 0:\n"
@@ -51,7 +51,7 @@ public class CommandParserTest {
     }
 
     @Test
-    public void testParseDisplayInfoFrqomDumpsysDisplayAPI31() {
+    public void testParseDisplayInfoFromDumpsysDisplayAPI31() {
         /* @formatter:off */
         String partialOutput = "Logical Displays: size=1\n"
                 + "  Display 0:\n"
@@ -97,7 +97,7 @@ public class CommandParserTest {
     }
 
     @Test
-    public void testParseDisplayInfoFrqomDumpsysDisplayAPI31NoFlags() {
+    public void testParseDisplayInfoFromDumpsysDisplayAPI31NoFlags() {
         /* @formatter:off */
         String partialOutput = "Logical Displays: size=1\n"
                 + "  Display 0:\n"
@@ -136,7 +136,6 @@ public class CommandParserTest {
         Assert.assertEquals(0, displayInfo.getDisplayId());
         Assert.assertEquals(0, displayInfo.getRotation());
         Assert.assertEquals(0, displayInfo.getLayerStack());
-        // FLAG_TRUSTED does not exist in Display (@TestApi), so it won't be reported
         Assert.assertEquals(0, displayInfo.getFlags());
         Assert.assertEquals(1080, displayInfo.getSize().getWidth());
         Assert.assertEquals(2280, displayInfo.getSize().getHeight());

pr3416.3

It's almost done.
but Regex ? is necessary. Testcase will fail.

[rom1v]

but Regex ? is necessary. Testcase will fail.

Hmm, indeed, but I don't understand why:

• .* means "any character, 0 to n times"
• for me, .*? means "possibly any character, 0 to n times"

Why is ? necessary?

[issess]

@rom1v
.* means Greedy quantifier. .*? means Lazy quantifier.
see, https://javascript.info/regexp-greedy-and-lazy

[issess]

I tested regex in https://regex101.com/r/8tpLez/1

[issess]

@rom1v
sync your commit 218a040 and latest dev branch

[rom1v]

I pushed pr3416.4 to make checkstyle happy.

However, I noticed a problem when the initial rotation is not the natural orientation (for example by running Firefox while the phone is in landscape mode), the reported rotation is not correct (so mirroring is broken).

The problem is that there are both mBaseDisplayInfo and mOverrideDisplayInfo:

    mBaseDisplayInfo=DisplayInfo{"Built-in Screen", displayId 0, FLAG_SECURE, FLAG_SUPPORTS_PROTECTED_BUFFERS, FLAG_TRUSTED, real 1440 x 3120, largest app 1440 x 31
20, smallest app 1440 x 3120, appVsyncOff 2000000, presDeadline 11111111, mode 2, defaultMode 1, modes [{id=1, width=1440, height=3120, fps=60.0}, {id=2, width=1440
, height=3120, fps=90.0}, {id=3, width=1080, height=2340, fps=90.0}, {id=4, width=1080, height=2340, fps=60.0}], hdrCapabilities HdrCapabilities{mSupportedHdrTypes=
[2, 3, 4], mMaxLuminance=540.0, mMaxAverageLuminance=270.1, mMinLuminance=0.2}, minimalPostProcessingSupported false, rotation 0, state ON, type INTERNAL, uniqueId 
"local:19260779396743297", app 1440 x 3120, density 600 (515.154 x 514.597) dpi, layerStack 0, colorMode 0, supportedColorModes [0, 7, 9], address {port=129, model=
0x446d93aa0a40}, deviceProductInfo DeviceProductInfo{name=, manufacturerPnpId=QCM, productId=1, modelYear=null, manufactureDate=ManufactureDate{week=27, year=2006},
 relativeAddress=null}, removeMode 0}
    mOverrideDisplayInfo=DisplayInfo{"Built-in Screen", displayId 0, FLAG_SECURE, FLAG_SUPPORTS_PROTECTED_BUFFERS, FLAG_TRUSTED, real 3120 x 1440, largest app 3120 x 2983, smallest app 1440 x 1303, appVsyncOff 2000000, presDeadline 11111111, mode 2, defaultMode 1, modes [{id=1, width=1440, height=3120, fps=60.0}, {id=2, width=1440, height=3120, fps=90.0}, {id=3, width=1080, height=2340, fps=90.0}, {id=4, width=1080, height=2340, fps=60.0}], hdrCapabilities HdrCapabilities{mSupportedHdrTypes=[2, 3, 4], mMaxLuminance=540.0, mMaxAverageLuminance=270.1, mMinLuminance=0.2}, minimalPostProcessingSupported false, rotation 3, state ON, type INTERNAL, uniqueId "local:19260779396743297", app 3120 x 1440, density 600 (515.154 x 514.597) dpi, layerStack 0, colorMode 0, supportedColorModes [0, 7, 9], address {port=129, model=0x446d93aa0a40}, deviceProductInfo DeviceProductInfo{name=, manufacturerPnpId=QCM, productId=1, modelYear=null, manufactureDate=ManufactureDate{week=27, year=2006}, relativeAddress=null}, removeMode 0}

I guess that the correct width and height must be read from the mBaseDisplayInfo (when rotation is not applied), but the rotation itself must be read from the mOverrideDisplayInfo). And I'm not even sure that mOverrideDisplayInfo is always present.

[rom1v]

I guess that the correct width and height must be read from the mBaseDisplayInfo (when rotation is not applied), but the rotation itself must be read from the mOverrideDisplayInfo).

Ah no, everything must be read from mOverrideDisplayInfo. So this patch fixes the problem:

diff --git a/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java b/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
index 19568489..bf172126 100644
--- a/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
+++ b/server/src/main/java/com/genymobile/scrcpy/wrappers/DisplayManager.java
@@ -21,8 +21,8 @@ public final class DisplayManager {
     // public to call it from unit tests
     public static DisplayInfo parseDisplayInfo(String dumpsysDisplayOutput, int displayId) {
         Pattern regex = Pattern.compile(
-                "^    mBaseDisplayInfo=DisplayInfo\\{\".*?\", displayId " + displayId + ".*?(, FLAG_.*)?, real ([0-9]+) x ([0-9]+).*?, rotation "
-                        + "([0-9]+).*?, layerStack ([0-9]+)",
+                "^    mOverrideDisplayInfo=DisplayInfo\\{\".*?\", displayId " + displayId + ".*?(, FLAG_.*)?, real ([0-9]+) x ([0-9]+).*?, "
+                        + "rotation ([0-9]+).*?, layerStack ([0-9]+)",
                 Pattern.MULTILINE);
         Matcher m = regex.matcher(dumpsysDisplayOutput);
         if (!m.find()) {

Branch pr3416.5.

(And I added a unit test to check the rotation value and the correct width and height when rotated.)

[issess]

@rom1v

You caught the bug well. :) I agree to change from "mBaseDisplayInfo" to "mOverrideDisplayInfo".
And, I sync 949b65d and latest dev branch.

thank you!

[rom1v]

Merged into dev 👍