Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[camera_android] Fix camera android deprecation warning for CamcorderProfile.get() #7062

Closed
wants to merge 4 commits into from
Closed

[camera_android] Fix camera android deprecation warning for CamcorderProfile.get() #7062

wants to merge 4 commits into from

Conversation

navaronbracke
Copy link
Contributor

@navaronbracke navaronbracke commented Jan 30, 2023

This PR adds a @TargetApi() annotation and moves a @SuppressWarnings("deprecation") annotation to the right place.

List which issues are fixed by this PR. You must list at least one issue.
Fixes flutter/flutter#94999

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
N/A

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@navaronbracke
Copy link
Contributor Author

@camsim99 I verified it by running the example app and I don't get any deprecation warnings now.

@Hixie
Copy link
Contributor

Hixie commented Jan 30, 2023

test-exempt: configuration change

would one way to test this though be to have a test that verifies that building an app with this feature doesn't cause any spurious console output?

long-term maybe we could have the flutter tool have a --fatal-warnings option or something that fails when the build includes warnings? cc @christopherfujino

@christopherfujino
Copy link

test-exempt: configuration change

would one way to test this though be to have a test that verifies that building an app with this feature doesn't cause any spurious console output?

long-term maybe we could have the flutter tool have a --fatal-warnings option or something that fails when the build includes warnings? cc @christopherfujino

Greg from 2021 has your back: flutter/flutter#92031

@Hixie
Copy link
Contributor

Hixie commented Jan 30, 2023

Ah interesting. Can we add that flag to the tests in this repo? If we do, before the patch, does it cause the tests for this plugin to fail? If so, that's the ideal way to test it.

@navaronbracke
Copy link
Contributor Author

I ran a build of the example app using flutter build apk --fatal-warnings.

Without the fix I get the following output (and I assume that the exit code is also non-zero, as VSCode shows a red x in the terminal): (there is additional output relating to Runtime JAR files in the classpath should have the same version. but I'm interested in the deprecation warnings for this PR)

navaronbracke@MacBook-Pro-van-Navaron example % flutter build apk --fatal-warnings

💪 Building with sound null safety 💪

/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:154: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_HIGH);
                                 ^
/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:158: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_2160P);
                                 ^
/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:162: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_1080P);
                                 ^
/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:166: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_720P);
                                 ^
/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:170: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_480P);
                                 ^
/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:174: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_QVGA);
                                 ^
/Users/navaronbracke/.pub-cache/hosted/pub.dev/camera_android-0.10.3/android/src/main/java/io/flutter/plugins/camera/features/resolution/ResolutionFeature.java:178: warning: [deprecation] get(int,int) in CamcorderProfile has been deprecated
          return CamcorderProfile.get(cameraId, CamcorderProfile.QUALITY_LOW);
                                 ^
7 warnings
w: Runtime JAR files in the classpath should have the same version. These files were found in the classpath:
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.5.30/5fd47535cc85f9e24996f939c2de6583991481b0/kotlin-stdlib-jdk8-1.5.30.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.5.30/525f5a7fa6d7790a571c07dd24214ed2dda352fe/kotlin-stdlib-jdk7-1.5.30.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.7.10/d2abf9e77736acc4450dc4a3f707fa2c10f5099d/kotlin-stdlib-1.7.10.jar (version 1.7)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.7.10/bac80c520d0a9e3f3673bc2658c6ed02ef45a76a/kotlin-stdlib-common-1.7.10.jar (version 1.7)
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
e: /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/androidx.annotation/annotation/1.5.0/857678d6b4ca7b28571ef7935c668bdb57e15027/annotation-1.5.0.jar!/META-INF/annotation.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
e: /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.7.10/d2abf9e77736acc4450dc4a3f707fa2c10f5099d/kotlin-stdlib-1.7.10.jar!/META-INF/kotlin-stdlib.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
e: /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.7.10/bac80c520d0a9e3f3673bc2658c6ed02ef45a76a/kotlin-stdlib-common-1.7.10.jar!/META-INF/kotlin-stdlib-common.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
w: Runtime JAR files in the classpath should have the same version. These files were found in the classpath:
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.5.30/5fd47535cc85f9e24996f939c2de6583991481b0/kotlin-stdlib-jdk8-1.5.30.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.5.30/525f5a7fa6d7790a571c07dd24214ed2dda352fe/kotlin-stdlib-jdk7-1.5.30.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.7.10/d2abf9e77736acc4450dc4a3f707fa2c10f5099d/kotlin-stdlib-1.7.10.jar (version 1.7)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.7.10/bac80c520d0a9e3f3673bc2658c6ed02ef45a76a/kotlin-stdlib-common-1.7.10.jar (version 1.7)
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
No issues found.
Running Gradle task 'assembleRelease'...                           67.9s
✓  Built build/app/outputs/flutter-apk/app-release.apk (18.1MB).
Logger received error output during the run, and "--fatal-warnings" is enabled.

With the fix I get the following output (and VSCode shows a blue dot, so I assume it exited with exit code 0):

navaronbracke@MacBook-Pro-van-Navaron example % flutter build apk --fatal-warnings

