Skip to content

Conversation

anshg1214
Copy link
Member

@anshg1214 anshg1214 commented Oct 29, 2024

This PR implements an enhanced track matching feature by utilising track links from the API (spotify & appleMusic) data. When available, album data is used to retrieve the correct URL for upcoming tracks, improving the accuracy and completeness of track information in the queue.

// 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.

@anshg1214 anshg1214 requested a review from MonkeyDo October 29, 2024 12:19
// Filter the tracks in the album that are not yet matched
const queueTracksInAlbum = currentQueueAfterCurrent.filter((track) => {
return (
getReleaseMBID(track) === releaseMBID &&
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.

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants