Lyrics Improvements

- Make the settings text actually consistent with behavior
- Add metadata (artist/track name) for fetched tracks
- Add ability to remove incorrectly fetched lyric
- Add lyric fetch cache; save the last 10 fetches
- Add ability to change offset in full screen, add more comments

src/renderer/api/types.ts

@@ -1029,6 +1029,14 @@ export type SynchronizedLyricsArray = Array<[number, string]>;
 
 export type LyricsResponse = SynchronizedLyricsArray | string;
 
+export type InternetProviderLyricResponse = {
+  artist: string;
+  lyrics: string;
+  title: string;
+};
+
+export type LyricOverride = Omit<InternetProviderLyricResponse, 'lyrics'>;
+
 export const instanceOfCancellationError = (error: any) => {
   return 'revert' in error;
 };

src/renderer/components/virtual-table/table-config-dropdown.tsx

@@ -9,6 +9,7 @@ import {
 } from '/@/renderer/store/settings.store';
 import { TableColumn, TableType } from '/@/renderer/types';
 import { Option } from '/@/renderer/components/option';
+import { NumberInput } from '/@/renderer/components/input';
 
 export const SONG_TABLE_COLUMNS = [
   { label: 'Row Index', value: TableColumn.ROW_INDEX },
@@ -180,6 +181,15 @@ export const TableConfigDropdown = ({ type }: TableConfigDropdownProps) => {
     });
   };
 