💪 Building with sound null safety 💪

Running Gradle task 'assembleRelease'...                            5.2s
✓  Built build/app/outputs/flutter-apk/app-release.apk (18.1MB).

What I would do is set up a test that exercises flutter build apk --fatal-warnings for the example app,
which then checks the stdout & stderr for the absence of the deprecation warnings (and that the process exited with zero ofcourse)

@christopherfujino Could you give me an example that uses Process.run()? (I assume some of the tooling tests do that as well)

And where would I put this kind of test exactly? Since it involves building an APK, its not exactly a regular test.

@navaronbracke
Copy link
Contributor Author

On second thought, in https://github.com/flutter/plugins/blob/main/script/tool/lib/src/build_examples_command.dart we already build the examples. @stuartmorgan is there a reason why the --fatal-warnings flag is not used when building?

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan is there a reason why the --fatal-warnings flag is not used when building?

I wasn't aware that it existed. How does it work exactly? I.e., what constitutes a warning for the purposes of that flag? Is it doing per-platform regexes on the build output?

If it works reliably, we can certainly add it; we mostly have this iOS native warnings erroring, and flutter/flutter#91868 tracks doing that for Android (and other platforms as necessary).

@navaronbracke
Copy link
Contributor Author

navaronbracke commented Jan 31, 2023

How does it work exactly? I.e., what constitutes a warning for the purposes of that flag? Is it doing per-platform regexes on the build output?

Good question, since --help doesn't actually tell me about it (filed flutter/flutter#119636 for that one), I'll take a look at what the command actually does.

If it works reliably, we can certainly add it

Apparently, the flag yields different results when building for the first time and when building the same app for the second time. It only reports the errors the first time. Building the same app again - without any changes - results in a success output, even though the warnings themselves still apply. Working as intended, due to incremental builds.

@stuartmorgan-g
Copy link
Contributor

It only reports the errors the first time. Building the same app again - without any changes - results in a success output, even though the warnings themselves still apply. Yikes.

Do you still see the warnings in the verbose build output in that case? That behavior may be fundamental to incremental builds, rather than an issue with the flag.

@navaronbracke
Copy link
Contributor Author

Turning verbose mode on for the second run did not affect the outcome, still no warnings. Didn't directly relate that to incremental builds, so yes that works as expected then.

@navaronbracke
Copy link
Contributor Author

Unfortunately, passing --fatal-warnings to the build flags for the build-examples script didn't work due to the following:
Kotlin classpath mismatches are also warnings.

Example:

============================================================
|| Running for webview_flutter_android
============================================================

Building for: Android


BUILDING webview_flutter/webview_flutter_android/example for Android (apk)
Running command: "flutter build apk --fatal-warnings" in /Users/navaronbracke/Documents/plugins/packages/webview_flutter/webview_flutter_android/example
Running "flutter pub get" in example...
Resolving dependencies...
+ archive 3.3.2 (3.3.6 available)
+ async 2.10.0
+ boolean_selector 2.1.1
+ characters 1.2.1
+ clock 1.1.1
+ collection 1.17.0 (1.17.1 available)
+ crypto 3.0.2
+ espresso 0.2.0+6
+ fake_async 1.3.1
+ ffi 2.0.1
+ file 6.1.4
+ flutter 0.0.0 from sdk flutter
+ flutter_driver 0.0.0 from sdk flutter
+ flutter_test 0.0.0 from sdk flutter
+ fuchsia_remote_debug_protocol 0.0.0 from sdk flutter
+ integration_test 0.0.0 from sdk flutter
+ js 0.6.5 (0.6.7 available)
+ matcher 0.12.13 (0.12.14 available)
+ material_color_utilities 0.2.0
+ meta 1.8.0 (1.9.0 available)
+ path 1.8.2 (1.8.3 available)
+ path_provider 2.0.12
+ path_provider_android 2.0.22
+ path_provider_foundation 2.1.1
+ path_provider_linux 2.1.7
+ path_provider_platform_interface 2.0.5
+ path_provider_windows 2.1.3
+ platform 3.1.0
+ plugin_platform_interface 2.1.3
+ process 4.2.4
+ sky_engine 0.0.99 from sdk flutter
+ source_span 1.9.1
+ stack_trace 1.11.0
+ stream_channel 2.1.1
+ string_scanner 1.2.0
+ sync_http 0.3.1
+ term_glyph 1.2.1
+ test_api 0.4.16 (0.4.18 available)
+ typed_data 1.3.1
+ vector_math 2.1.4
+ vm_service 9.4.0 (10.1.2 available)
+ webdriver 3.0.1 (3.0.2 available)
+ webview_flutter_android 3.2.2 from path ..
+ webview_flutter_platform_interface 2.0.1
+ win32 3.1.3 (4.1.1 available)
+ xdg_directories 0.2.0+3
Changed 46 dependencies!

💪 Building with sound null safety 💪

