whipper
whipper copied to clipboard
Reword 'track n: unknown (error)' to reflect missing from AccurateRip DB
Hello,
This wording catches me off guard wrong and I continually check for errors but it looks like the track is simply missing from the database.
Could this be re-worded to
"track n: unknown (track not present in AccurateRip database for verification)"
?
I'm used to "error" meaning I need to do something, but as far as I understand there's nothing I can do here.
The extra info would be helpful so I don't need to crack the .log
file open each time.
Let me know if this seems something worth a "beginner" PR from me! Thanks!
Hi, thanks for the issue report. I agree that the current message may be a bit too terse but keep in mind that the one you provided could be incorrect in case the given track does really exist in the AccurateRip DB but the extracted one has a different CRC (maybe because of read errors).
P.S.: The relevant code section is this one:
https://github.com/whipper-team/whipper/blob/04ff0058064b52ff9262a4a4820cb8b2b0b95125/whipper/common/accurip.py#L237-L239
In case that message gets changed, the associated tests need to be updated too.
Thanks for the feedback on this, @JoeLametta.
I was surprised to read there's another option here where the tracks do exist but the CRCs could be wrong! I have a handful of "Unknowns" right now, is there any way to tell if they are in the bad-read vs. absent state?
As you say, it looks like print report function does not distinguish this -- is it correct to rephrase this problem as "whipper doesn't provide any different response to the user for these two given states (found, not matching and entirely absent)"?
If yes, while designing this, a valuable part could be to divide these two error states so that the user is clear as to whether or not the fault could be on AccurateRip's lack of populated data vs. read errors and the like, as they're rather different.
As you say, it looks like print report function does not distinguish this -- is it correct to rephrase this problem as "whipper doesn't provide any different response to the user for these two given states (found, not matching and entirely absent)"?
I think that's not correct: whipper should issue the following messages in case no remote CRCs are found for the given track:
https://github.com/whipper-team/whipper/blob/04ff0058064b52ff9262a4a4820cb8b2b0b95125/whipper/common/accurip.py#L237-L239
In case no entry is found for the given CD, there are at least these (second one for the cd
subcommand):
https://github.com/whipper-team/whipper/blob/04ff0058064b52ff9262a4a4820cb8b2b0b95125/whipper/common/accurip.py#L141-L151
https://github.com/whipper-team/whipper/blob/87f3d00ee3686fe43f0d3a931dea6361c71a1461/whipper/command/cd.py#L545-L550
These are some of the cases, taken from our tests, which can be reported in relation to the AccurateRip status report:
https://github.com/whipper-team/whipper/blob/87f3d00ee3686fe43f0d3a931dea6361c71a1461/whipper/test/test_common_accurip.py#L190-L279
This is the specific situation you've mentioned in your first message (it means no AccurateRip entry has been found for the given track in the remote DB):
https://github.com/whipper-team/whipper/blob/87f3d00ee3686fe43f0d3a931dea6361c71a1461/whipper/test/test_common_accurip.py#L215-L223
That all sounds agreeable and after learning a bit more, I understand your meaning.
As I'm still learning, it is correct to paraphrase that "the track may indeed be in the AccurateRip database under a different disc (ID?) but the CRC given did not identify it"?
This seems to happen to me for entire discs -- could it ever happen on a single track in a disc?
If not, then perhaps this is closer:
track n: disc unknown (disc did not return AccurateRip entry for CRC verification)
WDYT, @JoeLametta - did I get closer or farther 🙂?