[video_player_android] Add video track selection support by nateshmbhat · Pull Request #11475 · flutter/packages

nateshmbhat · 2026-04-09T07:40:23Z

Summary

Android breakout PR for #10688.

Implements getVideoTracks() and selectVideoTrack() methods using ExoPlayer's TrackSelectionOverride
Adds onVideoTrackChanged event callback for track change notifications
Adds comprehensive Java and Dart unit tests

Dependency Chain

This PR is second in a series of breakout PRs:

video_player_platform_interface ([video_player_platform_interface] Add video track selection support #11474) - pending
video_player_android (this PR)
video_player_avfoundation (pending)
video_player + video_player_web (pending - original PR [video_player] : Add video track selection support for Android and iOS #10688 updated)

Note: This PR depends on video_player_platform_interface 6.7.0 being published first.

Test Plan

Java unit tests for getVideoTracks() and selectVideoTrack() in VideoPlayerTest.java
Java unit tests for onVideoTrackChanged callback in VideoPlayerEventCallbacksTest.java
Dart unit tests for AndroidVideoPlayer overrides

Implements getVideoTracks() and selectVideoTrack() methods for video track (quality) selection using ExoPlayer. Android breakout PR for flutter#10688.

gemini-code-assist

Code Review

This pull request introduces video track selection capabilities to the Android video player plugin. Key changes include adding getVideoTracks(), selectVideoTrack(), and enableAutoVideoQuality() methods to the VideoPlayerInstanceApi and their implementations in VideoPlayer.java and android_video_player.dart. A workaround is implemented in selectVideoTrack to handle video dimension changes by temporarily disabling and re-enabling the video track type to force a renderer reset. New Pigeon messages and data structures are introduced to support video track information exchange, and ExoPlayerEventListener is updated to notify about video track changes. Comprehensive unit tests for the new video track functionalities have also been added. The review comment points out a potential crash risk in the postDelayed workaround within VideoPlayer.java if the player is disposed during the 150ms delay, as the current trackSelector == null guard is ineffective. It recommends using a cancellable callback mechanism or an isDisposed flag for robust handling.

gemini-code-assist · 2026-04-09T07:47:23Z

+      new android.os.Handler(android.os.Looper.getMainLooper())
+          .postDelayed(
+              () -> {
+                // Guard against player disposal during the delay
+                if (trackSelector == null) {
+                  return;
+                }
+
+                trackSelector.setParameters(
+                    trackSelector
+                        .buildUponParameters()
+                        .setTrackTypeDisabled(C.TRACK_TYPE_VIDEO, false)
+                        .setOverrideForType(override)
+                        .build());
+
+                // Restore playback state
+                exoPlayer.seekTo(currentPosition);
+                if (wasPlaying) {
+                  exoPlayer.play();
+                }
+              },
+              150);


The postDelayed workaround for dimension changes introduces a potential crash risk. If the VideoPlayer is disposed (and the ExoPlayer released) during the 150ms delay, the calls to exoPlayer.seekTo() and exoPlayer.play() inside the delayed callback will throw an IllegalStateException because the player has been released.

The current guard if (trackSelector == null) (line 414) is likely ineffective because trackSelector is a final field in this class and is not nullified during the dispose() call in the current implementation of this plugin.

Recommendation:
Use a mechanism to cancel the pending callback or a robust way to check if the player has been disposed. For example, you could use a member Handler and call handler.removeCallbacksAndMessages(null) in dispose(), or maintain an isDisposed boolean flag that is set to true in dispose() and checked at the beginning of the delayed callback.

Please address this.

Addressed in 7bdc032. Switched to a member Handler plus an isDisposed flag; dispose() now calls removeCallbacksAndMessages(null) and sets the flag before releasing the player, so the 150 ms dimension-change callback cannot run on a released ExoPlayer. Added a Robolectric regression test (testSelectVideoTrack_disposeDuringDimensionChangeDelayDoesNotCrash) that uses ShadowLooper.idleFor(200ms) after dispose() and asserts no seekTo/play is invoked.

stuartmorgan-g · 2026-04-14T13:50:19Z

This PR requires #11474 to land and be published.

nateshmbhat · 2026-05-01T06:46:58Z

@stuartmorgan-g PR unblocked. good to merge ?

gemini-code-assist

Code Review

This pull request implements video track selection for the Android video player using ExoPlayer, including methods to retrieve available tracks, select a specific track, and enable auto-quality selection. The implementation includes a workaround for renderer resets when video dimensions change and updates the Pigeon-generated messaging layer. Feedback is provided regarding potential race conditions in the Dart selectVideoTrack implementation where concurrent calls could overwrite shared state, and concerns are raised about state inconsistency and potential crashes during the 150ms delay used for dimension-change renderer resets.

gemini-code-assist · 2026-05-01T06:47:35Z

+  Future<void> selectVideoTrack(VideoTrack? track) async {
+    // Create a completer to wait for the track selection to complete
+    _videoTrackSelectionCompleter = Completer<void>();
+
+    if (track == null) {
+      // Auto quality - use dedicated method
+      _expectedVideoTrackId = null;
+      try {
+        await _api.enableAutoVideoQuality();
+
+        // Wait for the onTracksChanged event from ExoPlayer with a timeout
+        await _videoTrackSelectionCompleter!.future.timeout(
+          const Duration(seconds: 5),
+          onTimeout: () {
+            // If we timeout, just continue - the track may still have been selected
+          },
+        );
+      } finally {
+        _videoTrackSelectionCompleter = null;
+        _expectedVideoTrackId = null;
+      }
+      return;
+    }
+
+    // Extract groupIndex and trackIndex from the track id
+    final List<String> parts = track.id.split('_');
+    if (parts.length != 2) {
+      throw ArgumentError(
+        'Invalid track id format: "${track.id}". Expected format: "groupIndex_trackIndex"',
+      );
+    }
+
+    final int groupIndex = int.parse(parts[0]);
+    final int trackIndex = int.parse(parts[1]);
+
+    _expectedVideoTrackId = track.id;
+
+    try {
+      await _api.selectVideoTrack(groupIndex, trackIndex);
+
+      // Wait for the onTracksChanged event from ExoPlayer with a timeout
+      await _videoTrackSelectionCompleter!.future.timeout(
+        const Duration(seconds: 5),
+        onTimeout: () {
+          // If we timeout, just continue - the track may still have been selected
+        },
+      );
+    } finally {
+      _videoTrackSelectionCompleter = null;
+      _expectedVideoTrackId = null;
+    }
+  }


The selectVideoTrack implementation is vulnerable to race conditions when multiple calls occur concurrently. Since _videoTrackSelectionCompleter and _expectedVideoTrackId are shared fields, a second call will overwrite the state of an ongoing first call. Additionally, the finally block of the first call to complete will clear these fields, which can cause subsequent calls to timeout or resolve prematurely with incorrect state. Using local variables to guard the cleanup ensures that each call only clears the fields if it is still the 'current' active request.

Future<void> selectVideoTrack(VideoTrack? track) async { final Completer<void> completer = Completer<void>(); _videoTrackSelectionCompleter = completer; final String? expectedId = track?.id; _expectedVideoTrackId = expectedId; try { if (track == null) { // Auto quality - use dedicated method await _api.enableAutoVideoQuality(); } else { // Extract groupIndex and trackIndex from the track id final List<String> parts = track.id.split('_'); if (parts.length != 2) { throw ArgumentError( 'Invalid track id format: "${track.id}". Expected format: "groupIndex_trackIndex"', ); } final int groupIndex = int.parse(parts[0]); final int trackIndex = int.parse(parts[1]); await _api.selectVideoTrack(groupIndex, trackIndex); } // Wait for the onTracksChanged event from ExoPlayer with a timeout await completer.future.timeout( const Duration(seconds: 5), onTimeout: () { // If we timeout, just continue - the track may still have been selected }, ); } finally { if (_videoTrackSelectionCompleter == completer) { _videoTrackSelectionCompleter = null; } if (_expectedVideoTrackId == expectedId) { _expectedVideoTrackId = null; } } }

Addressed in 7bdc032. Captured the completer and expected id in locals (completer, expectedId) and guarded the finally cleanup with identical(_videoTrackSelectionCompleter, completer) / _expectedVideoTrackId == expectedId, so an older overlapping selectVideoTrack call can no longer null out a newer call's state. Also merged the null/non-null track branches to remove the duplicated wait/cleanup logic. Added a regression test (concurrent selectVideoTrack calls do not clobber each other) that fails fast on call 1 and verifies call 2 still completes on its matching VideoTrackChangedEvent.

stuartmorgan-g · 2026-05-01T13:34:53Z

@stuartmorgan-g PR unblocked. good to merge ?

No PRs can me merged without review. This needs final re-review by the folks who looked at the original combination PR.

…deoTrack race - VideoPlayer.java: replace throwaway Handler + ineffective trackSelector null-check with a member mainHandler and isDisposed flag; dispose() now cancels the 150ms dimension-change callback so it cannot run on a released ExoPlayer. - android_video_player.dart: capture completer/expectedId in locals and guard cleanup with identical()/== so an older concurrent selectVideoTrack call no longer clears a newer call's state. - Add Java regression test (Robolectric ShadowLooper) for dispose during the dimension-change delay, and a Dart concurrent-call regression test.

tarrinneal · 2026-05-19T23:40:28Z

+        final int groupIndex = int.parse(parts[0]);
+        final int trackIndex = int.parse(parts[1]);


Is it possible these could end up not being ints? int.parse will crash if so.

No not possible

Updated this path to parse Android track IDs with int.tryParse and throw ArgumentError if the numeric contract is violated, rather than letting a FormatException bubble out. Added a regression test for non-numeric track IDs as well.

tarrinneal · 2026-05-19T23:47:59Z

+      await completer.future.timeout(
+        const Duration(seconds: 5),
+        onTimeout: () {
+          // If we timeout, just continue - the track may still have been selected


might be worth logging something for debugging purposes.

Added debug logging on the 5-second timeout path so we emit whether the stalled selection was for a specific track or auto quality selection. There is also a test that advances the timeout and asserts that the log is produced.

tarrinneal · 2026-05-20T00:27:43Z

+        // Find the selected track within this group
+        for (int i = 0; i < group.length; i++) {
+          if (group.isTrackSelected(i)) {
+            return groupIndex + "_" + i;


since this is mirrored on the dart side, we should probably add a comment on both to ensure that if this changes it's changed in both locations

Added sync comments on both sides. ExoPlayerEventListener.findSelectedAudioTrackId and findSelectedVideoTrackId now point at android_video_player.dart::_parseAndroidTrackId, and the Dart parser notes that the groupIndex_trackIndex contract is shared with the Java emitters.

tarrinneal · 2026-05-20T00:44:22Z

+    boolean dimensionsChanged =
+        currentFormat != null
+            && (currentFormat.width != newFormat.width || currentFormat.height != newFormat.height);


If currentFormat evaluates to null because the player hasn't fully negotiated or initialized the stream format yet, dimensionsChanged will resolve to false. This bypasses this workaround. if current == null and new != null should we consider that "dimensionsChanged"?

Updated the dimension-change check so currentFormat == null also takes the renderer-reset workaround path. That keeps the safe behavior when the current stream format has not been negotiated yet, and there is now a Robolectric regression test covering that branch.

[video_player_android] Add video track selection support

12597e9

Implements getVideoTracks() and selectVideoTrack() methods for video track (quality) selection using ExoPlayer. Android breakout PR for flutter#10688.

github-actions Bot added platform-android p: video_player labels Apr 9, 2026

nateshmbhat mentioned this pull request Apr 9, 2026

[video_player_avfoundation] Add video track selection support #11476

Open

2 tasks

gemini-code-assist Bot reviewed Apr 9, 2026

View reviewed changes

stuartmorgan-g marked this pull request as draft April 14, 2026 13:50

nateshmbhat force-pushed the breakout/video-track-android branch 2 times, most recently from 5ebe09b to 12597e9 Compare May 1, 2026 06:45

Merge branch 'main' into breakout/video-track-android

11c473b

nateshmbhat marked this pull request as ready for review May 1, 2026 06:45

gemini-code-assist Bot reviewed May 1, 2026

View reviewed changes

stuartmorgan-g requested review from camsim99 and tarrinneal May 1, 2026 13:34

stuartmorgan-g added triage-android Should be looked at in Android triage federated: partial_changes PR that contains changes for only a single package of a federated plugin change labels May 1, 2026

Merge branch 'main' into breakout/video-track-android

82fbad7

jesswrd requested review from mboetger and removed request for camsim99 May 5, 2026 21:18

nateshmbhat added 2 commits May 16, 2026 15:23

Merge branch 'main' into breakout/video-track-android

7e0d4db

tarrinneal reviewed May 20, 2026

View reviewed changes

nateshmbhat added 2 commits May 23, 2026 09:20

[video_player_android] Address remaining PR review comments

5abd49f

Merge branch 'main' into breakout/video-track-android

074c812

nateshmbhat requested a review from tarrinneal May 23, 2026 04:05

		final int groupIndex = int.parse(parts[0]);
		final int trackIndex = int.parse(parts[1]);

Conversation

nateshmbhat commented Apr 9, 2026

Summary

Dependency Chain

Test Plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Apr 14, 2026

Uh oh!

nateshmbhat commented May 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stuartmorgan-g commented May 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants