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

Improve audio normalization #11457

Merged
merged 5 commits into from
Apr 30, 2024
Merged

Conversation

Bond-009
Copy link
Member

  • Move calculation of LUFS to a scheduled task as it's pretty slow
  • Correctly calculate album LUFS
  • Don't try to convert replaygain tags to LUFS values

TODO:

  • Add progress to task
  • Scan album gain tags
  • Make more configurable

* Move calculation of LUFS to a scheduled task as it's pretty slow
* Correctly calculate album LUFS
* Don't try to convert replaygain tags to LUFS values
Copy link
Member

@nielsvanvelzen nielsvanvelzen left a comment

Choose a reason for hiding this comment

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

✅ API changes look fine, but please make sure they are merged (together with the required web changes!!) before the next unstable build so we have at least 1 week to find any left-over API mismatches between server and web.

@Bond-009 Bond-009 added this to In progress in Release 10.9.z via automation Apr 28, 2024
@Chaphasilor
Copy link

I've tested this with @Bond-009's help and it seems to be working (on Windows at least). We found one bug related to albums with only a single track but that is now fixed.

I've confirmed this works properly and already integrated the changes into Finamp without issues 😁

@Bond-009 Bond-009 requested a review from a team April 29, 2024 15:28
}

// Album gain
var albums = _libraryManager.GetItemList(new InternalItemsQuery
Copy link
Member

Choose a reason for hiding this comment

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

I'm never a fan of pulling all of an item type from the database.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but this can be changed when progress reporting is added

Recursive = true
});

foreach (var a in albums)
Copy link
Member

Choose a reason for hiding this comment

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

Should add a try..catch so we don't stop processing on first (or any) failure

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a try catch inside of CalculateLUFSAsync where I'd expect these failures to take place.
Seems unlikely anything would go wrong here, and it it does (like failing to create the concat file) it would likely happen to the other items as well

@crobibero crobibero merged commit 48bb164 into jellyfin:master Apr 30, 2024
13 of 14 checks passed
Release 10.9.z automation moved this from In progress to Done Apr 30, 2024
@Bond-009 Bond-009 deleted the audionormalization branch April 30, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

6 participants