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

read-toc progress information #299

Closed
anarcat opened this issue Oct 3, 2018 · 10 comments
Closed

read-toc progress information #299

anarcat opened this issue Oct 3, 2018 · 10 comments
Labels
Accepted Accepted issue on our roadmap Design Design or UX/UI related Feature New feature Sprintable Small enough to sprint on
Milestone

Comments

@anarcat
Copy link
Contributor

anarcat commented Oct 3, 2018

the first stage of the rip is the cdrdao read-toc command which takes a few minutes to run here. it provides some output yet that is somewhat silenced by whipper and there's zero information something is going on.

it would be nice to show that output to the user to reassure them the process is not hung.

@anarcat
Copy link
Contributor Author

anarcat commented Oct 3, 2018

i looked at the source and it seems this is deliberate:

https://github.com/JoeLametta/whipper/blob/cfcbfd56239d3c0c99ae029e40d8de6c3abdb6ce/whipper/program/cdrdao.py#L29-L42

it seems one of the reason the output is silenced is to handle gracefully a missing drive, but I would hope there are other ways of doing that. for example, we could loop over the output lines and parse that error message if present, while still keeping the output sane. another would be to duplicate the file descriptor to stderr and the pipe.

@JoeLametta
Copy link
Collaborator

Thanks for the issue report!
Will investigate whether not silencing cdrdao is the right way or if there's a better solution (as soon as I have enough time).

@MerlijnWajer, @RecursiveForest, @Freso Any suggestions about this?

Partially related to #106.

@Freso
Copy link
Member

Freso commented Oct 9, 2018

I'd say that it may make sense to show something (spinning something? could a meaningful progress indicator be added?), but maybe not the raw cdrdao output… Unless WHIPPER_DEBUG or a verbose flag is set.

@MerlijnWajer
Copy link
Collaborator

Can you see if you get any usable progress output by cdrdao? In my experience it can be reading a disk for minutes, and show almost no progress. It'd be good to first determine if it can show (sensible) progress.

@anarcat
Copy link
Contributor Author

anarcat commented Oct 10, 2018 via email

@jtl999
Copy link
Contributor

jtl999 commented Oct 18, 2018

I forked whipper and added a feature to display the contents of the "Reading TOC" stage after it's completed, and if I can figure out how to multithread said stage so I can asynchronously display the raw output while it's running I'd be willing to add a flag for it.

@anarcat
Copy link
Contributor Author

anarcat commented Oct 18, 2018

you mean 6d74686 ?

@jtl999
Copy link
Contributor

jtl999 commented Oct 18, 2018

Yes, but I'm not done yet.

@JoeLametta JoeLametta added Design Design or UX/UI related Sprintable Small enough to sprint on On Hold Waiting for other actions Needed: discussion More discussion needed before anything can be done (or still no agreement has been reached) and removed enhancement labels Nov 12, 2018
@jtl999
Copy link
Contributor

jtl999 commented Dec 2, 2018

@Freso suggested I elaborate more of the comment I made on #320 here.

I finally figured out how to run the cdrdao process as a Task, which means I can parse the output asynchronously. Now the question is how should I parse and display the output to the user? I was thinking of displaying a readable version of the CD TOC (maybe just an excerpt of the output from cdrdao) and then after it's done reading each track printing Track %d read, found %d Q sub-channels with CRC errors.

@jtl999
Copy link
Contributor

jtl999 commented Dec 3, 2018

Made some more progress.

Anyone have some thoughts?

Using configured read offset 6
Checking device /dev/sr0
Track 1 finished, found 10 Q sub-channels with CRC errors
Track 2 finished, found 11 Q sub-channels with CRC errors
Track 3 finished, found 6 Q sub-channels with CRC errors
Track 4 finished, found 8 Q sub-channels with CRC errors
Track 5 finished, found 6 Q sub-channels with CRC errors

@JoeLametta JoeLametta added the Status: in progress Issue/pull request which is currently being worked on label Dec 14, 2018
@JoeLametta JoeLametta added this to the 1.0 milestone Feb 13, 2019
@JoeLametta JoeLametta added Accepted Accepted issue on our roadmap Feature New feature and removed Needed: discussion More discussion needed before anything can be done (or still no agreement has been reached) On Hold Waiting for other actions Status: in progress Issue/pull request which is currently being worked on labels Feb 13, 2019
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 Design Design or UX/UI related Feature New feature Sprintable Small enough to sprint on
Projects
None yet
Development

No branches or pull requests

5 participants