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

report (&/or abort) on cdparanoia non-fatal errors #88

Open
RecursiveForest opened this issue Dec 14, 2016 · 7 comments
Open

report (&/or abort) on cdparanoia non-fatal errors #88

RecursiveForest opened this issue Dec 14, 2016 · 7 comments
Assignees
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Needed: patch A pull request is required Priority: high High priority
Milestone

Comments

@RecursiveForest
Copy link
Contributor

If cdparanoia returns "!" (or any other code that should cause alarm), whipper should stop ripping or at minimum report an error.

@RecursiveForest RecursiveForest added this to the 1.0 milestone Dec 14, 2016
@MerlijnWajer
Copy link
Collaborator

I'm repeating myself, but I have code for this. I will try to get it in soon, but if anyone wants to do it before I do, post here and I'll send my unclean patch.

@RecursiveForest
Copy link
Contributor Author

Iff the patch you have contains the entirety of your support for it, I'd be happy to integrate it.

@MerlijnWajer
Copy link
Collaborator

Here are two patches. They will not outright apply, but the gist is in there:

(1) https://ptpb.pw/EbXI
(2) https://ptpb.pw/feyY

And here's a patch to parse cdrdao warnings:

(1) https://ptpb.pw/Iwb0

@MerlijnWajer
Copy link
Collaborator

Can we make this perhaps part of the next milestone after 1.0? I don't think we really -need- this now?

@tn5421
Copy link

tn5421 commented Feb 6, 2018

My understanding of the rationale says that it probably is necessary for the 1.0 milestone, since acquiring the 'perfect rip' means we need to know if any kind of error occurs.

@45054
Copy link

45054 commented Feb 6, 2018

I'd also think that fixing / enhancing the error reporting should be part of the 1.0 milestone especially regarding the aforementioned bug #75 which would otherwise produce false logs.

@MerlijnWajer
Copy link
Collaborator

Fair enough. I've found that cdparanoia can spit out a lot of errors and still rip with 100% accuracy, after many retries and errors (and corrections). So they are not a completely accurate representation. But then again, maybe the ones in #75 are different from the errors/warnings I usually see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Needed: patch A pull request is required Priority: high High priority
Projects
None yet
Development

No branches or pull requests

5 participants