fix(video_player): buffering state events missing on Android & Web (fixes flutter/flutter#28494)

[kmod-midori]

Description

Currently the isBuffering field on Android does not chage even if the video is buffering, this PR fixed this by sending bufferingStart and bufferingEnd events in the event handler of ExoPlayer.

Related Issues

Fixes flutter/flutter#28494

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide). * No Java tests are available for this part *
• All existing and new tests are passing. * No Java tests are available for this part *
• I updated/added relevant documentation (doc comments with ///).
• The analyzer (flutter analyze) does not report any problems on my PR.
• I read and followed the Flutter Style Guide.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I signed the CLA.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
• No, this is not a breaking change.

[kmod-midori]

@iskakaushik @cyanglaz Is it possible to have a look at this?

Not a big deal, but quite annoying when the network condition is bad and we can't provide the users with correct message.

[obshksw]

Not sure if it is appropriate to add this here since this issue is specifically for Android, but it has the same problem for flutter web... It'd be great to have this fixed there also!

[kmod-midori]

Not sure if it is appropriate to add this here since this issue is specifically for Android, but it has the same problem for flutter web... It'd be great to have this fixed there also!

Just had a quick glance, the issue seems similar (events not triggered). However, I personally have no experience with directly controlling <video>s, so I am not sure if I could fix it :(

[obshksw]

@chengyuhui Yeah that's what it seemed like. Thanks anyway for taking the time to check it out! :)

[cyanglaz]

LGTM! Thanks for this. Could you also add tests for this? I think e2e test would work in these scenarios.
The version in the pubspec is not updated.

[p30arena]

LGTM
This is actually working 👍
I tested it in a production app.

[kmod-midori]

I was considering adding some tests, but have no idea how to do that, since it seems nothing in the native part has test associated with it, and this is a platform-specific bug.

[cyanglaz]

@chengyuhui You might be able to add something similar to https://github.com/flutter/plugins/blob/master/packages/video_player/video_player/example/test_driver/video_player_e2e.dart
Note that video player e2e tests were not working on iOS so you might want to skip iOS like the other tests.
Please let me know if you need additional helps. :)

[kmod-midori]

I got the idea, but still wondering how this could be done (controlling the underlying buffering status?). I originally discovered this bug entirely by accident (I am in China and the Apple HLS test stream is really slow, but the buffering indicator is not showing up).

The logic of applying changes to VideoPlayerValue based on events from native code is already tested.

[cyanglaz]

The logic of applying changes to VideoPlayerValue based on events from native code is already tested.

@chengyuhui Which tests did you mean? Could you link me to the code?

[kmod-midori]

@chengyuhui Which tests did you mean? Could you link me to the code?

It's here: https://github.com/flutter/plugins/blob/master/packages/video_player/video_player/test/video_player_test.dart#L392

[cyanglaz]

@chengyuhui This is only testing the dart apis with a mock controller. We might want to test the android implementation. Maybe an e2e test would help us here?

[kmod-midori]

The problem is to simulate a stream that requires buffering (ExoPlayer does not seem to have mocking ability). I see some 127.0.0.1 references in the test files, how does that work?

[veloosooo]

This is actually working

[Ehesp]

LGTM

[kmod-midori]

Is there someting I still need to do?

[Ehesp]

cc @amirh for review

[google-cla]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[cyanglaz]

@csells blasten is the expert on null-safety so I tagged him for confirmation. He just gave LGTM so I think it's good.