[video_player] Validate size only when assets contain video tracks on iOS

[AlexV525]

By adding a more specific type check, we can expand the support range of the video_player on iOS from videos to all supported media types. This is from the documentation of AVSimplePlayer: AVSimplePlayer/AVSPDocument.m#139

Resolves flutter/flutter#38480, flutter/flutter#90429, or more issues related to iOS audio playing.
Related to flutter/flutter#47105, flutter/flutter#75636.

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.

[flutter-dashboard]

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.

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.

[AlexV525]

Seems the duration for audios on iOS will always be 0:00:00.001000. Drafting for further investigations.

[AlexV525]

It's 5.041625 sec. The web takes 5.042 and natives take 5.041.

[AlexV525]

I've done some debuggin steps, and the root cause of this difference is from here:

plugins/packages/video_player/video_player_web/lib/video_player_web.dart

Line 301 in a10ac2e

milliseconds: (videoElement.duration * 1000).round(),

The videoElement.duration reports 5.041633 from the engine, and it seems to be the precision issue of the web platform. If we want to make them equal, we might use toInt() here.

[stuartmorgan-g]

I don't think this actually resolves flutter/flutter#47105 as claimed in the PR description; using video_player to play audio isn't actually a good idea, and will be even less so when if (most likely when) we migrate it to platform views.

I'm conflicted about this PR. The change itself is simple, but adding tests asserting that audio works sets a problematic precedent. What if a new platform we want to support has different APIs for supporting video and audio; are we now saying we require audio support in that implementation too? When there are inevitably new issues or feature requests that are specific to audio, will those all suddenly be valid?

I think where I come down on this is that because the mismatch with Android is currently a source of confusion, and the incremental cost is extremely low here, I'm inclined to take this PR, but:

• The official position should still be that we don't officially support audio
• We should still do Make it more clear that video_player doesn't support audio (even if it's accidentally supported in some platforms) flutter#75636, and explicitly clarify in the README that future versions and/or platforms may not support audio (I can do this in a follow-up)
• The test group being added here should have a comment saying that audio playback is tested to prevent accidental regression, but could be removed in the future if there's a technical reason to do so.

@jmagman Does that sound good to you?

[jmagman]

@jmagman Does that sound good to you?

SGTM

[AlexV525]

using video_player to play audio isn't actually a good idea

That's more like it used to be supported implicitly, but someday it becomes invalid. I'll update the above suggestions.

If we're targeting flutter/flutter#47015, it actually resolves 2/3, with basic audio playing. I agree with the support range part, then the PR is more like an enhancement (validate size only with video) plus a fix (with more reliable duration obtain).

[jmagman]

This LGTM with comment nit.