Add lyrics conversion and other fixes
Pull Request Title
Add lyrics conversion and other fixes
Description:
This PR includes a new feature (convert lyrics) and other small fixes for Nora.
Changelog:
-
🎉 New Features and Updates
- Convert Japanese, Chinese, and Korean lyrics to their respective Romanization systems (Romaji, Pinyin, and Romaja).
- Added new lyrics settings to enable/disable automation of translate and convert lyrics.
-
🔨 Fixes and Improvements
- Improved Discord Rich Presence; now Nora's rich presence will be displayed as 'Listening to Nora'.
- Fix the Discord Rich Presence timestamp so that Discord displays the start time and end time correctly.
- Update Discord Rich Presence when user seeks the song.
- Add a reset button in the lyrics page for removing translated and converted lyrics.
-
🐜 Known Issues and Bugs
Preview image:
Other Issues:
This PR is based on my previous PR (#277), so if you want, you can merge this PR and close the #277.
Checklist:
- [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
- [x] Have you linted your code locally before submission with
npm lintandnpm run prettier-writecommands? - [x] Have you successfully run the app with your changes locally with
npm run dev?
Thanks for the help. Tell me when the pull request is finished. I'll check it out once the release is at its final stage.
Can you also provide me with a song and an LRC file to test your pull request?
Thanks for the feedback! The pull request is finished and ready for review. I attached a zipped folder containing 3 songs and its corresponding LRC file in 3 languages (Chinese, Japanese, and Korean) below. You can download and test them against my PR. Music.zip Please note that some Japanese characters are not romanized/converted or wrongly converted due to how Japanese scripts work and the underlaying conversion libraries (WanaKana and Kuroshiro), and most Chinese lyrics are also detected as Japanese because they share the same code page.
Is there a reason for changing original/translated lyrics highlighting styles? Once translated, the original lyrics should be in a lower font size without highlighting and the translated lyrics should be highlighted like in this image.
Is there a reason for changing original/translated lyrics highlighting styles? Once translated, the original lyrics should be in a lower font size without highlighting and the translated lyrics should be highlighted like in this image.
I changed the lyrics style because I want the converted lyrics to display above the original lyrics when both converted lyrics and translated lyrics are displayed. Adding a setting entry to the setting page (or a button to the lyrics page) for selecting the primary lyrics type would solve the problem, as some people prefer translated lyrics over original lyrics while others don't.
Can you also describe what is romanizing lyrics? Could you tell me what the functionality is?
- Does it help in translations?
I assume that you refer to "romanize" as the converted form of Japanese, Chinese, and Korean lyrics, even though the term Romanize usually means convert Japanese text into Romaji, a way to write Japanese text using Latin characters (abc...). It helps people who want to learn or are learning those languages, especially new people, understand how to pronounce words in those languages. Some Spicetify extensions (romajin and beautiful-lyrics) also implement lyrics conversion.
@ElectroHeavenVN, It feels cluttered and unnecessary to display the original lyric after it has been converted.
I'll suggest a better way to go about this.
- The original lyrics are displayed with the option to romanize them by checking the
originalLanguageproperty. This could also remove theconvertedLyricsproperty from the lyrics object. - If lyrics conversion is requested, the
parsedLyrics.originalTextis updated with the romanized lyrics instead of setting it in theconvertedLyricsfield. - To toggle back, the function can use the
unparsedLyricsproperty data.
I edited the code so that translated lyrics will be the primary lyrics when available. Converted lyrics will be displayed below translated lyrics if they are available.
The originalLanguage property does not always exist in the LRC file, so I cannot rely 100% on it.
@ElectroHeavenVN
The originalLanguage property does not always exist in the LRC file, so I cannot rely 100% on it.
If it does not exist, you can use the functions you created to decide whether to use conversion.
Converted lyrics will be displayed below translated lyrics if they are available.
In beautiful-lyrics, you can see that they replace the original lyric with the converted lyric. Otherwise, in some cases, the lyrics will be too cluttered.
I got it. I modified the code so that lyrics will be displayed in 1 line by default (no small line above and below main lyrics). I apologize if I made you feel unconfortable.
I got it. I modified the code so that lyrics will be displayed in 1 line by default (no small line above and below main lyrics). I apologize if I made you feel unconfortable.
@ElectroHeavenVN, No worries. I really appreciate your contribution to Nora.
@ElectroHeavenVN, From your last commits, I think you still didn't quite get what I was saying.
Consider this example
- primary lyric
- secondary lyric (primarily used with translations)
Then consider the following scenarios.
- If the song playing has unsynced, synced, or enhanced synced lyrics without any conversions or translations, they will be displayed as
(1) primary lyrics. - If the user requests Nora to translate synced or enhanced synced lyrics, the original untranslated lyric will be displayed as the
(2) secondary lyric, and the translated lyric will be displayed as the(1) primary lyriclike in the image translating some language to English. - If the user requests lyric conversions without translations, the converted lyric will be displayed as
(1) primary lyricand no secondary lyric line will be displayed. - If the user requests lyric conversions with translations, the converted lyric will be displayed as
(2) secondary lyric, and the translated lyric will be displayed as the(1) primary lyric.
In the parseLyrics function,
- The original lyrics are displayed with the option to romanize them by checking the
originalLanguageproperty. If the LRC doesn't specify the original language or theoriginalLanguageproperty isundefined, you can use the functions you createdeg: isJapaneseString(), isChineseString()to set theoriginalLanguageproperty and decide whether to use conversion. - If lyrics conversion is requested, the
parsedLyrics.originalTextis updated with the romanized lyrics instead of setting it in theconvertedLyricsfield. - To toggle back, the function can use the
unparsedLyricsproperty data.
I hope you get the idea.
In the
parseLyricsfunction,
- The original lyrics are displayed with the option to romanize them by checking the
originalLanguageproperty. If the LRC doesn't specify the original language or theoriginalLanguageproperty isundefined, you can use the functions you createdeg: isJapaneseString(), isChineseString()to set theoriginalLanguageproperty and decide whether to use conversion.- If lyrics conversion is requested, the
parsedLyrics.originalTextis updated with the romanized lyrics instead of setting it in theconvertedLyricsfield.- To toggle back, the function can use the
unparsedLyricsproperty data.I hope you get the idea.
There is a problem with that approach.
https://github.com/Sandakan/Nora/blob/68b54f4063d4a256d4dc2350af302eee549f78a9/src/main/utils/getTranslatedLyrics.ts#L68
The unparsedLyrics property is modified after the translation, so if the lyrics are converted and then translated, the original lyrics will be lost.
@ElectroHeavenVN,
In the
parseLyricsfunction,
- The original lyrics are displayed with the option to romanize them by checking the
originalLanguageproperty. If the LRC doesn't specify the original language or theoriginalLanguageproperty isundefined, you can use the functions you createdeg: isJapaneseString(), isChineseString()to set theoriginalLanguageproperty and decide whether to use conversion.- If lyrics conversion is requested, the
parsedLyrics.originalTextis updated with the romanized lyrics instead of setting it in theconvertedLyricsfield.- To toggle back, the function can use the
unparsedLyricsproperty data.I hope you get the idea.
There is a problem with that approach.
https://github.com/Sandakan/Nora/blob/68b54f4063d4a256d4dc2350af302eee549f78a9/src/main/utils/getTranslatedLyrics.ts#L68
The
unparsedLyricsproperty is modified after the translation, so if the lyrics are converted and then translated, the original lyrics will be lost.
unparsedLyrics before translation
[re:Nora (https://github.com/Sandakan/Nora)]
[ve:3.0.0-stable]
[ti:nadaaniyan]
[ar:Akshath]
[al:nadaaniyan]
[offset:0]
[copyright:Lyrics by Lrclib (https://lrclib.net/)]
[00:00.54] कैसे तू गुनगुनाए, मुस्कुराए
[00:05.7] छोटी-मोटी बातों पे मुँह फुलाए
[00:07.71] ये नज़ाकत, मेरी आदत पास मुझे लाए
[00:12.16] नादानियाँ, नादानियाँ
[00:17.19] खींचें मुझे नादानियाँ
[00:22.37] नादानियाँ, नादानियाँ
[00:27.35] पागल करे तेरी हर अदा
[00:33.53] शाम-ओ-सुबह मैं तेरी याद करूँ
[00:38.75] तेरे ख़यालों से मैं बात करूँ
unparsedLyrics after translation
[re:Nora (https://github.com/Sandakan/Nora)]
[ve:3.0.0-stable]
[ti:nadaaniyan]
[ar:Akshath]
[al:nadaaniyan]
[offset:0]
[copyright:Lyrics by Lrclib (https://lrclib.net/). Lyrics translated using Google Translate.]
[00:00.54] कैसे तू गुनगुनाए, मुस्कुराए
[00:00.54][lang:en] how you hum, smile
[00:05.70] छोटी-मोटी बातों पे मुँह फुलाए
[00:05.70][lang:en] fuss over small things
[00:07.71] ये नज़ाकत, मेरी आदत पास मुझे लाए
[00:07.71][lang:en] This delicacy, my habit brought me closer
[00:12.16] नादानियाँ
Even though they are modified, you can see that it will preserve all the data required to return to the original lyrics.
No, I mean if I remove the convertedLyrics property and then set the originalText property with converted lyrics, when the lyrics are translated, the unparsedLyrics will be updated with converted lyrics, therefore the original lyrics are lost. Also, if the lyrics are converted before, when the user translates the lyrics, converted lyrics will be used instead of original lyrics, which in some cases does not translate at all.
No, I mean if I remove the
convertedLyricsproperty and then set theoriginalTextproperty with converted lyrics, when the lyrics are translated, theunparsedLyricswill be updated with converted lyrics, therefore the original lyrics are lost. Also, if the lyrics are converted before, when the user translates the lyrics, converted lyrics will be used instead of original lyrics, which in some cases does not translate at all.
@ElectroHeavenVN, Sorry for the delayed response
Instead of convertedLyrics, in the LyricLine interface, we can set convertedText property to store the romanized version of the originalText or undefined if it doesn't support romanization. We can also set isRomanized boolean property on LyricsData interface to easily find weather lyrics support romanization.
For your reference, here are the updated types relevant to lyrics from app.d.ts file.
type LyricsTypes = 'ENHANCED_SYNCED' | 'SYNCED' | 'UN_SYNCED' | 'ANY';
type LyricsRequestTypes = 'ONLINE_ONLY' | 'OFFLINE_ONLY' | 'ANY';
type LyricsSource = 'IN_SONG_LYRICS' | 'MUSIXMATCH' | string;
// Represents a single word in synced lyrics with timing
export type SyncedLyricsLineWord = {
text: string;
start: number;
end: number;
unparsedText: string;
};
// Represents a single translation of a lyric line in a specific language
interface TranslatedLyricLine {
lang: string; // Language code of the translation
text: string | SyncedLyricsLineWord[]; // Translated text or synced lyrics
}
// Represents a single line of lyrics, either synced or unsynced
interface LyricLine {
originalText: string | SyncedLyricsLineWord[]; // Original text of the lyric line
// THIS CHANGED
romanizedText?: string | SyncedLyricsLineWord[]; // Romanized text of the lyric line (optional)
translatedTexts: TranslatedLyricLine[]; // Array of translations in different languages
start?: number; // Timing start (for synced lyrics only)
end?: number; // Timing end (for synced lyrics only)
isEnhancedSynced: boolean; // Indicates if the original text is enhanced synced lyrics
}
// Holds all the lyrics data, whether synced or unsynced
interface LyricsData {
isSynced: boolean;
// THIS CHANGED
isRomanized: boolean; // Indicates if the lyrics have been romanized
isTranslated: boolean; // Indicates if translations are available
parsedLyrics: LyricLine[]; // Array of original lyric lines (both synced and unsynced)
unparsedLyrics: string; // Raw LRC or plain lyrics content
offset?: number; // Timing offset for synced lyrics
originalLanguage?: string; // Language of the original lyrics (optional)
translatedLanguages?: string[]; // Array of language codes of the translated lyrics (optional)
romanizedLanguage?: string; // Language code for the romanized lyrics (optional)
copyright?: string; // Copyright information
}
interface SongLyrics {
title: string;
source: LyricsSource;
lyricsType: LyricsTypes;
link?: string; // Link to external source
lyrics: LyricsData; // Original and translated lyrics data
isOfflineLyricsAvailable: boolean; // Indicates if offline lyrics are available
}
I have removed unnecessary properties and changed the code as you suggested. You can check the code when you have free time.
@ElectroHeavenVN,
https://github.com/Sandakan/Nora/blob/2accfef65aeaee560b9bcd5ee1f004b7927e64fe/src/common/parseLyrics.ts#L268
The above line shows an error in the IDE saying Property 'pinyin' does not exist on type 'typeof pinyin'..
I managed to get rid of the error in the IDE by importing pinyin with the following syntax and modifying the function.
import * as pinyin from 'pinyin';
if (pinyin.default(str[i])[0][0] != str[i]) count++;
Can you make sure it works as before?
@ElectroHeavenVN, Can you also make sure the tests also work with the included changes for the parseLyrics function?
I updated the test with additional properties, and it seems to run just fine, but in the app, when the lyric is loaded, the following exception is thrown and fails the getSongLyrics function:
TypeError: pinyin.default is not a function
at getPrecentageCN (d:\Working Repositories\Nora\src\common\parseLyrics.ts:268:16)
at parseLyrics (d:\Working Repositories\Nora\src\common\parseLyrics.ts:323:31)
at fetchLyricsFromLRCFile (d:\Working Repositories\Nora\src\main\core\getSongLyrics.ts:124:26)
at async fetchOfflineLyrics (d:\Working Repositories\Nora\src\main\core\getSongLyrics.ts:275:25)
at async getSongLyrics (d:\Working Repositories\Nora\src\main\core\getSongLyrics.ts:305:25)
at async WebContents.<anonymous> (node:electron/js2c/browser_init:2:87021) {stack: 'TypeError: pinyin.default is not a function
…us> (node:electron/js2c/browser_init:2:87021)', message: 'pinyin.default is not a function'}
I updated the test with additional properties, and it seems to run just fine, but in the app, when the lyric is loaded, the following exception is thrown and fails the
getSongLyricsfunction:TypeError: pinyin.default is not a function at getPrecentageCN (d:\Working Repositories\Nora\src\common\parseLyrics.ts:268:16) at parseLyrics (d:\Working Repositories\Nora\src\common\parseLyrics.ts:323:31) at fetchLyricsFromLRCFile (d:\Working Repositories\Nora\src\main\core\getSongLyrics.ts:124:26) at async fetchOfflineLyrics (d:\Working Repositories\Nora\src\main\core\getSongLyrics.ts:275:25) at async getSongLyrics (d:\Working Repositories\Nora\src\main\core\getSongLyrics.ts:305:25) at async WebContents.<anonymous> (node:electron/js2c/browser_init:2:87021) {stack: 'TypeError: pinyin.default is not a function …us> (node:electron/js2c/browser_init:2:87021)', message: 'pinyin.default is not a function'}
@ElectroHeavenVN,
I think it's because the @types/pinyin is not updated to the alpha release version of the pinyin package.
Can you also check out pinyin-pro, an alternative to the pinyin package that claims to be faster and uses less storage?
Sorry, I've been busy a lot recently, I will check that library as soon as possible.
It looks like pinyin-pro library is better. I updated the code to match the new library and confirmed that all the tests were passed, as well as the lyrics were converted and displayed properly in the app.
@ElectroHeavenVN,
Nice. Can you create another pull request for it?
@ElectroHeavenVN,
Nice. Can you create another pull request for it?
I opened a new PR here: #292
