Dolby Atmos stream selection w/ user preference for Native player

[thecosmicskye]

Key Features

• User Preference: Toggle setting for Dolby Atmos in Native
  Player settings (Note: Dolby Atmos requires Jellyfin FFmpeg 7.1.1-5+)
• Selects the first EAC3 Atmos track if available (If available, TrueHD is the first track typically, so I want to make sure that one is avoided - cannot be played back through direct play on AVKit)

[thecosmicskye]

Hmm, I can’t add the enhancement label

[JPKribs]

Hmm, I can’t add the enhancement label

I got you 😆

[thecosmicskye]

Haven't tested to see if it breaks VLC player or tvOS, would appreciate testing there.

[holow29]

I'm having a little trouble understanding the point of ordering the codecs here. As it stands, if the preferred audio stream is in a codec that is in the AudioCodec array in transcodingProfile, it should pass without transcoding. This means that the only streams that would be transcoded are those that are not, in this instance, in the following AudioCodec = [flac, alac, eac3, ac3, aac]. To me, the only point here is determining what the default transcoding codec should be for streams that are otherwise not supported. For example, what does a dts stream get transcoded to? I think it would be the codec at AudioCodec[0] in transcoding profile - although this is not always true given the server messes with HLS requests and has its own logic that supercedes device profiles...frankly I am not sure that the ordering here isn't (at this point) almost completely muddled by what the server's HLS logic does.
I don't think there is really a risk of "upmixing" a lossy codec to a lossless one. It will just decompress the lossy stream and recompress that data. There isn't a point to doing so, but it doesn't really matter, and if you need to convert to another codec anyway, why not avoid a lossy -> lossy conversion? Then there is a point to doing so.
I don't understand why the position of eac3 would matter since if it is in that codec already, it should pass through if eac3 is in the AudioCodec array of the transcoding profile. If not, I don't see a point to converting an unsupported format to eac3.

[LePips]

Frankly, I'm confused by these changes. Alongside the comment in https://github.com/jellyfin/Swiftfin/pull/1562/files#r2140710210, the transcoding profiles should be respected regardless of order. If they aren't, I deem that a server issue. Is all of this work really just to get non-Atmos content transcoded as such? Can you please describe the motivation to do so?

Not your fault, but the work touching the video player is redone in #1203. It's been a long while since I've worked on it, but I will be looking to pick it back up for testing for the 1.4 release. I would have any work now that touches the video player wait until #1203 is done.

Lastly, I can blatantly tell this was generated with AI. Do not get me wrong - it's awesome and if it gets the job done well that's all I care about. What I take issue with is having the description and motivations written by AI.

Please restate the description and motivations clearly.

[thecosmicskye]

Frankly, I'm confused by these changes. Alongside the comment in https://github.com/jellyfin/Swiftfin/pull/1562/files#r2140710210, the transcoding profiles should be respected regardless of order. If they aren't, I deem that a server issue. Is all of this work really just to get non-Atmos content transcoded as such? Can you please describe the motivation to do so?

Not your fault, but the work touching the video player is redone in #1203. It's been a long while since I've worked on it, but I will be looking to pick it back up for testing for the 1.4 release. I would have any work now that touches the video player wait until #1203 is done.

Lastly, I can blatantly tell this was generated with AI. Do not get me wrong - it's awesome and if it gets the job done well that's all I care about. What I take issue with is having the description and motivations written by AI.

Please restate the description and motivations clearly.

Fair feedback. I did review all the text it wrote in the descriptions. So it's my words, just re-written by AI.

In my own words:

Description: This is nearly the minimal amount of code changes required to get Atmos to playback on iOS in most scenarios. There are a couple of changes I added that aren't 100% necessary, but should help ensure proper remuxing - I can take those out if you'd prefer.

Motivations: I want Atmos to play back, and I want to avoid transcoding an HEVC file into H264 in bandwidth limited scenarios - as I'm pretty sure that would disable Atmos playback (not 100% sure). As is, Atmos won't play back in most scenarios, such as when the Atmos track isn't the first track (for example, if there is a truehd track as first track), or when remuxing/transcoding is needed at all. The only way to get an Atmos track (without these changes) to play is if in the media file, it's set as the first/default track, and if it's in an mp4 container, and if you are not in a bandwidth limited scenario.

And thanks for reviewing this! Means a lot :)

[holow29]

It might be helpful to break this PR down into smaller chunks. I see three main chunks, but please correct me if I am wrong:

1. Ranking audio streams (& associated settings)
2. Modifying transcode device profile to prioritize certain codecs
3. Fix requesting other audio tracks with re-request from server & allow HLS to stream copy (?) for remuxing [VideoPlayerManager, ViewModel, and overlay changes]

If this is correct, I think the work done in point 3 feels the most relevant to me. That said, I'm not sure if this is something solved by #1203, but I know that is something that could use a fix/redo if not because it doesn't currently work.

