Skip to content
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

Specify max amount of retries per sector #30

Open
MerlijnWajer opened this issue Apr 19, 2020 · 6 comments
Open

Specify max amount of retries per sector #30

MerlijnWajer opened this issue Apr 19, 2020 · 6 comments

Comments

@MerlijnWajer
Copy link

I wrote this mail to the cdparanoia-users list as a while ago:

https://marc.info/?l=cdparanoia&m=158566693027322&w=2


Hi,

I was wondering if anyone had any insight into limiting the amount of
re-reads and corrections cdparanoia will attempt to do for a single
sector (or at an even lower level). I find that for quite some CDs,
cdparanoia will try re-reading specific parts many, many times.

I've logged all the callbacks with --stderr-progress, mapped "cdframes"
to sectors to seconds of an audio CD, and for some parts, over 700000
correction() callbacks are issued for a single second (so 75 seconds).

Ripping a CD (in some cases) in it's entirety takes over 7 hours.
Ripping the same CD without any paranoia (-Z or -Y) will often finish
within minutes. Around the sectors that are problematic, the no-paranoid
mode will usually sound worse, and sometimes entire samples are gone.

So there is a clear advantage to using paranoia, but I can't help but
wonder if 7000000 reads and corrections are really required for a single
second of audio. Is there a way to set a cap on the amount of retries
for any given sector, so that I can attempt to strike a balance between
quality and time spent ripping? [1]

A sample breakdown of such a rip log looks like this [2] - 30 million
correction callbacks. Being able to impose an upper limit would likely
be helpful.

Regards,
Merlijn


[1] --never-skip=<number> doesn't seem to help here, since the drive is
not skipping, as far as I can tell.

[2] $ python breakdown.py ../5-legare-street-d3-t2/paranoid-1/log.txt
wrote 327631
finished 14
read 250191
verify 456005
jitter 101
correction 30109688
scratch 0
scratch repair 0
skip 160
drift 0
backoff 0
overlap 1526884
dropped 26
duped 29
transport error 0
cache error 0

Basically, I am interested in getting this in place for both libcdio-paranoia and cdparanoia.
Do you have any pointers / places to look?

Keep in mind that the above mail is written with cdparanoia in mind, not libcdio-cdparanoia, but I assume the same paranoia logic applies.

@rocky
Copy link
Collaborator

rocky commented Apr 19, 2020

What you suggest - limiting the maximum number of rereads or rereads/per specific sector, makes sense, especially if put under a flag.

My suggestion then is to code this up and try it. If that seems to work fine we can then put it here and let people try it. If there are no objections, then put out a release.

With respect to cdparanoia, I don't have any more insight or inside information than you do.

@MerlijnWajer
Copy link
Author

MerlijnWajer commented Apr 22, 2020

So after many hours of hitting my head against the wall, I found out that --never-skip=X actually should have some effect, but this piece of code prevents it from being anything but a multiple of 5...

https://github.com/rocky/libcdio-paranoia/blob/443905efa83bdb0bd83fe62c5f1a841542ef956a/lib/paranoia/paranoia.c#L3112

Why is that mod even there?

Anyway, with that removed, I can now play with different --never-skip=X settings. With --never-skip=1, it basically doesn't really do any verification beyond the bare minimum, but it's still miles better than just -Z, where I have seen it lose 10+ seconds of audio at times, and then offset everything back by 10 seconds, essentially completely messing up the tracks. (Of course, these were tests, and this doesn't happen with paranoia enabled, so don't see this as a bug!)

@rocky
Copy link
Collaborator

rocky commented Apr 22, 2020

Why is that mod even there?

That code hasn't been changed from cdparanoia-III-10.2. I haven't a clue as to why it's there other than what's in the comments which also pretty much go back to the original cdparanoia.

Actually, I do see a comment add a little above the line you cite:

    /* ???: To be studied
     */

@MerlijnWajer
Copy link
Author

I've replied to the paranoia mailing -- not sure if you're on the list: https://marc.info/?l=cdparanoia&m=158763276007524&w=2

@rocky
Copy link
Collaborator

rocky commented Apr 23, 2020

I am not on that list. If you don't get any response there, you might also try [email protected] which is low volume but a little more active. Thanks to spammers you have to join the list in order to post, but as I said, there is very little mail and of course you can ask people to email you directly and unsubscribe in a little while.

@MerlijnWajer
Copy link
Author

Subbed.

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

No branches or pull requests

2 participants