[video_player]: reduce video player position update interval from 500ms to 100ms

[abdelaziz-mahdy]

Description
reduce video player position update interval from 500ms to 100ms to fix the issue of fast captions not showing for their intended time

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#160855

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene 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/packages repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to 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, or this PR is exempt from CHANGELOG changes.
• 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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[stuartmorgan-g]

Per discussion linked from flutter/flutter#90080 (comment) I'm skeptical that this is API we want to add. If subtitles don't work reliably at the current update frequency, and higher update frequencies don't have noticeable performance impact, then we should simply increase the frequency. If it does have performance issues, then we should figure out a different way of triggering subtitle changes. Either way, I would expect that we should be able to resolve the issue without new public API and without requiring extra configuration.

[abdelaziz-mahdy]

I kept searching for the previous discussions but couldn't find them. Thank you for providing the link.

Would an update frequency of 100ms work? I think it would be very effective for subtitles and not frequent enough to cause a high load on devices.

if you agree, i can open a new pr with that change and add a test to make sure captions are included in the timer consideration

[abdelaziz-mahdy]

note: this is how i override it from the app side, i didnt like the approach this is why i opened this pr

  void startPositionUpdaterForcer() {
    _positionUpdaterForcerTimer?.cancel();
    _positionUpdaterForcerTimer =
        Timer.periodic(Duration(milliseconds: 100), (timer) {
      _positionUpdaterForcer();
    });
  }

  void stopPositionUpdaterForcer() {
    _positionUpdaterForcerTimer?.cancel();
  }
  /// These parts exists in the video_player package but slightly edited to be called from outside of the class
  Future<void> _positionUpdaterForcer() async {
    Duration? position = await controller.position;
    if (position == null) {
      return;
    }
    VideoPlayerValue value = controller.value;
    if (position > value.duration) {
      position = value.duration;
    }
    value = value.copyWith(
      position: position,
      caption: await _getCaptionAt(position),
      isCompleted: position == value.duration,
    );
  }

  Future<Caption> _getCaptionAt(Duration position) async {
    /// the internal [ClosedCaptionFile]
    ClosedCaptionFile? closedCaptionFile;
    closedCaptionFile = await controller.closedCaptionFile;
    if (closedCaptionFile == null) {
      return Caption.none;
    }

    VideoPlayerValue value = controller.value;
    final Duration delayedPosition = position + value.captionOffset;
    // TODO(johnsonmh): This would be more efficient as a binary search.
    for (final Caption caption in (closedCaptionFile).captions) {
      if (caption.start <= delayedPosition && caption.end >= delayedPosition) {
        return caption;
      }
    }

    return Caption.none;
  }

[abdelaziz-mahdy]

i updated the pr to be an update to value from 500 to 100 ms, and the test case. hope thats okay

[stuartmorgan-g]

Wouldn't this test as written pass without the rest of the PR?

[abdelaziz-mahdy]

just tested using 500 ms for the updates and it failed, but i will add another test related to the captions since its the more related to this pr

[abdelaziz-mahdy]

I added the other test and I hope its shows the problem more, let me know if there anything else I can fix.

Thank you.

[stuartmorgan-g]

LGTM with one change. @tarrinneal for second review.

[tarrinneal]

lgtm, with some clarity in the changelog.

[auto-submit]

autosubmit label was removed for flutter/packages/8346, because - The status or check suite Linux dart_unit_test_shard_1 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

[stuartmorgan-g]

This is hitting the known camerax flake; it's unrelated to the PR.