Skip to content
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

Fix type conversion issue in VideoViewController #1620

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AdamTV
Copy link

@AdamTV AdamTV commented Mar 13, 2024

Gradle forward compatibility. Tested on a Flutter project using Gradle version 8.3, along with the current Agora SDK Gradle version

Gradle forward compatibility. Tested on a Flutter project using Gradle version 8.3, along with the current Agora SDK Gradle version
@littleGnAl
Copy link
Contributor

Sorry for the late reply, thanks for your contribution.
May I know more about the background of this pull request? What error do we face?

@AdamTV
Copy link
Author

AdamTV commented Mar 14, 2024

No worries. The error we face is a compile time incompatible type error in Flutter projects using more recent versions of Gradle. It is possible that flutter is converting integers to incompatible types when passing them into the VideoViewController.createTextureRender method. The proposed solution follows suit with the solution used for longs, moving away from type casting and using the native java type conversion method.

@littleGnAl
Copy link
Contributor

The error we face is a compile time incompatible type error in Flutter projects using more recent versions of Gradle.

I'm wondering what error you are facing, can you share the error logs?

I have tried to upgrade our example to Gradle 8.3 and migrate to the new flutter gradle plugin, it works fine without any other code changes included in this PR.

@AdamTV
Copy link
Author

AdamTV commented Mar 23, 2024

Relevant section of output from flutter run -v:
[ ] Compiling with JDK Java compiler API.
[ ] Incremental compilation of 11 classes completed in 1.235 secs.
[ ] ...\Pub\Cache\hosted\pub.dev\agora_rtc_engine-6.3.0\android\src\main\java\io\agora\agora_rtc_ng\VideoViewController.java:190: error: incompatible types: capture#1 of ? cannot be converted to int
[ +1 ms] final int videoSourceType = (int) args.get("videoSourceType");
[ ] ^
[ ] ...\Pub\Cache\hosted\pub.dev\agora_rtc_engine-6.3.0\android\src\main\java\io\agora\agora_rtc_ng\VideoViewController.java:191: error: incompatible types: capture#1 of ? cannot be converted to int
[ ] final int videoViewSetupMode = (int) args.get("videoViewSetupMode");
[ ] ^
[ ] 2 errors
[ +98 ms] FAILURE: Build failed with an exception.
[ ] * What went wrong:
[ ] Execution failed for task ':agora_rtc_engine:compileDebugJavaWithJavac'.
[ ] > Compilation failed; see the compiler error output for details.
[ ] * Try:
[ ] > Run with --scan to get full insights.
[ ] * Exception is:
[ ] org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':agora_rtc_engine:compileDebugJavaWithJavac'.
[ ] at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda$executeIfValid$1(ExecuteActionsTaskExecuter.java:149)
[ ] at org.gradle.internal.Try$Failure.ifSuccessfulOrElse(Try.java:282)
[ ] at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:147)
[ ] at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:135)
[ +1 ms] at org.gradle.api.internal.tasks.execution.FinalizePropertiesTaskExecuter.execute(FinalizePropertiesTaskExecuter.java:46)
[ ] at org.gradle.api.internal.tasks.execution.ResolveTaskExecutionModeExecuter.execute(ResolveTaskExecutionModeExecuter.java:51)
[ ] at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:57)
[ ] at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:74)
[ ] at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:36)
[ ] at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.executeTask(EventFiringTaskExecuter.java:77)
[ ] at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:55)
[ ] at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:52)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
[ ] at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
[ ] at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
[ ] at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter.execute(EventFiringTaskExecuter.java:52)
[ ] at org.gradle.execution.plan.LocalTaskNodeExecutor.execute(LocalTaskNodeExecutor.java:42)
[ ] at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:331)
[ ] at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:318)
[ +1 ms] at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.lambda$execute$0(DefaultTaskExecutionGraph.java:314)
[ ] at org.gradle.internal.operations.CurrentBuildOperationRef.with(CurrentBuildOperationRef.java:80)
[ ] at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:314)
[ ] at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:303)
[ ] at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.execute(DefaultPlanExecutor.java:463)
[ ] at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.run(DefaultPlanExecutor.java:380)
[ ] at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
[ ] at org.gradle.internal.concurrent.AbstractManagedExecutor$1.run(AbstractManagedExecutor.java:47)
[ ] at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
[ ] at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
[ ] at java.base/java.lang.Thread.run(Thread.java:840)
[ ] Caused by: org.gradle.api.internal.tasks.compile.CompilationFailedException: Compilation failed; see the compiler error output for details.
[ ] at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:57)
[ ] at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:39)
[ ] at org.gradle.api.internal.tasks.compile.daemon.AbstractIsolatedCompilerWorkerExecutor$CompilerWorkAction.execute(AbstractIsolatedCompilerWorkerExecutor.java:78)
[ ] at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
[ ] at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:54)
[ ] at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:48)
[ ] at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:100)
[ ] at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:48)
[ ] at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:32)
[ ] at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:22)
[ ] at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:96)
[ ] at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:65)
[ ] at org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:138)
[ ] at org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
[ ] at org.gradle.process.internal.worker.request.WorkerAction.lambda$run$0(WorkerAction.java:135)
[ ] at org.gradle.internal.operations.CurrentBuildOperationRef.with(CurrentBuildOperationRef.java:80)
[ ] at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:127)
[ ] at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[ ] at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[ ] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[ ] at java.lang.reflect.Method.invoke(Method.java:498)
[ ] at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
[ ] at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
[ ] at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
[ ] at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
[ ] at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
[ ] at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
[ ] at org.gradle.internal.concurrent.AbstractManagedExecutor$1.run(AbstractManagedExecutor.java:47)
[ ] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
[ ] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
[ ] at java.lang.Thread.run(Thread.java:750)
[ ] BUILD FAILED in 4s

