Skip to content

[video_player_avfoundation] Add video track selection support#11476

Open
nateshmbhat wants to merge 1 commit intoflutter:mainfrom
nateshmbhat:breakout/video-track-avfoundation
Open

[video_player_avfoundation] Add video track selection support#11476
nateshmbhat wants to merge 1 commit intoflutter:mainfrom
nateshmbhat:breakout/video-track-avfoundation

Conversation

@nateshmbhat
Copy link
Copy Markdown
Contributor

Summary

AVFoundation breakout PR for #10688.

  • Implements getVideoTracks() and selectVideoTrack() methods using AVFoundation
  • Video track selection requires iOS 15+ / macOS 12+ for HLS streams
  • Adds comprehensive Swift and Dart unit tests

Dependency Chain

This PR is third in a series of breakout PRs:

  1. video_player_platform_interface ([video_player_platform_interface] Add video track selection support #11474) - pending
  2. video_player_android ([video_player_android] Add video track selection support #11475) - pending
  3. video_player_avfoundation (this PR)
  4. 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

  • Swift unit tests for video track retrieval and selection in VideoPlayerTests.swift
  • Dart unit tests for AVFoundationVideoPlayer overrides

Implements getVideoTracks() and selectVideoTrack() methods for video
track (quality) selection using AVFoundation.
Video track selection requires iOS 15+ / macOS 12+ for HLS streams.

AVFoundation breakout PR for flutter#10688.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces video track selection functionality to the AVFoundation video player plugin, specifically for HLS streams on iOS 15+ and macOS 12+. It adds new native methods to retrieve available video tracks and to select a preferred bitrate. The Dart side integrates these native methods, converting native data structures into VideoTrack objects and providing an API for track selection. New tests have been added to cover this functionality, and the plugin version has been updated to 2.10.0. Feedback suggests improving error handling in getVideoTracks for failed asset variant loading and populating assetTracks for non-HLS videos to ensure consistent track information and avoid dead code on the Dart side.

Comment on lines +475 to +478
AVKeyValueStatus status =
[urlAsset statusOfValueForKey:kFVPAssetVariantsKey error:&error];

if (status == AVKeyValueStatusLoaded) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The implementation of getVideoTracks does not handle the case where loading the asset variants fails. If status is AVKeyValueStatusFailed, the error parameter of statusOfValueForKey:error: will be populated, but it is currently ignored, and the method proceeds to return an empty result. It would be better to check for failure and return a FlutterError to provide better diagnostics to the Dart side.

}

FVPNativeVideoTrackData *result =
[FVPNativeVideoTrackData makeWithAssetTracks:nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The assetTracks parameter is explicitly set to nil here, which means that for non-HLS videos (or HLS videos where variants aren't used), no video track information will be returned. However, the Pigeon definition and the Dart implementation in AVFoundationVideoPlayer.dart (lines 250-271) include logic to handle assetTracks. This results in dead code on the Dart side. Consider populating assetTracks using [urlAsset tracksWithMediaType:AVMediaTypeVideo] to provide consistent track information for all video types, even if they don't support quality selection.

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.

1 participant