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.

Sign up for GitHub

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

BrainzPlayer album-aware music search #3014

Open
wants to merge 5 commits into
base: ansh/improve-apple-music-match
Choose a base branch
from
Open

BrainzPlayer album-aware music search #3014

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a2a320
feat: Add Matching Data
anshg1214 Oct 27, 2024
8111310
feat: Improve BrainzPlayer Album Mapping
anshg1214 Oct 28, 2024
7b9f887
test: Fix Tests
anshg1214 Oct 28, 2024
00848b6
refactor: misc
anshg1214 Nov 1, 2024
afb5b93
feat: Improve saving mapping information
anshg1214 Nov 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 52 additions & 5 deletions frontend/js/src/common/brainzplayer/AppleMusicPlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,30 @@ import { Link } from "react-router-dom";
import fuzzysort from "fuzzysort";
import {
getArtistName,
getReleaseMBID,
getTrackName,
loadScriptAsync,
} from "../../utils/utils";
import { DataSourceProps, DataSourceType } from "./BrainzPlayer";
import GlobalAppContext from "../../utils/GlobalAppContext";
import { dataSourcesInfo } from "../../settings/brainzplayer/BrainzPlayerSettings";

export type AppleMusicPlayerProps = DataSourceProps;
export type AppleMusicPlayerProps = DataSourceProps & {
handleAlbumMapping: (
dataSource: keyof MatchedTrack,
releaseMBID: string,
album: {
trackName: string;
uri: string;
}[]
) => void;
};