Running Gradle task 'assembleRelease'...                        
Running Gradle task 'assembleRelease'...                           72.1s
✓  Built build/app/outputs/flutter-apk/app-release.apk (19.9MB).
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
w: Runtime JAR files in the classpath should have the same version. These files were found in the classpath:
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.5.31/ff5d99aecd328872494e8921b72bf6e3af97af3e/kotlin-stdlib-jdk8-1.5.31.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.5.31/77e0f2568912e45d26c31fd417a332458508acdf/kotlin-stdlib-jdk7-1.5.31.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.7.10/d2abf9e77736acc4450dc4a3f707fa2c10f5099d/kotlin-stdlib-1.7.10.jar (version 1.7)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.7.10/bac80c520d0a9e3f3673bc2658c6ed02ef45a76a/kotlin-stdlib-common-1.7.10.jar (version 1.7)
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
e: /Users/navaronbracke/.gradle/caches/transforms-3/3f412394cb23a31b0201c82aa8b2f686/transformed/espresso-core-3.5.1/jars/classes.jar!/META-INF/androidx.test.espresso.screenshot.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
e: /Users/navaronbracke/.gradle/caches/transforms-3/d56a68884f5ae51a5671899c19f56f75/transformed/jetified-core-1.5.0/jars/classes.jar!/META-INF/androidx.test.core.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
e: /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/androidx.annotation/annotation/1.5.0/857678d6b4ca7b28571ef7935c668bdb57e15027/annotation-1.5.0.jar!/META-INF/annotation.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
e: /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.7.10/d2abf9e77736acc4450dc4a3f707fa2c10f5099d/kotlin-stdlib-1.7.10.jar!/META-INF/kotlin-stdlib.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
e: /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.7.10/bac80c520d0a9e3f3673bc2658c6ed02ef45a76a/kotlin-stdlib-common-1.7.10.jar!/META-INF/kotlin-stdlib-common.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.7.1, expected version is 1.5.1.
w: Runtime JAR files in the classpath should have the same version. These files were found in the classpath:
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk8/1.5.31/ff5d99aecd328872494e8921b72bf6e3af97af3e/kotlin-stdlib-jdk8-1.5.31.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-jdk7/1.5.31/77e0f2568912e45d26c31fd417a332458508acdf/kotlin-stdlib-jdk7-1.5.31.jar (version 1.5)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib/1.7.10/d2abf9e77736acc4450dc4a3f707fa2c10f5099d/kotlin-stdlib-1.7.10.jar (version 1.7)
    /Users/navaronbracke/.gradle/caches/modules-2/files-2.1/org.jetbrains.kotlin/kotlin-stdlib-common/1.7.10/bac80c520d0a9e3f3673bc2658c6ed02ef45a76a/kotlin-stdlib-common-1.7.10.jar (version 1.7)
w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath
Logger received error output during the run, and "--fatal-warnings" is enabled.

@stuartmorgan-g
Copy link
Contributor

I don't know enough about Android builds to know if that's something we should (or even can?) be fixing in the build.

I've added a note about --fatal-warnings to flutter/flutter#91868 for future investigation; since we don't have a short-term solution to preventing warnings in general on Android, but do have an issue tracking it, I'm fine proceeding without a test here for now.

Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the change looks good, but I just made some changes to this code, so you may have to change something. Let me know after you merge the changes in main!

@navaronbracke
Copy link
Contributor Author

navaronbracke commented Feb 10, 2023

@camsim99 So I pulled in your recent changes relating to the preview size, but now I get the deprecation warnings again.

I saw that you wrote a comment about a workaround for a bug with EncoderProfiles, which requires the legacy code to work. I believe that the single @SuppressWarnings("deprecation") - on line 128 - that is now in ResolutionFeature.java (for the workaround) is not doing its job as intended, hence the warnings came back. In my original change I also removed such an annotation, which in turn did make the warnings disappear.

Do you have any ideas? Or will this only get fixed once that workaround is removed?

@camsim99
Copy link
Contributor

@navaronbracke Yeah, you're right. It will default to legacy code if the other code doesn't work. Does it work if you remove the annotation? Keeping it there may be the incorrect thing to do actually suppress the warning.

@@ -144,6 +143,8 @@ static Size computeBestPreviewSize(int cameraId, ResolutionPreset preset)
* @return The best possible {@link android.media.CamcorderProfile} that matches the supplied
* {@link ResolutionPreset}.
*/
@TargetApi(Build.VERSION_CODES.R)
@SuppressWarnings("deprecation")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@camsim99 Apparently @TargetApi() on its own does not silence the deprecation warning? IIRC my original fix (before your changes for that edge case with EncoderProfiles) only added that annotation to get rid of the deprecation warnings, which did work back then. Strange that I did have to add it back here.

@@ -125,8 +125,7 @@ static Size computeBestPreviewSize(int cameraId, ResolutionPreset preset)
}
}

@SuppressWarnings("deprecation")
// TODO(camsim99): Suppression is currently safe because legacy code is used as a fallback for SDK >= S.
// TODO(camsim99): Suppression is currently safe because legacy code is used as a fallback for SDK < S.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was probably a typo? You have if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {} a few lines above.

@stuartmorgan-g
Copy link
Contributor

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[camera] Build warnings on camera 0.9.4+5
5 participants