rasmuslos / AmpFin

Native Jellyfin music player for iOS & iPadOS
Other
154 stars 11 forks source link

Improve lyrics timing for better anticipation #54

Closed RiccardoUva closed 3 months ago

RiccardoUva commented 3 months ago

Hi,

First of all, great job with the project! It's awesome! I've been testing it for almost a month and I have some thoughts on how to improve it. I'll create more pull requests focusing on individual issues, but for now, I have a single request: the lyrics should be anticipated slightly to give the reader time to read them, exactly like in the original Apple Music.

I've read the code and noticed that there wasn't an attempt to anticipate the lyrics. After a couple of tests, I've found that an 800ms offset is perfect. I've already modified the code—it's literally a change of four characters. However, this is a hardcoded fix that should be good for everyone.

A simple slider for the offset in the settings would be fantastic, allowing the user to adjust it as they prefer.

Thanks!

Multiplatform/NowPlaying/Lyrics.swift :

private extension NowPlaying.Lyrics {
    func updateLyricsIndex() {
        guard let lyricsKeys = lyricsKeys, !lyricsKeys.isEmpty else {
            setActiveLineIndex(0)
            return
        }

        let currentTime = AudioPlayer.current.currentTime  + 0.8 // 0.8 seconds offset to anticipate slightly the lyrics
        if let index = lyricsKeys.lastIndex(where: { $0 <= currentTime }) {
            setActiveLineIndex(index)
        } else {
            setActiveLineIndex(0)
        }
    }
rasmuslos commented 3 months ago

Hey,

thanks for you contribution, but you seem to have committed some other changes (updated icon, development team, ...). I will look into your request but this PR has to many unrelated changes. You also seem to have changed some xcconfig files, as well as changing the development team. The correct way to setup your environment is described here: https://github.com/rasmuslos/AmpFin#sideload