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

local API: Support multiple audio tracks #3563

Merged
merged 3 commits into from
May 20, 2023

Conversation

absidue
Copy link
Member

@absidue absidue commented May 17, 2023

local API: Support multiple audio tracks

Pull Request Type

  • Feature Implementation

Related issue

closes #2419

iv-org/invidious#3620 (WIP Invidious PR to support multiple audio tracks)

LuanRT/YouTube.js#308, LuanRT/YouTube.js#338, LuanRT/YouTube.js#366, LuanRT/YouTube.js#402 (relevant YouTube.js PRs)

Description

Support for audio track switching is finally here 🥳!
Disclaimer: local API only 🙈

The DASH side of things is handled by videojs-http-streaming, through the information in the DASH manifest (related YouTube.js PRs linked above, if you are interested in what I did over there). As we handle the audio formats, we also need to handle the multiple audio tracks ourselves, so that's what the majority of the code in this pull request is for, as the legacy formats don't have multiple audio tracks, we don't have to do anything about them.

Unfortunately we can't support them for Invidious yet, as Invidious doesn't support them itself yet, however they do have a work in progress pull request to add support (linked above). This pull request is written in a way to keep the Invidious stuff unaffected by the changes.

Screenshots

Audio formats:
audio

DASH formats:
dash

Testing

For the local API test that it works and for Invidious check that the behaviour is still the same as it was before.
Test both DASH and audio formats to check that it works and the legacy ones to check that they didn't break.

same language, with original, dubbed and audio descriptions:
https://youtu.be/Kn56bMZ9OE8

mutliple languages:
https://youtu.be/WTOm65IZneg

normal video:
https://youtu.be/Xzhp1HTt43M

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: d98a66c

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label May 17, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) May 17, 2023 18:05
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Minor

src/renderer/components/ft-video-player/ft-video-player.js Outdated Show resolved Hide resolved
Copy link
Member

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc left a comment

Choose a reason for hiding this comment

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

Please move the audio switch icon to the left of the caption. The icons in the player a grouped a bit. On the right side there is everything related to the window so fullscreen, fullwindow, theater and pip (dont look at the qualities one thats an exception -.- ). So it makes sense to move it beside the captions

@efb4f5ff-1298-471a-8973-3d47447115dc efb4f5ff-1298-471a-8973-3d47447115dc added PR: changes requested and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels May 18, 2023
Co-authored-by: PikachuEXE <[email protected]>
@efb4f5ff-1298-471a-8973-3d47447115dc

Some questions:

When u change format it will reset to original language why is that? I mean if i set my video to hindi and for some reason i want to switch to audio now i have to switch it to hindi again.

VirtualBoxVM_xrMZvlCaro.mp4

Why is the sorting on our side different then YT?

firefox_NRCigLKEDY.mp4

@absidue
Copy link
Member Author

absidue commented May 18, 2023

The current format switching implementation completely destroys the player and recreates it with the new formats, this makes it a lot easier to have things like the default quality selection working and using the right quality selector (the DASH formats use a different one than the audio and legacy ones, as switching qualities with DASH works completely differently in video.js than it does with single file formats). However that means that we have to explicitly back up the settings we want to copy over from the previous player and restore them on the new player instance.

Currently the only player information we backup and reapply for format switches, is the playback position.

The volume and muted state get carried over because they are stored in the current window so that they are the same across videos. The subtitle settings e.g. font size, also get reapplied as they are stored with the other app settings in the settings database).

@absidue
Copy link
Member Author

absidue commented May 18, 2023

As for the ordering, we don't do any sorting, we just use them in the order they are in the response from YouTube. Don't think it really matters, as YouTube's sorting doesn't seem to make sense either (it's not alphabetically or anything like that).

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

@PikachuEXE
Copy link
Collaborator

PikachuEXE commented May 19, 2023

Got this when

Can reproduce reliably (with steps above)
image

@efb4f5ff-1298-471a-8973-3d47447115dc
Copy link
Member

efb4f5ff-1298-471a-8973-3d47447115dc commented May 19, 2023

Isnt that the same as #3532

@absidue
Copy link
Member Author

absidue commented May 19, 2023

Yes, that's an issue that existed before this PR.

@absidue absidue added PR: waiting for review For PRs that are complete, tested, and ready for review and removed PR: changes requested labels May 20, 2023
@FreeTubeBot FreeTubeBot merged commit d34cf0d into FreeTubeApp:development May 20, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label May 20, 2023
@absidue absidue deleted the local-audio-tracks branch May 20, 2023 12:57
@Ein-Tim
Copy link

Ein-Tim commented Aug 6, 2023

When will this feature be actually in the app?

@absidue
Copy link
Member Author

absidue commented Aug 6, 2023

@Ein-Tim it already is, if you switch to the nightly builds, you can use it.

https://docs.freetubeapp.io/development/nightly-builds

@hboetes
Copy link
Contributor

hboetes commented Aug 19, 2023

Interesting case: With this video https://youtu.be/S4L_0CDsd1I which definitely also has an English audio track, I only get the French track whilst using a French invidious mirror.

@absidue
Copy link
Member Author

absidue commented Aug 19, 2023

@hboetes As mentioned in the pull request title and description they are only supported on the local API, Invidious doesn't give us enough information yet.

@hboetes
Copy link
Contributor

hboetes commented Aug 19, 2023

@hboetes As mentioned in the pull request title and description they are only supported on the local API, Invidious doesn't give us enough information yet.

@absidue , I am using the local API.

@absidue
Copy link
Member Author

absidue commented Aug 19, 2023

You just said you are using a french Invidious mirror though?

@hboetes
Copy link
Contributor

hboetes commented Aug 19, 2023

I was, but that's because whenever I clear the invidious mirror, after the next time FT starts up another invidious mirror is used. Is it even possible to disable invidious?
https://0x0.st/HLJJ.png

@absidue
Copy link
Member Author

absidue commented Aug 19, 2023

When the local API is set to preferred Invidious is only used if something goes wrong with the local API and "Revert to non-preferred backend on failure" is turned on.

You are presumably using 0.18.0 which was released in November 2022, this pull request was merged in May this year.

@hboetes
Copy link
Contributor

hboetes commented Aug 19, 2023

Nope, downloaded a nightly yesterday.

%  rpm -q freetube
freetube-0.18.0~nightly~3260-1.x86_64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Choose audio track
7 participants