+  const handleLyricOffset = (e: ChangeEvent<HTMLInputElement>) => {
+    setSettings({
+      lyrics: {
+        ...useSettingsStore.getState().lyrics,
+        delayMs: Number(e.currentTarget.value),
+      },
+    });
+  };
+
   return (
     <>
       <Option>
@@ -209,6 +219,16 @@ export const TableConfigDropdown = ({ type }: TableConfigDropdownProps) => {
           />
         </Option.Control>
       </Option>
+      <Option>
+        <Option.Label>Lyric offset (ms)</Option.Label>
+        <Option.Control>
+          <NumberInput
+            defaultValue={lyricConfig.delayMs}
+            step={10}
+            onBlur={handleLyricOffset}
+          />
+        </Option.Control>
+      </Option>
       <Option>
         <Option.Control>
           <Slider

src/renderer/features/lyrics/lyric-skip.tsx

@@ -0,0 +1,32 @@
+import { RiCloseFill } from 'react-icons/ri';
+import styled from 'styled-components';
+import { Button } from '/@/renderer/components';
+
+const LyricClearButton = styled(Button)`
+  position: absolute;
+  right: 10px;
+  z-index: 999;
+  top: 7vh;
+
+  @media (max-width: 768px) {
+    top: 5vh;
+  }
+`;
+
+interface LyricSkipProps {
+  onClick: (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => void;
+}
+
+export const LyricSkip = ({ onClick }: LyricSkipProps) => {
+  return (
+    <LyricClearButton
+      leftIcon={<RiCloseFill />}
+      size="xl"
+      tooltip={{ label: 'Remove incorrect lyrics', position: 'bottom' }}
+      variant="default"
+      onClick={onClick}
+    >
+      Clear
+    </LyricClearButton>
+  );
+};

src/renderer/features/lyrics/lyrics.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from 'react';
+import { useCallback, useEffect, useRef, useState } from 'react';
 import { Center, Group } from '@mantine/core';
 import isElectron from 'is-electron';
 import { ErrorBoundary } from 'react-error-boundary';
@@ -7,7 +7,12 @@ import { getServerById, useCurrentSong } from '/@/renderer/store';
 import { UnsynchronizedLyrics } from '/@/renderer/features/lyrics/unsynchronized-lyrics';
 import { RiInformationFill } from 'react-icons/ri';
 import { TextTitle } from '/@/renderer/components';
-import { LyricsResponse, SynchronizedLyricsArray } from '/@/renderer/api/types';
+import {
+  InternetProviderLyricResponse,
+  LyricOverride,
+  LyricsResponse,
+  SynchronizedLyricsArray,
+} from '/@/renderer/api/types';
 import { useSongLyrics } from '/@/renderer/features/lyrics/queries/lyric-query';
 import { SynchronizedLyrics } from './synchronized-lyrics';
 
@@ -22,7 +27,8 @@ export const Lyrics = () => {
   const currentSong = useCurrentSong();
   const currentServer = getServerById(currentSong?.serverId);
 
-  const [override, setOverride] = useState<string | null>(null);
+  const [overrideLyrics, setOverrideLyrics] = useState<string | null>(null);
+  const [overrideData, setOverrideData] = useState<LyricOverride | null>(null);
   const [source, setSource] = useState<string | null>(null);
   const [songLyrics, setSongLyrics] = useState<LyricsResponse | null>(null);
 
@@ -36,10 +42,18 @@ export const Lyrics = () => {
 
   useEffect(() => {
     lyrics?.remoteLyricsListener(
-      (_event: any, songName: string, lyricSource: string, lyric: string) => {
+      (
+        _event: any,
+        songName: string,
+        lyricSource: string,
+        lyric: InternetProviderLyricResponse,
+      ) => {
         if (songName === songRef.current) {
+          const { lyrics, ...rest } = lyric;
           setSource(lyricSource);
-          setOverride(lyric);
+
+          setOverrideData(rest);
+          setOverrideLyrics(lyrics);
         }
       },
     );
@@ -60,7 +74,8 @@ export const Lyrics = () => {
 
     songRef.current = currentSong?.name ?? null;
 
-    setOverride(null);
+    setOverrideData(null);
+    setOverrideLyrics(null);
     setSource(null);
   }, [currentSong, remoteLyrics.isLoading, remoteLyrics?.data, remoteLyrics?.isSuccess]);
 
@@ -70,9 +85,9 @@ export const Lyrics = () => {
     if (currentSong?.lyrics) {
       lyrics = currentSong.lyrics;
 
-      setSource(currentSong?.name ?? 'music server');
-    } else if (override) {
-      lyrics = override;
+      setSource(currentServer?.name ?? 'music server');
+    } else if (overrideLyrics) {
+      lyrics = overrideLyrics;
     } else if (remoteLyrics.data) {
       setSource(currentServer?.name ?? 'music server');
       setSongLyrics(remoteLyrics.data);
@@ -102,7 +117,12 @@ export const Lyrics = () => {
     } else {
       setSongLyrics(null);
     }
-  }, [currentServer?.name, currentSong, override, remoteLyrics.data]);
+  }, [currentServer?.name, currentSong, overrideLyrics, remoteLyrics.data]);
+
+  const clearOverride = useCallback(() => {
+    setOverrideData(null);
+    setOverrideLyrics(null);
+  }, []);
 
   return (
     <ErrorBoundary FallbackComponent={ErrorFallback}>
@@ -123,12 +143,16 @@ export const Lyrics = () => {
           {Array.isArray(songLyrics) ? (
             <SynchronizedLyrics
               lyrics={songLyrics}
+              override={overrideData}
               source={source}
+              onRemoveLyric={clearOverride}
             />
           ) : (
             <UnsynchronizedLyrics
               lyrics={songLyrics}
+              override={overrideData}
               source={source}
+              onRemoveLyric={clearOverride}
             />
           )}
         </>

src/renderer/features/lyrics/synchronized-lyrics.tsx

@@ -10,8 +10,9 @@ import { PlaybackType, PlayerStatus } from '/@/renderer/types';
 import { LyricLine } from '/@/renderer/features/lyrics/lyric-line';
 import isElectron from 'is-electron';
 import { PlayersRef } from '/@/renderer/features/player/ref/players-ref';
-import { SynchronizedLyricsArray } from '/@/renderer/api/types';
+import { LyricOverride, SynchronizedLyricsArray } from '/@/renderer/api/types';
 import styled from 'styled-components';
+import { LyricSkip } from '/@/renderer/features/lyrics/lyric-skip';
 
 const mpvPlayer = isElectron() ? window.electron.mpvPlayer : null;
 
@@ -21,10 +22,17 @@ const SynchronizedLyricsContainer = styled.div`
 
 interface SynchronizedLyricsProps {
   lyrics: SynchronizedLyricsArray;
+  onRemoveLyric: () => void;
+  override: LyricOverride | null;
   source: string | null;
 }
 
-export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps) => {
+export const SynchronizedLyrics = ({
+  lyrics,
+  onRemoveLyric,
+  override,
+  source,
+}: SynchronizedLyricsProps) => {
   const playersRef = PlayersRef;
   const status = useCurrentStatus();
   const playerType = usePlayerType();
@@ -48,14 +56,20 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
   const delayMsRef = useRef(settings.delayMs);
   const followRef = useRef(settings.follow);
 
-  useEffect(() => {
-    delayMsRef.current = settings.delayMs;
-  }, [settings.delayMs]);
+  const getCurrentLyric = (timeInMs: number) => {
+    if (lyricRef.current) {
+      const activeLyrics = lyricRef.current;
+      for (let idx = 0; idx < activeLyrics.length; idx += 1) {
+        if (timeInMs <= activeLyrics[idx][0]) {
+          return idx === 0 ? idx : idx - 1;
+        }
+      }
 
-  useEffect(() => {
-    // Copy the follow settings into a ref that can be accessed in the timeout
-    followRef.current = settings.follow;
-  }, [settings.follow]);
+      return activeLyrics.length - 1;
+    }
+
+    return -1;
+  };
 
   const getCurrentTime = useCallback(async () => {
     if (isElectron() && playerType !== PlaybackType.WEB) {
@@ -78,21 +92,6 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
     return player.currentTime;
   }, [playerType, playersRef]);
 
-  const getCurrentLyric = (timeInMs: number) => {
-    if (lyricRef.current) {
-      const activeLyrics = lyricRef.current;
-      for (let idx = 0; idx < activeLyrics.length; idx += 1) {
-        if (timeInMs <= activeLyrics[idx][0]) {
-          return idx === 0 ? idx : idx - 1;
-        }
-      }
-
-      return activeLyrics.length - 1;
-    }
-
-    return -1;
-  };
-
   const setCurrentLyric = useCallback((timeInMs: number, epoch?: number, targetIndex?: number) => {
     const start = performance.now();
     let nextEpoch: number;
@@ -147,7 +146,26 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
     }
   }, []);
 
+  const removeLyric = useCallback(() => {
+    onRemoveLyric();
+
+    if (lyricTimer.current) {
+      clearTimeout(lyricTimer.current);
+    }
+
+    timerEpoch.current += 1;
+  }, [onRemoveLyric]);
+
   useEffect(() => {
+    // Copy the follow settings into a ref that can be accessed in the timeout
+    followRef.current = settings.follow;
+  }, [settings.follow]);
+
+  useEffect(() => {
+    // This handler is used to handle when lyrics change. It is in some sense the
+    // 'primary' handler for parsing lyrics, as unlike the other callbacks, it will
+    // ALSO remove listeners on close. Use the promisified getCurrentTime(), because
+    // we don't want to be dependent on npw, which may not be precise
     lyricRef.current = lyrics;
 
     if (status === PlayerStatus.PLAYING) {
@@ -159,7 +177,7 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
             return false;
           }
 
-          setCurrentLyric(timeInSec * 1000 + delayMsRef.current);
+          setCurrentLyric(timeInSec * 1000 - delayMsRef.current);
 
           return true;
         })
@@ -171,7 +189,9 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
         rejected = true;
 
         // Case 2: Cleanup happens after we hear back from main process but
-        // (potentially) before the next lyric. In this case, clear the timer
+        // (potentially) before the next lyric. In this case, clear the timer.
+        // Do NOT do this for other cleanup functions, as it should only be done
+        // when switching to a new song (or an empty one)
         if (lyricTimer.current) clearTimeout(lyricTimer.current);
       };
     }
@@ -180,6 +200,45 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
   }, [getCurrentTime, lyrics, playerType, setCurrentLyric, status]);
 
   useEffect(() => {
+    // This handler is used to deal with changes to the current delay. If the offset
+    // changes, we should immediately stop the current listening set and calculate
+    // the correct one using the new offset. Afterwards, timing can be calculated like normal
+    const changed = delayMsRef.current !== settings.delayMs;
+
+    if (!changed) {
+      return () => {};
+    }
+
+    if (lyricTimer.current) {
+      clearTimeout(lyricTimer.current);
+    }
+
+    let rejected = false;
+
+    delayMsRef.current = settings.delayMs;
+
+    getCurrentTime()
+      .then((timeInSec: number) => {
+        if (rejected) {
+          return false;
+        }
+
+        setCurrentLyric(timeInSec * 1000 - delayMsRef.current);
+
+        return true;
+      })
+      .catch(console.error);
+
+    return () => {
+      // In the event this ends earlier, just kill the promise. Cleanup of
+      // timeouts is otherwise handled by another handler
+      rejected = true;
+    };
+  }, [getCurrentTime, setCurrentLyric, settings.delayMs]);
+
+  useEffect(() => {
+    // This handler is used specifically for dealing with seeking. In this case,
+    // we assume that now is the accurate time
     if (status !== PlayerStatus.PLAYING) {
       if (lyricTimer.current) {
         clearTimeout(lyricTimer.current);
@@ -195,7 +254,7 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
       clearTimeout(lyricTimer.current);
     }
 
-    setCurrentLyric(now * 1000 + delayMsRef.current);
+    setCurrentLyric(now * 1000 - delayMsRef.current);
   }, [now, seeked, setCurrentLyric, status]);
 
   useEffect(() => {
@@ -216,6 +275,15 @@ export const SynchronizedLyrics = ({ lyrics, source }: SynchronizedLyricsProps)
           text={`Lyrics provided by ${source}`}
         />
       )}
+      {override && (
+        <>
+          <LyricLine
+            className="lyric-credit"
+            text={`(Matched as ${override.title} by ${override.artist})`}
+          />
+          <LyricSkip onClick={removeLyric} />
+        </>
+      )}
       {lyrics.map(([, text], idx) => (
         <LyricLine
           key={idx}

src/renderer/features/lyrics/unsynchronized-lyrics.tsx

@@ -1,9 +1,13 @@
 import { useMemo } from 'react';
 import styled from 'styled-components';
 import { LyricLine } from '/@/renderer/features/lyrics/lyric-line';
+import { LyricOverride } from '/@/renderer/api/types';
+import { LyricSkip } from '/@/renderer/features/lyrics/lyric-skip';
 
 interface UnsynchronizedLyricsProps {
   lyrics: string;
+  onRemoveLyric: () => void;
+  override: LyricOverride | null;
   source: string | null;
 }
 
@@ -11,7 +15,12 @@ const UnsynchronizedLyricsContainer = styled.div`
   padding: 5rem 0;
 `;
 
-export const UnsynchronizedLyrics = ({ lyrics, source }: UnsynchronizedLyricsProps) => {
+export const UnsynchronizedLyrics = ({
+  onRemoveLyric,
+  lyrics,
+  override,
+  source,
+}: UnsynchronizedLyricsProps) => {
   const lines = useMemo(() => {
     return lyrics.split('\n');
   }, [lyrics]);
@@ -24,6 +33,15 @@ export const UnsynchronizedLyrics = ({ lyrics, source }: UnsynchronizedLyricsPro
           text={`Lyrics provided by ${source}`}
         />
       )}
+      {override && (
+        <>
+          <LyricLine
+            className="lyric-credit"
+            text={`(Matched as ${override.title} by ${override.artist})`}
+          />
+          <LyricSkip onClick={onRemoveLyric} />
+        </>
+      )}
       {lines.map((text, idx) => (
         <LyricLine
           key={idx}

src/renderer/preload.d.ts

@@ -1,6 +1,6 @@
 import { IpcRendererEvent } from 'electron';
 import { PlayerData, PlayerState } from './store';
-import { QueueSong } from '/@/renderer/api/types';
+import { InternetProviderLyricResponse, QueueSong } from '/@/renderer/api/types';
 
 declare global {
   interface Window {
@@ -10,7 +10,12 @@ declare global {
       ipcRenderer: {
         APP_RESTART(): void;
         LYRIC_FETCH(data: QueueSong): void;
-        LYRIC_GET(event: IpcRendererEvent, songName: string, source: string, lyric: string): void;
+        LYRIC_GET(
+          event: IpcRendererEvent,
+          songName: string,
+          source: string,
+          lyric: InternetProviderLyricResponse,
+        ): void;
         PLAYER_AUTO_NEXT(data: PlayerData): void;
         PLAYER_CURRENT_TIME(): void;
         PLAYER_GET_TIME(): number | undefined;