Add read speed option for macOS and Linux

[strict-flower]

Description

Background discussion is #59

This PR will add the ReadSpeed enum (src/data_reader/read_speed.rs) and related public API. The enum represents a request to the drive for a particular read speed.

Limitation

• This PR will add the public API and macOS / Linux backend implementations, but it doesn't add a Windows backend implementation.
• The requested speed doesn't necessarily match the actual read speed, according to the specification.

[Bloomca]

I think this API looks good! I left a few comments for the implementation, also there a few typos in the comments but we can fix it later.

I think we should an example of "slow_read_track" which would read the first track from the default drive at 10x.

@strict-flower One question I am curious about and forgot to ask in the issue. If custom multiplier does persist (we'll need to test it), do you think we should set speed back to optimal in the drive destructor?

[strict-flower]

@Bloomca Thanks for the quick review. I've fixed them. I'll write & testing the Linux implementation today.

One question I am curious about and forgot to ask in the issue. If custom multiplier does persist (we'll need to test it), do you think we should set speed back to optimal in the drive destructor?

I tested that.

The DKIOCCDSETSPEED ioctl seems to set the drive speed until the disc is ejected. Below is my experiment summary.

1. Read with Unchanged -> corrupted and 24x
2. Read with CustomMultiplier(10) -> success and 10x
3. Read with Unchanged -> success and 10x
4. Eject and re-insert the disc
5. Read with Unchanged -> corrupted and 24x

... and the default state seems to be Optimal, but we can't confirm that because DKIOCCDGETSPEED only returns the current speed¹, according to my experiments.

Footnotes

1. I also tested that. I inserted eprintln! in the read loop to print the return value of the DKIOCCDGETSPEED ioctl, and it changed between 10x and 24x depending on the actual read speed. This means that it returns the current speed. ↩

[strict-flower]

I think it's difficult to restore the original state. At least, macOS doesn't seem to provide a way to get the original state.

Moreover, we could get the read speed performance data of the drive, but we can't get the current speed policy of the drive. In other words, even if the current speed is 10x, we can't determine whether the value was set by another program, is the default value of the drive, or was selected automatically.

Therefore, I think just setting Optimal is the best way to implement the destructor. (deleted, please read the next comment)

[strict-flower]

I wrote the implementation for Linux and tested. It seems to work correctly.

Also I tested Optimal / Unchanged / CustomMultiplier with the USB-connected drive that I used for the macOS test, and it doesn't behave the same way as on macOS. I didn't retrieve the read speed, but I only observed whether the read succeeded or returned corrupted data (It could probably be obtained using GET PERFORMANCE, but I haven't implemented that yet).

1. Read with CustomMultiplier(10) -> success
2. Read with Unchanged -> success
3. Read with Optimal -> corrupted
4. Read with Unchanged -> success
5. Read with CustomMultiplier(1) -> success

Although I didn't eject the CD during the above procedure, the speed setting did not persist. On Linux, it seems to be reset on each open(2).

Additionally, I tested this on an internal disc drive (ASUS BC-12D2HT). This drive seems to ignore the requested speed with the SET CD SPEED command¹. Nevertheless, this behaviour doesn't necessarily violate MMC-3, because MMC-3 specifies that The Logical Unit is to select the Logical Unit Read Speed specified or any higher rate.

Thus, unlike my earlier comments, I think the best destructor behaviour may be OS-dependent. However, there is also the possibility that we don't need to restore the policy at all.

• macOS: Optimal (?)
• Linux: Nothing to do.
• Windows: ?

Footnotes

1. Unlike the USB-connected drive, it could read the disc correctly with all settings I tested. However, the USB-connected drive could read most discs correctly at 24x, so there may still be some hidden hardware-specific issue. ↩

[Bloomca]

Thanks for the comprehensive testing!

Alright, I believe it is easier to skip restoring and just document that the behavior is OS/drive dependent. I thought for a second to expose setDriveSpeed method on the CdReader itself but since we don't expose the the drive primitive I don't think it is necessary.

Can you please add a test where we read first track on ~10x speed, and then second track on optimal speed?

After that I think we can just merge your PR and I will make a separate Windows PR.

Note: apologies but I merged a PR which changed internal API a little bit, so there are some conflicts

[strict-flower]

I believe it is easier to skip restoring and just document that the behavior is OS/drive dependent

I agree.

10x / optimal test

Should the test be deterministic? We can add a read example, but I think it's hard to make this a deterministic test because the actual behaviour depends on the hardware.

(If you didn't mean an integration test, could you clarify what kind of test you had in mind?)

I'll fix the conflicts and write the documentation in tomorrow. I've also changed the PR title.

[Bloomca]

Oh, I apologize -- I meant an example. Something similar to https://github.com/Bloomca/rust-cd-da-reader/blob/main/examples/read_first_track.rs, but with the speed config.

Tests are tricky exactly because the behaviour depends on the hardware. The library is not particularly well-tested because of that. I validate each release by myself by running all the examples mostly, and I have a CD ripper app which I run as well.

[strict-flower]

@Bloomca Okay, I wrote an example and related documentation. Could you review it?

[Bloomca]

I think it looks good! I ran the example, and it works well.

A few things:

• I think the formatter has some issues
• can you please add an empty implementation for windows? Just return (), otherwise Windows build won't compile

Thanks again for working on this!

[strict-flower]

Thanks for the review! I've fixed that and I've added a stub implementation for Windows backend.

I think the formatter has some issues

I've rerun cargo fmt. I'm not sure why, but it seems to have been fixed.

[Bloomca]

Awesome, thanks! The clippy fails because of unused variables on Windows, but that's fine, I will open a PR ~tomorrow to add Windows support.

After that I'll review all the Rustdocs and should make a 1.0 release this week, which will include this feature 🎉

[strict-flower]

Thanks!