@littleGnAl
Copy link
Contributor

This issue is most likely related to the JDK version.

@littleGnAl
Copy link
Contributor

I would recommend you use the MethodCall.argument to handle it rather than use our self-implemented function.

@AdamTV
Copy link
Author

AdamTV commented Mar 26, 2024

I appreciate your suggestion to utilize MethodCall.argument for handling the type conversion issue. While MethodCall.argument is indeed a robust way to retrieve call arguments in a type-safe manner within Flutter's plugin ecosystem, the solution might not fully address the underlying type compatibility challenges in this specific context for a few reasons:

Type Inference Limitations: MethodCall.argument provides a generic way to access arguments with some level of type safety. However, it may not always infer the correct type, especially when interfacing with dynamically-typed languages like Dart. This can lead to situations where explicit casting or type checking is still necessary, thus not eliminating the root cause of the "incompatible types" compilation error.

Runtime Over Compile-time Checking: Relying solely on MethodCall.argument shifts some type checking from compile-time to runtime. While this can offer flexibility, it potentially increases the risk of runtime errors in type conversion, especially when dealing with a wide variety of input types and values. The explicit nature of a dedicated conversion method like getInt() ensures that any type mismatches are handled predictably and consistently, enhancing error detection during development.

Performance Considerations: Although minimal, the overhead of runtime type checking and the potential for exception handling with MethodCall.argument could impact performance, especially in critical paths of an application. A direct method for type conversion allows for more streamlined and efficient code execution.

Consistency Across Different Data Types: The proposed getInt() approach provides a consistent pattern for handling various data types beyond just integers. It allows for a unified strategy for type conversion that can be extended to handle other primitives or complex types, ensuring uniformity in how data is processed and converted throughout the application.

Given these considerations, while MethodCall.argument is undoubtedly useful for many scenarios within Flutter plugins, the specific challenges posed by the current type conversion issue suggest that a more explicit and controlled approach, like implementing getInt(), might offer better safeguards against type mismatches, alongside improvements in code clarity and maintainability.

@littleGnAl
Copy link
Contributor

littleGnAl commented Mar 26, 2024

What I am concerned about the getInt() implementation logic, is that it parses the value to the string, and then parses it to int, I believe it's not necessary. It also has no type check on compile time.

I think the root cause of the error is related to the type definition of Map<?, ?> args, change it to Map<Object, Object> args may fix the issue.
https://github.com/AgoraIO-Extensions/Agora-Flutter-SDK/pull/1620/files#diff-9d77f8ce033caff1019afe20783d0a53a1c3bf75481fc97723bdca8da50aa265L184

I recommend the MethodCall.argument because, for a long-term purpose, using the built-in function from the Flutter SDK should be more reliable.

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

Successfully merging this pull request may close these issues.

2 participants