packages icon indicating copy to clipboard operation
packages copied to clipboard

[video_player] Adds audio track metadata fetching and audio track selection feature

Open nateshmbhat opened this issue 4 months ago • 22 comments

Description

This PR adds comprehensive audio track retrieval and selection support to the video_player package, enabling developers to access detailed information about available audio tracks and switch between them during playback.

Breakout PRs:

  1. Platform interface : #10171
  2. Andrioid : #10312
  3. ios : #10313

Changes Made

Core Features

  • Added VideoAudioTrack model with comprehensive metadata fields: id, label, language, isSelected, bitrate, sampleRate, channelCount, codec
  • Added getAudioTracks() method to retrieve all available audio tracks with real metadata
  • Added selectAudioTrack() method to switch between audio tracks during playback
  • Updated VideoPlayerController to expose audio track functionality

Platform Implementations

  • Android:
    • Real metadata extraction using ExoPlayer's getCurrentTracks() API
    • Robust track selection using TrackSelectionOverride with proper error handling
    • Support for multiple audio formats (AAC, AC3, EAC3, MP3, etc.)
  • iOS:
    • Metadata extraction from AVFoundation using AVAssetTrack for regular videos
    • HLS stream support using AVMediaSelectionGroup for adaptive streams
    • Proper track selection for both asset tracks and media selection options

Technical Infrastructure

  • Updated Pigeon interfaces for both Android and iOS with new data structures:
    • AudioTrackMessage, ExoPlayerAudioTrackData, AssetAudioTrackData, MediaSelectionAudioTrackData, NativeAudioTrackData
  • Enhanced platform interface with new methods and data classes
  • Has native unit tests for both Android and iOS platforms
  • Created demo screen showcasing audio track functionality with interactive UI

Demo Features

  • Interactive video player with audio track selection
  • Real-time metadata display (bitrate, sample rate, channels, codec)
  • Support for multiple video sources including HLS streams
  • Visual indicators for currently selected tracks

Related Issues

#59437 - Add audio track metadata support to video_player

Testing

  • Added native unit tests for both Android and iOS
  • Tested with various video formats and HLS streams
  • Verified backward compatibility with existing functionality
  • Demo screen widget for manual testing and validation

Breaking Changes

None - all changes are additive and backward compatible.