export type AppleMusicPlayerState = {
currentAppleMusicTrack?: MusicKit.MediaItem;
progressMs: number;
durationMs: number;
listen?: BrainzPlayerQueueItem;
};
export async function loadAppleMusicKit(): Promise<void> {
if (!window.MusicKit) {
Expand Down Expand Up @@ -173,10 +184,15 @@ export default class AppleMusicPlayer
return;
}
try {
await this.appleMusicPlayer.setQueue({
const queueData = await this.appleMusicPlayer.setQueue({
song: appleMusicId,
startPlaying: true,
});
const albumId =
// eslint-disable-next-line no-underscore-dangle
queueData._queueItems[0].item.relationships.albums.data[0].id;
anshg1214 marked this conversation as resolved.
Show resolved Hide resolved

this.fetchAlbumTracksAndUpdateMappings(albumId);
} catch (error) {
handleError(error.message, "Error playing on Apple Music");
onTrackNotFound();
Expand All @@ -188,7 +204,7 @@ export default class AppleMusicPlayer
return AppleMusicPlayer.hasPermissions(appleMusicUser);
};

searchAndPlayTrack = async (listen: Listen | JSPFTrack): Promise<void> => {
searchAndPlayTrack = async (listen: BrainzPlayerQueueItem): Promise<void> => {
if (!this.appleMusicPlayer) {
await this.connectAppleMusicPlayer();
await this.searchAndPlayTrack(listen);
Expand Down Expand Up @@ -258,12 +274,14 @@ export default class AppleMusicPlayer
return false;
};

playListen = async (listen: Listen | JSPFTrack): Promise<void> => {
playListen = async (listen: BrainzPlayerQueueItem): Promise<void> => {
const { show } = this.props;
if (!show) {
return;
}
const apple_music_id = AppleMusicPlayer.getURLFromListen(listen as Listen);
this.setState({ listen });

const apple_music_id = listen.matchedTrack?.appleMusic;
if (apple_music_id) {
await this.playAppleMusicId(apple_music_id);
return;
Expand Down Expand Up @@ -448,6 +466,35 @@ export default class AppleMusicPlayer
this.setState({ currentAppleMusicTrack: item });
};

fetchAlbumTracksAndUpdateMappings = async (albumId: string) => {
// Exptract the album id from the url
const { listen } = this.state;
const releaseMBID = getReleaseMBID(listen as Listen);
if (!releaseMBID || !albumId) {
return;
}
const { handleAlbumMapping } = this.props;

const response = await this.appleMusicPlayer?.api.music(
`/v1/catalog/{{storefrontId}}/albums/${albumId}`
);

// @ts-ignore
const tracks = response?.data?.data?.[0]?.relationships?.tracks
?.data as MusicKit.Song[];

if (!tracks || tracks.length === 0) {
return;
}

const trackMappings = tracks.map((track) => ({
uri: track.id,
trackName: track.attributes?.name,
}));

handleAlbumMapping("appleMusic", releaseMBID, trackMappings);
};

getAlbumArt = (): JSX.Element | null => {
const { currentAppleMusicTrack } = this.state;
if (
Expand Down
79 changes: 78 additions & 1 deletion frontend/js/src/common/brainzplayer/BrainzPlayer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import * as React from "react";
import { toast } from "react-toastify";
import { Link } from "react-router-dom";
import { Helmet } from "react-helmet";
import Fuse from "fuse.js";
import {
ToastMsg,
createNotification,
Expand All @@ -39,11 +40,16 @@ import {
useBrainzPlayerContext,
useBrainzPlayerDispatch,
} from "./BrainzPlayerContext";
import {
getReleaseMBID,
getReleaseName,
getTrackName,
} from "../../utils/utils";

export type DataSourceType = {
name: string;
icon: IconProp;
playListen: (listen: Listen | JSPFTrack) => void;
playListen: (listen: BrainzPlayerQueueItem) => void;
togglePlay: () => void;
seekToPositionMs: (msTimecode: number) => void;
canSearchAndPlayTracks: () => boolean;
Expand Down Expand Up @@ -945,6 +951,75 @@ export default function BrainzPlayer() {
}
};

const handleAlbumMapping = (
dataSource: keyof MatchedTrack,
releaseMBID: string,
album: {
trackName: string;
uri: string;
}[]
): void => {
const currentQueue = queueRef.current;
const currentAmbientQueue = ambientQueueRef.current;

// Filter the currentQueue after the current playing track
const currentPlayingListenIndex = currentListenIndexRef.current;
const currentQueueAfterCurrent = currentQueue.slice(
anshg1214 marked this conversation as resolved.
Show resolved Hide resolved
currentPlayingListenIndex + 1
);

const newQueueMatchedTracks: Record<string, string> = {};
const newAmbientQueueMatchedTracks: Record<string, string> = {};

// Filter the tracks in the album that are not yet matched
const queueTracksInAlbum = currentQueueAfterCurrent.filter((track) => {
return (
getReleaseMBID(track) === releaseMBID &&
Copy link
Member Author

Choose a reason for hiding this comment

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

@MonkeyDo Should we exclude tracks without a releaseMBID? In some cases, a listen might lack this mapping data but still have the same trackName we’re searching for.

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 reading all this for the first time, so i hope I won't say anything too stupid XD

I don't think we should filter out non-mapped tracks, but I also don't understand why you want to do that in the first place.
Considering the mapping is a best-effort sort of feature, assume it won't always be present.

You can additionally use the releaseName and do some fuzzy matching on that if you are trying to group all tracks from the same album.

track.matchedTrack?.[dataSource] === undefined
);
});

const ambientQueueTracksInAlbum = currentAmbientQueue.filter((track) => {
return (
getReleaseMBID(track) === releaseMBID &&
track.matchedTrack?.[dataSource] === undefined
);
});

if (!queueTracksInAlbum.length && !ambientQueueTracksInAlbum.length) {
return;
}

const fuzzysearch = new Fuse(album, {
keys: ["trackName"],
});

// Find the matches for the tracks in the album
queueTracksInAlbum.forEach((track) => {
const matches = fuzzysearch.search(getTrackName(track));
if (matches[0]) {
newQueueMatchedTracks[track.id] = matches[0].item.uri;
}
});

ambientQueueTracksInAlbum.forEach((track) => {
const matches = fuzzysearch.search(getReleaseName(track));
if (matches[0]) {
newAmbientQueueMatchedTracks[track.id] = matches[0].item.uri;
}
});

// Update the matched tracks
dispatch({
type: "UPDATE_MATCHED_TRACKS",
data: {
dataSource,
queueMatchedTracks: newQueueMatchedTracks,
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it would be easier, instead of storing this info in the listens in the queue, to have a separate object belonging to the BP component, which can be used to retrieve a URI (for a specific music source) using an MBID or name string.
Something like:

albumMappings: {
  [releaseMBID || releaseName]: {
    appleMusic: appleMusicAlbumURI,
    spotify: spotifyAlbumURI,
    etc.
  }
}

That way, for example if you have the same album spread over two pages of listens, when you go to page 2 you already have the album info you need without having to do the whole process again.
Also means not updating the whole queue, which might re-render the queue UI needlessly.

On the music components side, basically in playListen you would need to call a BP method to check if a value exists in the albumMappings object for this release (by MBID/releasName), and use that if there is a hit; otherwise do the search as usual.

What do you think?

Copy link
Member Author

@anshg1214 anshg1214 Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

There is a problem in this implementation. We'll have a album mapping in this case, which might not be relevant. In order to use a album mapping, we'll have to make an API call to the data source provider every time in order to use this information.

Right now, what I'm doing is that fetching all the tracks of the current album, and then using the trackName <> URI mapping to update the upcoming tracks in the queue.

So, if we want to keep this information separate from queueItem, we can save all this information like

albumMapping: {
  [releaseName]: {
    [trackName]: {
      appleMusic: appleMusicURI,
      spotify: spotifyURI,
    }
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for the explanation, I misunderstood the code at first.

Do you think my suggestion makes sense,as opposed to modifying the queue items?

Copy link
Member Author

@anshg1214 anshg1214 Nov 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this makes sense, as we don't have to update queue items again and again, and will have a better performance with ambient queues

ambientQueueMatchedTracks: newAmbientQueueMatchedTracks,
},
});
};

React.useEffect(() => {
window.addEventListener("storage", onLocalStorageEvent);
window.addEventListener("message", receiveBrainzPlayerMessage);
Expand Down Expand Up @@ -1029,6 +1104,7 @@ export default function BrainzPlayer() {
handleError={handleError}
handleWarning={handleWarning}
handleSuccess={handleSuccess}
handleAlbumMapping={handleAlbumMapping}
/>
)}
{userPreferences?.brainzplayer?.youtubeEnabled !== false && (
Expand Down Expand Up @@ -1095,6 +1171,7 @@ export default function BrainzPlayer() {
handleError={handleError}
handleWarning={handleWarning}
handleSuccess={handleSuccess}
handleAlbumMapping={handleAlbumMapping}
/>
)}
</BrainzPlayerUI>
Expand Down
36 changes: 35 additions & 1 deletion frontend/js/src/common/brainzplayer/BrainzPlayerContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ export type BrainzPlayerActionType = Partial<BrainzPlayerContextT> & {
| "ADD_LISTEN_TO_TOP_OF_QUEUE"
| "ADD_LISTEN_TO_BOTTOM_OF_QUEUE"
| "ADD_LISTEN_TO_BOTTOM_OF_AMBIENT_QUEUE"
| "ADD_MULTIPLE_LISTEN_TO_BOTTOM_OF_AMBIENT_QUEUE";
| "ADD_MULTIPLE_LISTEN_TO_BOTTOM_OF_AMBIENT_QUEUE"
| "UPDATE_MATCHED_TRACKS";
data?: any;
};

Expand Down Expand Up @@ -317,6 +318,39 @@ function valueReducer(
ambientQueue: [...ambientQueue, ...tracksToAdd],
};
}
case "UPDATE_MATCHED_TRACKS": {
const {
queueMatchedTracks,
ambientQueueMatchedTracks,
dataSource,
} = action.data as {
queueMatchedTracks: Record<string, string>;
ambientQueueMatchedTracks: Record<string, string>;
dataSource: keyof MatchedTrack;
};

const newQueue = state.queue.map((track) => ({
...track,
matchedTrack: {
...track.matchedTrack,
[dataSource]: queueMatchedTracks[track.id],
},
}));

const newAmbientQueue = state.ambientQueue.map((track) => ({
...track,
matchedTrack: {
...track.matchedTrack,
[dataSource]: ambientQueueMatchedTracks[track.id],
},
}));

return {
...state,
queue: newQueue,
ambientQueue: newAmbientQueue,
};
}
default: {
throw Error(`Unknown action: ${action.type}`);
}
Expand Down
Loading