Pigeon upgrade

[romaingyh]

Hello,

I updated the pigeon package because it was a very old version, 0.2.4 is nearly 4 years old when dart was still in 2.12.
Version 25.0.2 is more powerful, messages.dart is very simplified now because we can get rid of many XMessage classes and use primitive types in functions directly.

Sorry I know it's a big PR but there is no core or fundamental changes, just a big refractor due to pigeon upgrade.

Summary :

• Flutter: Upgrades pigeon and solid_lints
• Simplifies messages.dart and replaces viewId with playerId to prepare a future PR.
• Flutter: refactors method_channel_vlc_player.dart to reflect pigeon api changes.
• Android : refactors FlutterVlcPlayerBuilder.java and enums to reflect pigeon changes
• iOS: Makes pigeon generate swift instead of Obj-C (Messages.swift replaces messages.h and messages.m)
• iOS: refactors VlcViewController.swift to reflect pigeon changes

My aim with this PR is to make this package easier to maintain for the future, in particular I'd like to update it later with libvlc 4 which brings HDR and Picture in Picture.

I also think it would be great to get closer of official flutter video_player package federated architecture. We already have flutter_vlc_player_platform_interface, what would you think of another PR to go further and go fully federated with fluter_vlc_player, fluter_vlc_player_platform_interface, fluter_vlc_player_android, fluter_vlc_player_ios ?

[illia-romanenko]

Thanks for the PR @romaingyh!

Do you mind fixing formatting in Formatted lib/src/messages/messages.dart as per github check.

We are a bit overloaded now to do a quick review, but hope to get back to it in a couple of weeks.

[romaingyh]

@illia-romanenko Done 😃

[romaingyh]

Hi, do you have more time to review now ?

I need to implement some features for my app but I don't want to spam you with PRs.

Based on this branch I've been able to make the package fully federated using new pub workspace feature

name: flutter_vlc_player_workspace     # Can be anything

environment:
  sdk: ^3.6.0

publish_to: none

workspace:
  - flutter_vlc_player
  - flutter_vlc_player/example
  - flutter_vlc_player_platform_interface
  - flutter_vlc_player_android
  - flutter_vlc_player_ios

I'll soon start experiments with vlc 4 to see if it's possible to have Picture in Picture and windows/macOS support

[illia-romanenko]

/gemini review

[gemini-code-assist]

Code Review

This is a significant and valuable pull request that modernizes the pigeon usage across the project. The upgrade to Pigeon v25.0.2 and the subsequent refactoring have greatly simplified the message definitions and the native code interacting with them, especially by allowing primitive types directly in function calls and returns. The move to Swift for iOS Pigeon generation is also a welcome modernization.

Overall, the changes make the codebase cleaner, more maintainable, and easier to understand. The PR description clearly outlines the goals and the scope of changes, which is very helpful for a large refactor like this.

I've identified a few areas for improvement, primarily related to potential null pointer exceptions, resource management, and type conversions, detailed in the specific comments. Addressing these will further enhance the robustness and correctness of the plugin.

Summary of Findings

• Pigeon API Modernization: The upgrade to Pigeon v25.0.2 and the associated refactoring have significantly improved the API design by using direct primitive types and simplifying message structures. This enhances code clarity and maintainability across Dart, Java, and Swift.
• iOS Modernization: Migrating iOS Pigeon message generation from Objective-C to Swift is a positive step, aligning with modern iOS development practices.
• Improved Resource Management: Explicitly calling player.dispose() in the Android FlutterVlcPlayerBuilder.dispose() method is a key improvement for robust resource handling.
• Bug Fixes: Several bugs were fixed, including a NullPointerException in Android's getSnapshot and an incorrect null return in Android's getVideoTrack.
• Code Simplification in Enums (Java): Java enums DataSourceType and HwAcc were simplified by removing manual integer mapping, likely now handled by Pigeon using ordinals. This was not commented on directly due to review settings (low severity).
• Refactoring in FlutterVlcPlayerBuilder.java: The introduction of the getPlayer() helper method and consistent use of playerId streamlines the Android native code. This was not commented on directly due to review settings (low severity).
• Swift API Idiomatic Improvements: The iOS VlcViewController.swift and VlcViewBuilder.swift now use more idiomatic Swift constructs like computed properties and direct type parameters, enhancing readability. This was not commented on directly due to review settings (low severity).

Merge Readiness

This pull request introduces substantial and beneficial changes by upgrading Pigeon and refactoring the associated native and Dart code. The codebase is now cleaner and more modern.

Before merging, I recommend addressing the high and medium severity issues identified in the review comments, particularly:

1. The potential precision loss in seekTo on Android.
2. Ensuring consistent null handling in Android's getSnapshot.
3. Clarifying the hwAcc default behavior in iOS if an invalid value is passed.

Once these points are addressed, the PR will be in excellent shape. As an AI, I am not authorized to approve pull requests, so please ensure further review and approval by team members before merging.

[illia-romanenko]

@romaingyh please take a look at automated code review comments, and we should have some time this week to review too. As for other PRs - feel free to submit them as well - it's easier to look at multiple at once.

Thanks!

[romaingyh]

@illia-romanenko thanks, I resolved all comments except one for hardware acceleration default behavior. I let you see my answers to gemini

For other PRs I'll wait for this one to be completed because I based other branches on this and it would create too many diffs.

[gemini-code-assist]

Thanks for the update, @romaingyh! It's great to hear that you've addressed most of the points raised in the automated review.

I see you've noted the comment regarding the hardware acceleration default behavior. Feel free to continue the discussion directly on that specific review comment thread if you have further questions or would like to elaborate on your perspective. I'm here to help clarify or discuss alternative approaches.

Looking forward to seeing this PR finalized!

[solid-vovabeloded]

@romaingyh, added one minor suggestion here, but all the rest looks good to me. Thanks for your contribution!

[romaingyh]

@solid-vovabeloded I added your suggestion. I also and optimized imports, ran code formatting and fixed some warnings for java part.

I've just seen the setSize method in VLCTextureView.java with a weird if block on width and height but I think it's not related to this PR. I added a TODO though.