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

Quality selection UI for audio-only content #2071

Closed
joeyparrish opened this issue Jul 31, 2019 · 7 comments · Fixed by #3649 or #4009
Closed

Quality selection UI for audio-only content #2071

joeyparrish opened this issue Jul 31, 2019 · 7 comments · Fixed by #3649 or #4009
Assignees
Labels
component: UI The issue involves the Shaka Player UI flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@joeyparrish
Copy link
Member

Have you read the FAQ and checked for duplicate open issues?
Yes

Is your feature request related to a problem? Please describe.
In the default UI controls, audio-only content gets a language menu, but not a quality menu (equivalent to the resolutions menu for video content). Although the variant tracks API will allow the choice of quality level for audio-only, the default UI doesn't expose this.

Describe the solution you'd like
We should add a new overflow menu item for audio-only content, to allow the selection of a specific bitrate.

Describe alternatives you've considered
We could always leave it up to audio-only apps to register their own button for this, but we could easily make the UI more usable for audio-only content.

Additional context
Idea based on feedback from @vickymin13. Thanks!

@joeyparrish joeyparrish added the type: enhancement New feature or request label Jul 31, 2019
@theodab theodab added the component: UI The issue involves the Shaka Player UI label Jul 31, 2019
@shaka-bot shaka-bot added this to the Backlog milestone Jul 31, 2019
@joeyparrish joeyparrish modified the milestones: Backlog, Backlog 2 Jan 28, 2020
@mbvenu
Copy link

mbvenu commented Apr 2, 2021

Hey @joeyparrish any update here in 2021 ?

@michellezhuogg michellezhuogg added the flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this label Apr 2, 2021
@michellezhuogg
Copy link
Contributor

Hello @mbvenu, we haven't got time to add that yet. I just marked it as "contributions welcome" to see if anyone is interested to work on it :) Thank you!

@ismena
Copy link
Contributor

ismena commented Apr 5, 2021

This might also be a good gsoc issue if anyone's interested.

@ismena ismena added the gsoc label Apr 5, 2021
nbcl added a commit to nbcl/shaka-player that referenced this issue Apr 23, 2021
For audio-only content, in the UI, the resolution button becomes a quality button where users can select their desired audioBandwidth.

Issue: shaka-project#2071
@nbcl
Copy link
Contributor

nbcl commented Apr 23, 2021

Hello, everyone!

I approached the issue by introducing the quality selection to the currently existing resolution button in the UI, conditionally, when audio-only content is being displayed.

tracks = tracks.filter((track, idx) => {
  // If audio-only content is being displayed
  if (this.player.isAudioOnly()) {
    // Keep the first one with the same audioBandwidth.
    otherIdx = tracks.findIndex(
        (t) => t.audioBandwidth == track.audioBandwidth);
  // If video content is being displayed
  } else {
    // Keep the first one with the same height.
    otherIdx = tracks.findIndex(
        (t) => t.height == track.height);
  }
  return otherIdx == idx;
});

This replaces the resolution menu with an audio quality menu when audio-only content is displayed, but I have a couple of questions.

Are there any downsides to this design or should I reiterate by directly creating a new button that should only handle audio quality?

Also, is using the .audioBandwith attribute the correct approach to obtaining the bits/s, or am I mixing concepts together here?

I left my approach on a draft at my personal fork @draft:feat-2071 as a working proof of concept! 😄

@joeyparrish
Copy link
Member Author

You should use bandwidth instead of audioBandwidth. HLS doesn't give us separate audio and video bandwidth numbers, so audioBandwidth may not be present in all cases. (DASH content is more explicit.) For audio-only content, bandwidth and audioBandwidth should be the same anyway.

I haven't reviewed your draft in detail, but in sounds reasonable. Thanks!

@nbcl
Copy link
Contributor

nbcl commented Apr 23, 2021

Thank you for your review and comments on the draft!

For audio-only content, bandwidth and audioBandwidth should be the same anyway.

This was so obvious now that I think about it, if no video is being played, of course there is no videoBandwith, then bandwidth and audioBandwidth should be exactly the same, and clearly, bandwidth is preferred.

Anyways, I implemented the changes - Reiterated to bandwidth and fixed the declaration problem with a conditional operator.

Updated the draft @draft:feat-2071!

@joeyparrish
Copy link
Member Author

Great! Please take the PR out of draft and let us know when it's ready for a more thorough review. @ismena should take a look for UI, or I can take a look if she's busy.

@joeyparrish joeyparrish added priority: P3 Useful but not urgent and removed gsoc labels Sep 27, 2021
joeyparrish pushed a commit that referenced this issue Apr 21, 2022
Replaces resolution menu with audio quality menu when content is audio-only.

Fixes: #2071
@avelad avelad modified the milestones: Backlog, v4.0 May 4, 2022
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jun 20, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: UI The issue involves the Shaka Player UI flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
8 participants