-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get camera list from camera service directly #5669
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you 👍
I just read a first pass, I will check in more details later.
It makes a difference only on some Samsung devices, is that right?
} | ||
} | ||
if (!isBackwardCompatible) { | ||
continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do that even with the CameraManager
then (the current implementation), correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. On my Xiaomi devices, there are two invalid cameras ("facing" is "external" and fails to open) that can be filtered by this capability. However, they are hidden in CameraManager.getCameraIdList
. I don't know if any OS returns non-backward-compatible cameras from CameraManager
|
||
List<String> cameraIds = new ArrayList<>(); | ||
for (CameraStatus cameraStatus : cameraStatuses) { | ||
if (cameraStatus.status != ICameraServiceListener.STATUS_NOT_PRESENT && cameraStatus.status != ICameraServiceListener.STATUS_ENUMERATING) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contrary to CameraManager.getCameraIdList()
, shouldHideCamera()
is not called. Is it on purpose to show camera that are wrongly hidden?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. shouldHideCamera
was added recently, so for example when a companion device app added a virtual camera, it won't be seen by normal apps, I don't know if anyone is doing that, but it should be interesting to try to include them.
It shouldn't affect Samsung devices, because It will affect Xiaomi devices (#4392 (comment)) and LineageOS (#4392 (comment)) |
While I was making tests for this PR with a Xiaomi Redmi Note 13, I noticed that dc2fcc4 (#5659) already fixes the problem. On
On
Can you confirm? |
I can confirm On Xiaomi Mi 11:
On Redmi K70 Pro:
Check for |
Thank you for your test 👍
Maybe we can keep camera 8 and 9 listed, but without printing the exception (and also disable it for other errors): diff --git server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java
index 088be7e7d..76bf503e1 100644
--- server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java
+++ server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java
@@ -141,11 +141,12 @@ public final class LogUtils {
try {
// Capture frame rates for low-FPS mode are the same for every resolution
Range<Integer>[] lowFpsRanges = characteristics.get(CameraCharacteristics.CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES);
- SortedSet<Integer> uniqueLowFps = getUniqueSet(lowFpsRanges);
- builder.append(", fps=").append(uniqueLowFps);
- } catch (Exception e) {
+ if (lowFpsRanges != null) {
+ SortedSet<Integer> uniqueLowFps = getUniqueSet(lowFpsRanges);
+ builder.append(", fps=").append(uniqueLowFps);
+ }
+ } catch (RuntimeException e) {
// Some devices may provide invalid ranges, causing an IllegalArgumentException "lower must be less than or equal to upper"
- Ln.w("Could not get available frame rates for camera " + id, e);
}
builder.append(')'); |
I think cameras 8 and 9 don't work at all, so not worth listing
|
OK, is it sufficient: diff --git server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java
index 088be7e7d..81d499209 100644
--- server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java
+++ server/src/main/java/com/genymobile/scrcpy/util/LogUtils.java
@@ -120,6 +120,23 @@ public final class LogUtils {
}
}
+ private static boolean isCameraBackwardCompatible(CameraCharacteristics characteristics) {
+ int[] capabilities = characteristics.get(CameraCharacteristics.REQUEST_AVAILABLE_CAPABILITIES);
+ if (capabilities == null) {
+ return false;
+ }
+
+ // Ignore depth cameras as suggested by official documentation
+ // <https://developer.android.com/media/camera/camera2/camera-enumeration>
+ for (int capability : capabilities) {
+ if (capability == CameraCharacteristics.REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
public static String buildCameraListMessage(boolean includeSizes) {
StringBuilder builder = new StringBuilder("List of cameras:");
CameraManager cameraManager = ServiceManager.getCameraManager();
@@ -132,6 +149,10 @@ public final class LogUtils {
builder.append("\n --camera-id=").append(id);
CameraCharacteristics characteristics = cameraManager.getCameraCharacteristics(id);
+ if (!isCameraBackwardCompatible(characteristics)) {
+ continue;
+ }
+
int facing = characteristics.get(CameraCharacteristics.LENS_FACING);
builder.append(" (").append(getCameraFacingName(facing)).append(", ");
In other words, is it still necessary on try {
…
} catch (IllegalArgumentException ignore) {
// Samsung devices might throw an IllegalArgumentException
// when getting camera characteristics for hidden cameras
} ? |
That diff still prints the IDs
( I think using a try-catch (with |
OK, so I removed this part: https://github.com/Genymobile/scrcpy/pull/5669/files#diff-b0c97217600727b27d9e22e3e26e2f68cbec7ac8a6666e16f8cf91e7080898ffR169-R171 Please check the |
|
Thank you, I close this PR since CameraService is not necessary (at least for now). |
This is a cleaned-up version of #4392 (comment).
I'm using similar code in my project to enumerate cameras, then use Scrcpy to mirror them, but recently 2337f52 broke this usage.
I think it should rather list and accept non-functional camera IDs, than rejecting functional ones.
This doesn't include the code to handle logical vs. physical cameras.