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

[video_player_avplay]Add setData and getData interface #822

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

xiaowei-guan
Copy link
Contributor

@xiaowei-guan xiaowei-guan commented Feb 28, 2025

#821

new interface setData and getData for dash player:

enum DashPlayerProperty {
  /// Max band width of dash player, the value is int.
  maxBandWidth,

  /// Dash player stream info, the value is json string.
  dashStreamInfo,
}

/// Set dashplayer properties,can be called after initialized.
  Future<bool> setData(int playerId, Map<DashPlayerProperty, Object> data);

  /// Get dashplayer properties,can be called after initialized.
  Future<Map<DashPlayerProperty, Object>> getData(
    int playerId,
    Set<DashPlayerProperty> keys,
  );

@xiaowei-guan xiaowei-guan changed the title [WIP][video_player_avplay]Add setData and getData interface [wip][video_player_avplay]Add setData and getData interface Mar 4, 2025
@xiaowei-guan xiaowei-guan force-pushed the video_player_avplay_set_get_data branch from cd858a1 to b705706 Compare March 5, 2025 06:21
@xiaowei-guan xiaowei-guan force-pushed the video_player_avplay_set_get_data branch 3 times, most recently from ac7e675 to 94e24e0 Compare March 5, 2025 07:16
@xiaowei-guan xiaowei-guan changed the title [wip][video_player_avplay]Add setData and getData interface [video_player_avplay]Add setData and getData interface Mar 6, 2025
@xiaowei-guan xiaowei-guan marked this pull request as ready for review March 6, 2025 01:11
@xiaowei-guan xiaowei-guan requested review from JSUYA and gin7773 March 6, 2025 01:12
@JSUYA
Copy link
Member

JSUYA commented Mar 6, 2025

Before I get into review...
rapid json is an external library. If clang-format updated rapidjson should also be updated.
How about applying .clang-format-ignore?

@xiaowei-guan
Copy link
Contributor Author

.clang-format-ignore for rapidjson.> Before I get into review... rapid json is an external library. If clang-format updated rapidjson should also be updated. How about applying .clang-format-ignore?

I have tried to use .clang-format-ignore, but it doesn't work.
I viewed the tools_runner.sh, I found that it uses flutter_plugins_tools plugin to run clang-format command.
but it doesn't have ignore option. Have you used .clang-fromat-ignore before?

@xiaowei-guan
Copy link
Contributor Author

.clang-format-ignore for rapidjson.> Before I get into review... rapid json is an external library. If clang-format updated rapidjson should also be updated. How about applying .clang-format-ignore?

I have tried to use .clang-format-ignore, but it doesn't work. I viewed the tools_runner.sh, I found that it uses flutter_plugins_tools plugin to run clang-format command. but it doesn't have ignore option. Do you konw how to use .clang-fromat-ignore ?

@JSUYA
Copy link
Member

JSUYA commented Mar 6, 2025

I have tried to use .clang-format-ignore, but it doesn't work. I viewed the tools_runner.sh, I found that it uses flutter_plugins_tools plugin to run clang-format command. but it doesn't have ignore option. Have you used .clang-fromat-ignore before?

Oh I'm sorry. I tested it wrong, I was under the mistaken think that it worked.
For now, please applying format. in the future i will check way to bypass clang-format.

Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

Should this API also be called in advance like setStreamingProperty?

Comment on lines 275 to 277
return <DashPlayerProperty, Object>{
for (final MapEntry<Object?, Object?> entry in msg.data.entries)
_dashPlayerPropertyReverseMap[entry.key]!: entry.value!,
Copy link
Member

Choose a reason for hiding this comment

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

You can get type from value. so you don't add reverse_map.

return <DashPlayerProperty, Object>{
      for (final MapEntry<Object?, Object?> entry in msg.data.entries)
        _dashPlayerPropertyMap.keys.firstWhere((DashPlayerProperty key) =>
            _dashPlayerPropertyMap[key] == entry.key!): entry.value!,
    };

+)
Can you provide me some code to test this feature?
I tested it like this in the example but I couldn't get any result.

Map<DashPlayerProperty, Object> data = {
      DashPlayerProperty.maxBandWidth: 15000,
      // DashPlayerProperty.mpeghMetadata: 'METADATA TEST',
    };
    _controller.setData(data);

    _controller.setLooping(true);
    _controller.initialize().then((_) => setState(() {
          final Set<DashPlayerProperty> keys = {
            DashPlayerProperty.maxBandWidth,
            // DashPlayerProperty.mpeghMetadata
          };
          _controller.getData(keys).then((onValue) => print(onValue));
        }));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done,

setData must call after initialize successful.
but it staill has issue with mpeghMetadata, I'm checking with MM team.

Copy link
Member

Choose a reason for hiding this comment

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

setData must call after initialize successful.

Could you please add this explanation to the comments or README?
+) As far as I know, setStreamingProperty should be called before initialization.
Please also include this content in the comments or README.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

Does this API only provide DashProperty? Are there any plans to provide other properties?

Copy link
Member

@JSUYA JSUYA left a comment

Choose a reason for hiding this comment

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

but it staill has issue with mpeghMetadata, I'm checking with MM team.

Once this is fixed, please verify the behavior and merge.

@xiaowei-guan
Copy link
Contributor Author

but it staill has issue with mpeghMetadata, I'm checking with MM team.

Once this is fixed, please verify the behavior and merge.

The MM team member said that mpeghMetadata doesn't open for 3rd-party, I have deleted it form DashPropertyType enum.

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

Successfully merging this pull request may close these issues.

2 participants