Support both AccurateRip V1 and AccurateRip V2 at the same time

[JoeLametta]

This one is kindly being worked on by MerlijnWajer.

thomasvs/morituri#119 (comment)

[peaveyman]

Has there been progress made on this?

[JoeLametta]

@MerlijnWajer told me he already completed this one but I'm still waiting for him to reply my last e-mails.

[peaveyman]

Thanks!!!

[MerlijnWajer]

I'm going to see if I can prepare some initial patches over the next week.

[JoeLametta]

@MerlijnWajer Thanks for the reply.
Didn't want to put pressure on you.

[MerlijnWajer]

Clearly didn't manage to do it that soon. Really hoping to send some initial patches your way soon.

[JoeLametta]

@MerlijnWajer No problem.

[MerlijnWajer]

I have submitted an initial PR for the tool used to calculate both V1 and V2 checksums. Once that works fine, the next step is to integrate V2 in the report/logger code, the offset code and the image code.

[MerlijnWajer]

Since the code to calculate V1 and V2 will be in Very Soon™, this is a next good thing to have. I have written some code to do this, but it's outside of morituri's code base, so I need to figure out how to integrate it. OTOH, this should be fairly straightforward:

1. Add the right fields to the result structure
2. For each track, calculcate both checksums
3. Perform some clever logic to decide if a track matches. (Some decisions need to be made there, I would suggest that if ARv1 or ARv2 matches, it's good enough)
4. Add code to the logger to print this extra info (I have code that probably applies 1:1 here)

[JoeLametta]

I would suggest that if ARv1 or ARv2 matches, it's good enough)

+1 for the OR

PS: If you think I can help you completing this one just let me know.

[RecursiveForest]

This has since landed with #37 , no? Or did that merely replace the ARvN method without enabling v1 OR v2 checking?

[MerlijnWajer]

Correct. It only uses v1 still. But the task can support both v1 and v2. So, as my post above, it shouldn't a lot of work to add using ARv1 and ARv2 (half a day or so, with testing)

[JoeLametta]

@MerlijnWajer
Is there any status update available for this one?

[MerlijnWajer]

Yep. I'm working on it right now. Will see if I can get something ready for merging on friday.

[JoeLametta]

Yep. I'm working on it right now. Will see if I can get something ready for merging on friday.

OK, thanks.

[ArchangeGabriel]

Just as a thought, I think it would be nice to have both ARv1 and v2 in the log file. Or at least to make it configurable. ;)

[MerlijnWajer]

@ArchangeGabriel - that is exactly what will happen. This code has been mostly ready for some time, but I am first getting the flac/encode code in. By the way, this should trivial for others to pick up too. So if someone wants to pick it up, I can just drop my current code here and guide someone to actually finish it. Otherwise - wait for the flac+tagging code to be done.

[MerlijnWajer]

I've done most of the work, but need to test it still (it's getting close to 3am)

This commit adds retreiving/storing/matching the ARv1 AND ARv2 checksums: MerlijnWajer@feaf3ff

It doesn't yet modify result/logger.py to 'print the right thing'. And that will have to be done for every logger, too. So that's some work that I do not intend to do all by myself.

tl;dr commit I linked might work for both v1 and v2 matching, but is not complete, needs testing (by me), but once that is done, I need help to update all our loggers. I can get this part done tomorrow.

@JoeLametta could you help with that (loggers)?

[gorgobacka]

@MerlijnWajer @JoeLametta Are there any updates for this?

[MerlijnWajer]

Somebody (forgot the nickname) said they would make the loggers ARv1/ARv2 capable, so I'm kind of waiting for someone to do that. I can also pick this up later for one of the loggers.

[solstice0]

I hacked together ARv2 support; it's in my fork on the branch SupportARv2. I mainly did this for fun and my approach to implementing it probably isn't ideal but I'm kind of proud so I thought I'd share. I've ripped about a dozen CD's with it and have not encountered any issues so far.

[RecursiveForest]

I'd appreciate if this could get some user testing: #187

[gorgobacka]

In the next days, I will test it on some CDs.

[JoeLametta]

This issue can now be closed because #187 (RecursiveForest) has been merged.