Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic support for subtitles and audio tracks #501

Closed
11 tasks done
defagos opened this issue Aug 2, 2023 · 6 comments · Fixed by #516
Closed
11 tasks done

Basic support for subtitles and audio tracks #501

defagos opened this issue Aug 2, 2023 · 6 comments · Fixed by #516
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@defagos
Copy link
Member

defagos commented Aug 2, 2023

As a user I want to be able to see subtitles and audio tracks available for some content, as well as to select / deselect a track.

Acceptance criteria

  • An API is provided to list available subtitles and audio tracks.
  • An API is provided to list the current subtitle and audio track selection.
  • An API is provided to select a track (or remove selection if this makes sense).
  • Forced subtitles must not treated in a special way (will be addressed in Forced subtitles support #502).
  • The behavior is correct when no subtitles or audio tracks are available.
  • Up to non-supported features results are consistent with the Apple system player user interface.

Hints

If this makes sense track management can be moved to a tracker management state object, which can be bound to a player like progress tracker or visibility tracker can. However this might not be justified here:

  • Progress tracker makes it possible to isolate frequent updates. Here it is likely that only a couple updates are added if track management is built in the player itself.
  • Visibility tracker was introduced to extract a UI-only behavior which makes no sense in the player itself.

Documentation

See official documentation.

Tasks

  • Add API to list available subtitles and audio tracks (forced subtitles not filtered).
  • Add API to get the current selection (forced subtitles not filtered).
  • Add API to change the current track selection (basic, no automatic mode).
  • Use MediaAccessibility where appropriate, most notably for interactive vs. non-interactive selection and, if not automatically taken into account, for retrieval.
  • Display menu in the demo.
  • Increase settings menu icon size.
  • Perform basic check with Play CH content.
  • Compare behavior between vanilla player UI and custom PB UI and ensure all scenarios match.
  • Check behavior over AirPlay (e.g. correct initial detection).
  • Provide menu components in the Player library if meaningful.
  • Attempt to build a test stream with AUTOSELECT set to YES for only some subtitles, if possible. Write a dedicated unit test that verifies we are not only relying on MediaAccessibility. Shaka Packager manages this flag internally.
@defagos defagos added this to Pillarbox Aug 2, 2023
@defagos defagos converted this from a draft issue Aug 2, 2023
@defagos defagos added the enhancement New feature or request label Aug 2, 2023
@defagos defagos added this to the Tracks milestone Aug 2, 2023
@defagos defagos moved this from 📋 Backlog to 🚧 In Progress in Pillarbox Aug 2, 2023
@defagos defagos self-assigned this Aug 2, 2023
@defagos
Copy link
Member Author

defagos commented Aug 8, 2023

I introduced a MediaSelector type to implement the media selection logic. If there is later a need for more flexibility we could:

  1. Create a MediaSelector protocol with the required selection methods.
  2. Have the current StandardMediaSelector (renaming required) implement MediaSelector.
  3. Provide a way for developers to supply their own type conforming to MediaSelector during player configuration.

Currently MediaSelector is created again when the context (selection or available tracks) changes but maybe the above would require creation once (tracks available), then updates when the media selection changes. This way MediaSelector would have a defined lifecycle:

  • Initialization, where we can decide which selection needs to be applied when an item starts playback.
  • Later updates.

@defagos
Copy link
Member Author

defagos commented Aug 8, 2023

Interesting use case to write as a unit test:

  1. Play content supporting Japanese as subtitles (expect Japanese).
  2. Play other content which does not support Japanese (expect Off according to Apple player implementation).
  3. Play content in Japanese again. Japanese must still be the active subtitles (both displayed as well as returned from the API).

Interesting use cases also include:

  • SDH or AD preferences.
  • Current media selection publisher when the low-level player selection API is used.
  • Publisher behavior in playlist transitions.

@defagos
Copy link
Member Author

defagos commented Aug 8, 2023

The current implementation based on currentMediaSelection already correctly takes into account SDH and AD flags.

@defagos
Copy link
Member Author

defagos commented Aug 9, 2023

The current media selection is incorrectly reported over AirPlay, see #511.

@defagos
Copy link
Member Author

defagos commented Aug 9, 2023

I discovered empirically that there is no need to select a legible option on AVPlayerItem. Using MediaAccessibility API automatically ensures that this happens consistently. I could therefore avoid such calls in AudibleSelectionGroup selection implementation.

Wrong. This works but only for subtitles marked with AUTOSELECT, as can be seen with Play CH content.

@defagos
Copy link
Member Author

defagos commented Aug 9, 2023

I implemented behavior similar to AVPlayerViewController. Unlike SRG Media Player we get all standard behaviors for free without any dedicated code which is rather convenient:

  • Though it can be possible disabling audio (if present) is not an option. The audio option selected on startup is also automatically based on the device language.
  • Subtitle selection relies on MediaAccessibility.

As said above we could allow custom MediaSelector implementations should the need to have different behaviors be needed.

@defagos defagos linked a pull request Aug 10, 2023 that will close this issue
5 tasks
@defagos defagos moved this from 🚧 In Progress to 🍿 Code Review in Pillarbox Aug 11, 2023
@github-project-automation github-project-automation bot moved this from 🍿 Code Review to ✅ Done in Pillarbox Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant