Conversation
c16fd18 to
a4e2363
Compare
| /// Sub-Channel information to include. | ||
| #[repr(u8)] | ||
| #[derive(Clone, Copy, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)] | ||
| pub enum SubchannelOption { | ||
| /// RAW P-W sub-channel data (96 bytes) | ||
| RawPw = 0b001, | ||
|
|
||
| /// Formatted Q sub-channel data (16 bytes). | ||
| #[default] | ||
| Q = 0b010, | ||
|
|
||
| /// Corrected and de-interleaved R-W sub-channel data (96 bytes). | ||
| Rw = 0b100, | ||
| } |
There was a problem hiding this comment.
If this corresponds to
of https://www.13thmonkey.org/documentation/SCSI/x3_304_1997.pdf I am seeing other fields mentioned, like 000b (no sub-channel data - which is mandatory to support).
If these are the only options supported right now, we should indicate that there are others and where one can find information on the other unsupported options.
There was a problem hiding this comment.
The 000b variant is covered by the None variant of the Option<SubchannelOption> above.
In this table, we realistically have three choices: RAW, Q and P-W.
The 000b value is to not include this data.
Therefore, a user can specify:
- To request
RAW:Some(SubchannelOption::Raw). - To request
Q:Some(SubchannelOption::Q). - To not include any sub-channel data:
None.
There was a problem hiding this comment.
Ok. Please add a comment adding this information, and it would be nice to refer to or include table 37 or reference this somewhere. We want to make things more transparent as to how some specifications are being followed.
There was a problem hiding this comment.
When an Option is used to represent a type, it's implicitly understood that there is a choice to not give any values, akin to C's NULL (but properly typed).
Speaking of referring to tables in the spec, I'd have preferred to do that if there were any ambiguity in finding it.
In this case, there isn't any, since you were perfectly able to do just that above.
I feel it's redundant to have them.
Nevertheless, I've added the said comments.
There was a problem hiding this comment.
What you have in the comments is good and sufficient.
I was able to find a table that matches the correspondences in this situation, but that doesn't mean this is good practice. Or that we want users to do this.
From experience, what I've found happens is that someone will find a different table or different piece of information. So they too feel that you could find it on your own, and therefore you didn't follow what they saw (but didn't specify what it was that they saw).
I understand that table numbers, pages, and table names in specs are fragile. But I'd like to have a solution somehow to reduce the problem of: you didn't follow the spec that I'm thinking of (and you should be able to find on your own); no, I didn't, I followed the spec that I am thinking of according to some table that you should be able to find on your own.
|
@skr4n Sorry that there are merge conflicts due to breaking this up. Would you handle the conflict, please? In prior attempts, I botched things. Thanks. |
|
Done. |
2 participants
A direct interface to the MMC command, with all options provided.
It allows for extracting data from various parts of the disc, including sub-channel data and error correction information.