I don't think the work for point 2 is necessary or useful given my initial comment because the server decides codec preference for many transcoding decisions; for example, unless something has changed, it will take the videoCodec array of the transcoding profile and move HEVC at the front if the serverside setting to transcode to HEVC is enabled. (Edit: I was wrong: the server does still do its own codec shifting in these arrays, but it doesn't prioritize, it deprioritizes for video codecs at least.) That said, the audio codec changes in the transcoding profile still don't do anything that we want, I think.

I might be misunderstanding, but the motivation for point 1 feels like a workaround. If your motivation would be to ensure that Atmos tracks play back if selected, even if they aren't the first track, the solution to that seems to fix that bug instead of making it the track that is automatically selected. I think you are attempting to give more options, and that seems nice, but without knowing about the other complicated logic that goes into audiostream selection both client and server-side, it might be difficult to do that. I can't think of a good example of a file with an atmos track that is not marked as default/the first track that would be a better 'fit' to be chosen automatically - please share one if you have one in mind because I am having trouble understanding that part of your motivation!

Edit: I also see you might have made some changes right before I commented, so some of this might no longer apply

[thecosmicskye]

It might be helpful to break this PR down into smaller chunks. I see three main chunks, but please correct me if I am wrong:

1. Ranking audio streams (& associated settings)
2. Modifying transcode device profile to prioritize certain codecs
3. Fix requesting other audio tracks with re-request from server & allow HLS to stream copy (?) for remuxing [VideoPlayerManager, ViewModel, and overlay changes]

Agreed. HLS stream copy wasn't needed, I removed. However, audio selection in HLS params is necessary for audio selection to work. This ties 1 and 3 together.

1 and 2 somewhat tied together as the player settings would impact both. I'll have to put the player settings in both pull requests.

I'll go ahead and separate these out.

I don't think the work for point 2 is necessary or useful given my initial comment because the server decides codec preference for many transcoding decisions; for example, unless something has changed, it will take the videoCodec array of the transcoding profile and move HEVC at the front if the serverside setting to transcode to HEVC is enabled.

Sadly, this is not the case :( I will triple check. Transcoding decisions rest primarily on the client from what I've seen (as long as server has those transcoding options selected). This change was absolutely needed to get movies transcoded to HEVC instead of H264 (this should be separated into its own pull request, which I can make). It's up to the device profile to set the preferred codecs.

I might be misunderstanding, but the motivation for point 1 feels like a workaround. If your motivation would be to ensure that Atmos tracks play back if selected, even if they aren't the first track, the solution to that seems to fix that bug instead of making it the track that is automatically selected. I think you are attempting to give more options, and that seems nice, but without knowing about the other complicated logic that goes into audiostream selection both client and server-side, it might be difficult to do that. I can't think of a good example of a file with an atmos track that is not marked as default/the first track that would be a better 'fit' to be chosen automatically - please share one if you have one in mind because I am having trouble understanding that part of your motivation!

Basically all my Atmos movies have TrueHD Atmos has the default track with EAC3 Atmos as backup. This is the standard in Atmos test files as well. Players shouldn't select a transcoded track when a direct play track is available (imo).

Edit: I also see you might have made some changes right before I commented, so some of this might no longer apply

Still applies :)

[thecosmicskye]

#1582 ready. A MUCH smaller change :)

[holow29]

Looks like I was mistaken about some of the server's behavior. Looking at the server's code, the codec shifting happens if the options aren't allowed (deprioritizing or removing them from the array); for some reason I thought it was the opposite - I'm not sure if that was once the case or not, but either way, good change there from you to prioritize HEVC.

[thecosmicskye]

Okay, I stripped this one down to just an Atmos preference and automatic selector for the native player. This also fixes the audio selection logic, since it only used to happen AFTER the URL was already built.

[JPKribs]

Just a first note, I think something happened with your localizations. It's showing changes for all of them. I ran into something similar today, you will likely have to rebase and make sure these aren't been changed.

[thecosmicskye]

Just a first note, I think something happened with your localizations. It's showing changes for all of them. I ran into something similar today, you will likely have to rebase and make sure these aren't been changed.

Wait, that's intentional - I added translations for all languages. I rebased today. Per the contribution guide: "new strings that are not part of an experimental feature must be localized"

[holow29]

I think splitting up the changes was a good idea. My main issue with this PR now is simply the overall motivation. It has to do with audio stream preferences, and that is unfortunately a complicated area given it touches on language preferences, media default flags, and other criteria like quality sometimes too. As I understand your motivation, you want the EAC3 track to be chosen because it can be DirectPlayed over something like TrueHD which cannot. To me, this change still feels like a workaround to a larger issue/discussion. For example: what if that EAC3 track is in a different language? Regardless, maybe some people would prefer the TrueHD track to be losslessly transcoded to FLAC for DirectStream instead. What if the default track is FLAC stereo or mono which can be DirectPlayed but there is also an EAC3 track?

I understand your earlier work in this PR was attempting to deal with some of these issues with preferences, but I think it merits a larger discussion and blueprint for how this should be handled in general because having this one-off setting for atmos seems to be catering to a very specific preference. I'm not sure what the Jellyfin maintainers stance is right now on audio track selection - I know at one point, it was generally that DirectPlay should be prioritized after language preferences. I think that basically looked like was cycling through the audio streams and seeing if they had language information attached and then narrowing the choice to those in the desired language - then looking for the track that could be DirectPlayed. However it wasn't this simple otherwise you might end up with a commentary track playing instead of the main title's audio.

I am probably a broken record at this point, but some of this is mediated by the server as well. I believe the media encoder files as well as this one have logic that might interfere with exactly what the client requests (or at least help steer the client?). I know the developer of the Tizen Jellyfin app wants to make audiostreams compatability more queryable by the client (some of that related work here: jellyfin/jellyfin#12689). That isn't to say that Swiftfin can't have its own logic, but I am simply trying to say that research into how the server decides audio track selection might be merited as well.

This is just my opinion, and I am not sure what the maintainers want. It might be worth opening a discussion thread for the broader topic of audio stream (default) selection.

[thecosmicskye]

I think splitting up the changes was a good idea. My main issue with this PR now is simply the overall motivation. It has to do with audio stream preferences, and that is unfortunately a complicated area given it touches on language preferences, media default flags, and other criteria like quality sometimes too. As I understand your motivation, you want the EAC3 track to be chosen because it can be DirectPlayed over something like TrueHD which cannot. To me, this change still feels like a workaround to a larger issue/discussion. For example: what if that EAC3 track is in a different language? Regardless, maybe some people would prefer the TrueHD track to be losslessly transcoded to FLAC for DirectStream instead. What if the default track is FLAC stereo or mono which can be DirectPlayed but there is also an EAC3 track?

Totally. (Previous changes had a prefer lossless option for those that would want TrueHD transcoded to FLAC instead).

As for user language, I agree, that's a problem, but currently the app doesn't support language selection, or even track selection at all in the native player.

There definitely needs to be an audio selector on the movie preview screen like in the standard Jellyfin app, but I chose a less invasive implementation.

because having this one-off setting for atmos seems to be catering to a very specific preference.

Agree. I also had a lossless preference selector before I started splitting up the PR :P But a full implementation would also have a language selector. I could also include an audio track language selector in preferences as well, but trying to make this a small PR. Hmm, though with what you brought up about serverside track selection, I'd need to sync that language selector up with the server it looks like. No clue what the API for that would be.

I am probably a broken record at this point, but some of this is mediated by the server as well. I believe the media encoder files as well as this one have logic that might interfere with exactly what the client requests (or at least help steer the client?). I know the developer of the Tizen Jellyfin app wants to make audiostreams compatability more queryable by the client (some of that related work here: jellyfin/jellyfin#12689). That isn't to say that Swiftfin can't have its own logic, but I am simply trying to say that research into how the server decides audio track selection might be merited as well.

This is just my opinion, and I am not sure what the maintainers want. It might be worth opening a discussion thread for the broader topic of audio stream (default) selection.

Oh this is great info. I will look into figuring out how to respect user language preference if set. The client can override the server selection, as I have done with this change.

I do think that some of this logic has to live on the client, simply because EAC3 Atmos support but not TrueHD Atmos support is a fairly Apple centric issue, so it wouldn't make sense to have server-side settings for this. Though I would much prefer that audio track preference was able to be added to the device profile.

[LePips]

First off, thank you for breaking up your changes, explaining your motivations, and the continuing insightful conversation.

I'm going to have to agree with this comment about the motivations of this PR, and that this does seem like a one-shot niche solution.

As for user language, I agree, that's a problem, but currently the app doesn't support language selection, or even track selection at all in the native player.

You are right, but I would require that those features are developed prior to something like this. Given a user's default language, we could add a similar option to this to request filtered on those languages the best available stream. But, should that already be happening or is there some issue with the server that would absolutely require client logic like this? That was in reference to:

I do think that some of this logic has to live on the client, simply because EAC3 Atmos support but not TrueHD Atmos support is a fairly Apple centric issue

---

There definitely needs to be an audio selector on the movie preview screen like in the standard Jellyfin app

The mechanism for selecting the media tracks prior to playback will be possible after (now) #1581.

---

Mish-mash of a few comments:

I believe the media encoder files as well as this one have logic that might interfere with exactly what the client requests ... serverside track selection, I'd need to sync that language selector up with the server it looks like. No clue what the API for that would be.

I agree with the previously linked comment that more investigation on how the server picks a stream is warranted. Especially if it, for some reason, would select a track that actually differs from the one requested via its index when we get the posted playback information.

The "syncing user settings with the server" would just be through other simple API calls editing user settings.

[LePips]

Please do not explicitly add translations in PRs.

Wait, that's intentional - I added translations for all languages. I rebased today. Per the contribution guide: "new strings that are not part of an experimental feature must be localized"

Apologies, we could be a bit more clear by rephrasing: "new strings that are not part of an experimental feature must be added for localization". We use Weblate for our localizations and new strings are only to be added to the English file.