Skip to content

Add AlbumNormalizationGain field to BaseItemDto#1596

Open
HeroBrine1st wants to merge 4 commits into
finamp-app:redesignfrom
HeroBrine1st:redesign
Open

Add AlbumNormalizationGain field to BaseItemDto#1596
HeroBrine1st wants to merge 4 commits into
finamp-app:redesignfrom
HeroBrine1st:redesign

Conversation

@HeroBrine1st

@HeroBrine1st HeroBrine1st commented Apr 11, 2026

Copy link
Copy Markdown

Changes

Following jellyfin/jellyfin#14745, this PR uses new dto field to (hopefully) avoid using metadata provider somewhere in the future. But currently it at least allows to avoid waiting on response :-)

There's no Jellyfin release with that PR currently. Do not merge until version is updated in comment I added.

I used 658adae as an example on how to add support for download repair.

Other changes

  • Renamed "parentNormalizationGain" to "albumNormalizationGain" in MetadataProvider. I was thinking Jellyfin uses normalization gain for everything (e.g. movies) and named it so because.. well, it's fixed now
  • Removed outdated comment about secondary metadataProvider arguments while there's only one

No tests aside from basic building were done - waiting for Jellyfin release. Also there can be problems with check-codegen task (it does a lot of removals and reformatting on my system), fortunately it prints a diff so I can apply it.

Comment on lines +1543 to +1545
final albumNormalizationGain =
baseItem.albumNormalizationGain ??
providerContainer.read(metadataProvider(baseItem)).valueOrNull?.albumNormalizationGain;

@HeroBrine1st HeroBrine1st Apr 11, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Idk why formatter suddenly changed its mind :-(
Aside from outdated comment, generated code and rename, there could be exactly one line replacement and everything else is added. I really like non-invasive changes :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it's probably because of the ?? operator and Dart trying to avoid excessive indentation

@HeroBrine1st HeroBrine1st force-pushed the redesign branch 2 times, most recently from 556a745 to e6978ac Compare April 11, 2026 09:59
@HeroBrine1st

HeroBrine1st commented Apr 11, 2026

Copy link
Copy Markdown
Author

Oh wow there's no problems with check-codegen aside of messed up hive field number (I used duplicated one initially and IDK how it generated code on my end without errors)

@Chaphasilor

Copy link
Copy Markdown
Member

@HeroBrine1st your upstream changes have landed in the Jellyfin 12.0 RC builds, so I think it's time to merge this in? Or does this break backwards compatibility in any way?

@HeroBrine1st

Copy link
Copy Markdown
Author

This does not break compatibility at all but I'm still considering upgrading to RC to test it

@HeroBrine1st

Copy link
Copy Markdown
Author

It required me to clean up app folder (probably just a conflict with #1630). Otherwise, this works on 10.11.x so we can merge it. I won't have enough time to update to RC due to plugin upgrades and I don't want to clone my instance.

@HeroBrine1st HeroBrine1st marked this pull request as ready for review July 1, 2026 17:08
@Chaphasilor

Copy link
Copy Markdown
Member

I can test this on RC-2. Any specific steps I should follow?

@HeroBrine1st

Copy link
Copy Markdown
Author

So, this is the part where it gets tricky.
Normally, you'd just set it to dynamic/album-based and test right away, looking into logs for absence of request, but it is also done by UI.

You need to make metadata fetcher slow using patch from this message. It passes if it doesn't break after that :-)

Or you can simulate reproduction of that bug (in PR of patch) by keeping player in background (UI won't make request) and then looking into logs to see whether it did metadata requests or not.

@Chaphasilor

Copy link
Copy Markdown
Member

Okay, so:
With dynamic mode enabled and the 15 second delay in the metadata provider, I started a playlist that didn't contain tracks from the album I was testing with.
Then I added the first track from the album to Next Up, and skipped to it directly. It played at track normalization. Then I added the second track to next up (without changing tracks), and the volume of the current track (first in album) was adjusted to match the album normalization. That was before the metadata for the track had loaded.

I guess this means your changes are working? I did test this on 10.10 too though, with the same behavior, so I'm not entirely sure.

12.0 returns AlbumNormalizationGain by default though, whereas 10.10 doesn't return ParentNormalizationGain by default.

@HeroBrine1st

HeroBrine1st commented Jul 4, 2026

Copy link
Copy Markdown
Author

It's strange it works like that on 10.10 given you tested the same revision on both jellyfin versions. It should have waited 15 seconds before changing to album gain, but note that it will change to null gain because there's no metadata for current track, not the next one. It will then change again, to album gain this time.

@Chaphasilor

Copy link
Copy Markdown
Member

@HeroBrine1st I think what's happening is that music_player_background_task.dart kicks in, and that somehow get the parentNormalizationGain?
Can you confirm that you actually get the behavior you describe on 10.10/10.11? Can you maybe share a video of how you're testing it?

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.

2 participants