momodalo / rubyripper

Automatically exported from code.google.com/p/rubyripper
3 stars 2 forks source link

Review EAC-Style Logs #507

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
Purpose of code changes on this branch:

Change logging to EAC style (Resolves Issue 411)

When reviewing my code changes, please focus on:

Checking that log.rb and rippingInfoAtStart.rb correctly output meaningful log 
information.  secureRip.rb should also be checked in its CRC calculation 
(getCRC) and that other changes hold.

Unfortunately, I already checked this into the master branch (as I hadn't seen 
this issue template beforehand), but the relevant revisions include:

Revision 2f3fcea943df: Gracefully handle CRC/digest calculation when file size 
is wrong
Revision 1660542c2a64: Fix issue with lengths in rippingInfoAtStart.rb
Revision 2b10bee58aec: Change log format to EAC

Original issue reported on code.google.com by comradec...@gmail.com on 11 Feb 2012 at 6:50

GoogleCodeExporter commented 8 years ago
Can you please explain why the CRC calculation is necessary at all? It may be 
faster to calculate this in C, but certainly not in ruby. I also understand the 
peak level has nothing to do with the ripping quality, only with the loudness.

It drags another dependency in as well, people have to install zlib in order to 
use the ruby module.

I suggest we change the logfile as follows.

Track %s

  Filename %s

  Trial 1: %s seconds (%sx)
    MD5 sum = %s
  Trial 2: %s seconds (%sx)
    MD5 sum = %s
  All chunks matched!

Original comment by boukewou...@gmail.com on 26 Mar 2012 at 8:07

GoogleCodeExporter commented 8 years ago
Sorry for interfering and my comment will not be looking into the 
technicalities of the Issue BUT having EAC style logs (plus other things) is 
important so RubbyRipper can be accepted as an official tool for "communities" 
that are strict with the quality of the rips 

we need to have an "official" native ripping tool for linux so please keep that 
in mind 

thanks 

Original comment by barz...@gmail.com on 6 Apr 2012 at 8:03

GoogleCodeExporter commented 8 years ago
One reason for the CRC calculations is for analogy (and comparison) to EAC and 
XLD rips, which both generate CRC values in a similar fashion.  Existing 
communities (and rest assured barz that I am aware of such communities, which 
is why I wrote the patch) may depend on the CRC of other log formats rather 
than the MD5sum.  As such, we should probably support logging the CRC for 
legacy purposes.

CRC values are also crucial in that several different mechanisms for hashing 
exist which may generate different results, depending on whether the padding 
due to drive offset is included or excluded from the CRC.  The added comment 
"Null samples used in CRC calculations       : Yes" renders this debate 
unambiguous (as it implies that the padding samples will be added).

Original comment by comradec...@gmail.com on 6 Apr 2012 at 9:01

GoogleCodeExporter commented 8 years ago
EAC-style logs should report the correct starting offset of the first track, 
rather than assuming it is 0 in some cases (where the pre-gap is non-zero but 
too short for an HTOA track).  Resolving this issue depends on the resolution 
to (and hence blocks on) Issue 533.

Original comment by comradec...@gmail.com on 31 Aug 2012 at 11:55

GoogleCodeExporter commented 8 years ago

Original comment by boukewou...@gmail.com on 9 Jan 2013 at 10:51