-
-
Notifications
You must be signed in to change notification settings - Fork 394
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.
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
PICARD-2584: Load recording if AcoustId metadata is missing #2351
Merged
phw
merged 8 commits into
metabrainz:master
from
phw:PICARD-2584-acoustid-missing-metadata
Jan 7, 2024
Merged
PICARD-2584: Load recording if AcoustId metadata is missing #2351
phw
merged 8 commits into
metabrainz:master
from
phw:PICARD-2584-acoustid-missing-metadata
Jan 7, 2024
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ab65038
to
f7485ce
Compare
zas
reviewed
Dec 22, 2023
9ff8e76
to
6e3bea1
Compare
zas
reviewed
Dec 24, 2023
This resolves problem with AcoustID scan finding no matches or only standalone-recordings.
6e3bea1
to
f1aa249
Compare
…25% submission count
f1aa249
to
0bfcc2a
Compare
This reduces the number of additional calls while still providing enough data to avoid mismatches.
This avoids duplicate lookup calls for the essentially the same recording.
@zas I made a few enhancements, and I think this is now ready for review. With the changes I don't get significantly longer loading times while still eliminating the standalone recording results for my 5k test files. The changes allowing this are:
|
zas
approved these changes
Jan 6, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very clean patch, good job! LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Problem
AcoustID can sometimes contain no MB metadata for linked recordings. This happens if:
AFAIK both should be handled by AcoustID, but currently this does not seem to work correctly (e.g. acoustid/acoustid-server#114). There can always be some time frame where this has not been done as there is some delay in AcoustID database synchronizing., though
In case of 1) this can lead to recording data not being available and files getting matched to standalone recordings instead. It also messes with submission counts.
Examples:
In case of 2) this can lead to files being matched to a standalone-recording that does not load.
Examples:
Solution
If AcoustID provides no MB metadata but only MB recording ID load the recording data from the MB web service. This is done by the new
RecordingResolver
in the picard.acoustid package.If metadata is provided, the recording is used directly. If not a MB web service call to load the recording is initiated.
If the recording no longer exist (404) then the result is ignored. If it exists and was redirected to another recording MBID that is also linked to the same AcoustID, the submission counts get combined.