Pre-Review Checklist

  • [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • [x] I read the [Tree Hygiene] page, which explains my responsibilities.
  • [x] I read and followed the [relevant style guides] and ran [the auto-formatter].
  • [x] I signed the [CLA].
  • [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [video_player]
  • [x] I [linked to at least one issue that this PR fixes] in the description above.
  • [x] I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or I have commented below to indicate which [version change exemption] this PR falls under[^1].
  • [x] I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style], or I have commented below to indicate which [CHANGELOG exemption] this PR falls under[^1].
  • [x] I updated/added any relevant documentation (doc comments with ///).
  • [x] I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • [x] All existing and new tests are passing.

nateshmbhat avatar Aug 29 '25 08:08 nateshmbhat

I updated native andorid tests and ran the tests and verified that all native android tests are passing.

nateshmbhat avatar Aug 30 '25 12:08 nateshmbhat

@stuartmorgan-g @ash2moon can you please comment on this PR and lemme know if there are any concerns ?

nateshmbhat avatar Sep 01 '25 07:09 nateshmbhat

any update on this PR ?

nateshmbhat avatar Sep 09 '25 07:09 nateshmbhat

From triage: ping @tarrinneal @ash2moon @LongCatIsLooong @hellohuanlin for review

@nateshmbhat Apologies for the delay; things have been unusually busy recently. A couple of high-level notes while you are waiting for full reviews:

  • The video_player_web changes can all be reverted; it looks like you probably ran the auto-formatter in the package without running pub get, so applied the wrong formatter version.
  • It appears that one of the new tests is crashing on macOS.
  • If this feature isn't going to be implemented on all platforms, it needs to have a support query so clients know if they can call it.

stuartmorgan-g avatar Sep 09 '25 11:09 stuartmorgan-g

From triage: ping @tarrinneal @ash2moon @LongCatIsLooong @hellohuanlin for review

Pining @tarrinneal @ash2moon @LongCatIsLooong @hellohuanlin for review

nateshmbhat avatar Sep 12 '25 14:09 nateshmbhat

@LongCatIsLooong Have updated code based on review comments and added api support query check to return false in web as suggested by @stuartmorgan-g

nateshmbhat avatar Sep 20 '25 06:09 nateshmbhat

The new files here will require a minor change to the license header when you merge in main; you can re-copy from an existing file post-merge.

stuartmorgan-g avatar Sep 24 '25 14:09 stuartmorgan-g

The new files here will require a minor change to the license header when you merge in main; you can re-copy from an existing file post-merge.

i'm not understanding this, can u please elaborate ?

nateshmbhat avatar Sep 24 '25 15:09 nateshmbhat

i'm not understanding this, can u please elaborate ?

This line will fail the license check after your next merge from main, because the repo has been updated to longer include "All rights reserved". Once you merge, replace that line with a copy from any other existing (post-merge) file.

stuartmorgan-g avatar Sep 24 '25 15:09 stuartmorgan-g

This line will fail the license check after your next merge from main, because the repo has been updated to longer include "All rights reserved". Once you merge, replace that line with a copy from any other existing (post-merge) file.

Pushed the change. pls check @stuartmorgan-g

nateshmbhat avatar Sep 24 '25 16:09 nateshmbhat

any update on this PR ? what should i do next to get this merged ?

nateshmbhat avatar Sep 27 '25 06:09 nateshmbhat

Pushed the change. pls check @stuartmorgan-g

Our CI checks the license conformance.

any update on this PR ? what should i do next to get this merged ?

It still needs review and approval from @tarrinneal, @ash2moon, and @LongCatIsLooong

stuartmorgan-g avatar Sep 30 '25 15:09 stuartmorgan-g

@tarrinneal, @ash2moon, @LongCatIsLooong please help with the review

nateshmbhat avatar Oct 02 '25 08:10 nateshmbhat

@nateshmbhat Our triage process will follow up with people regularly; pinging everyone two days after I did reminder pings is not productive.

stuartmorgan-g avatar Oct 02 '25 13:10 stuartmorgan-g

@nateshmbhat Our triage process will follow up with people regularly; pinging everyone two days after I did reminder pings is not productive.

Got it, thanks for clarifying!

nateshmbhat avatar Oct 02 '25 14:10 nateshmbhat

A few nits and questions. I think a break off pr for the interface is good to go from here.

raised the break off pr for the interface here : https://github.com/flutter/packages/pull/10171

nateshmbhat avatar Oct 04 '25 07:10 nateshmbhat

Any update on this PR needed from my side?

nateshmbhat avatar Oct 27 '25 16:10 nateshmbhat

@nateshmbhat You can split out a new PR with the platform implementations (and with the overrides removed in favor of requiring the newly published interface package version), and then the rest of the platform implementation reviews can happen there.

stuartmorgan-g avatar Oct 27 '25 16:10 stuartmorgan-g

@nateshmbhat You can split out a new PR with the platform implementations (and with the overrides removed in favor of requiring the newly published interface package version), and then the rest of the platform implementation reviews can happen there.

@stuartmorgan-g Raised #10313 and #10312

nateshmbhat avatar Oct 28 '25 15:10 nateshmbhat

@LongCatIsLooong have updated the waiting for exoplayer to select the track approach (as per this comment https://github.com/flutter/packages/pull/9925#discussion_r2466556034 )

please check

nateshmbhat avatar Nov 03 '25 16:11 nateshmbhat

Have addressed all the major concerns in this PR , can we please have the platform PRs merged ? If any furthur changes are needed, pls let me know at the earliest so that i can quickly push the changes and we can take this forward sooner.

cc: @stuartmorgan-g @ash2moon @LongCatIsLooong

nateshmbhat avatar Nov 07 '25 16:11 nateshmbhat

Have addressed all the major concerns in this PR , can we please have the platform PRs merged ? If any furthur changes are needed, pls let me know at the earliest so that i can quickly push the changes and we can take this forward sooner.

I'm not sure why you are pinging people here. This PR can't land until the sub-PRs have landed, and the sub PRs are still under review. Those PRs will be landed when they have been reviewed and approved.

stuartmorgan-g avatar Nov 07 '25 17:11 stuartmorgan-g

@nateshmbhat I'll take a look at the Android changes again once you port them over from the split out PR. Thanks!

camsim99 avatar Dec 22 '25 17:12 camsim99