Add pregap detection for physical CDs. #115
Conversation
|
Related issue and pull request I opened with libcdio so that the macOS implementation doesn't have to use libcdio private structs: |
|
I was struggling with why everything seemed fine except for the track 1 pregap lengths. Apparently it's standard practice to count the 2 second lead-in towards the track 1 pregap length even though the lead-in doesn't usually get included when the track 1 pregap is dumped as an HTOA. For example, XLD hard-codes in 2 bonus seconds for track 1 pregap here: I suspect EAC is doing the same but I don't have any way of confirming that. I've similarly adjusted the log writer to give track 1 a fixed 2 seconds extra pre-gap duration. I've been told that https://github.com/superg/redumper has a more rigorous solution that can read into the lead-in and search for non-zero audio data or account for rare cases of extended lead-in if we ever want to go that route. |
| #ifdef __APPLE__ | ||
| #include <IOKit/storage/IOCDTypes.h> | ||
| #include <IOKit/storage/IOCDMediaBSDClient.h> | ||
| #endif | ||
|
|
||
|
|
||
| // TODO The implementation for macOS requires access to private internal cdio | ||
| // data, namely p_cdio->env.fd. Right now we just copy-paste some struct | ||
| // definitions to work around this. Is there a less brittle way to do do this? :-/ | ||
|
|
||
| #ifdef __APPLE__ | ||
|
|
||
| // lib/driver/mmc/mmc_private.h | ||
| typedef driver_return_code_t (*mmc_run_cmd_fn_t) | ||
| ( void *p_user_data, | ||
| unsigned int i_timeout_ms, | ||
| unsigned int i_cdb, | ||
| const mmc_cdb_t *p_cdb, | ||
| cdio_mmc_direction_t e_direction, | ||
| unsigned int i_buf, void *p_buf); | ||
|
|
||
| // lib/driver/cdio_private.h | ||
| typedef struct { | ||
| driver_return_code_t (*audio_get_volume) | ||
| (void *p_env, /*out*/ cdio_audio_volume_t *p_volume); | ||
| driver_return_code_t (*audio_pause) (void *p_env); | ||
| driver_return_code_t (*audio_play_msf) ( void *p_env, | ||
| msf_t *p_start_msf, | ||
| msf_t *p_end_msf ); | ||
| driver_return_code_t (*audio_play_track_index) | ||
| ( void *p_env, cdio_track_index_t *p_track_index ); | ||
| driver_return_code_t (*audio_read_subchannel) | ||
| ( void *p_env, cdio_subchannel_t *subchannel ); | ||
| driver_return_code_t (*audio_resume) ( void *p_env ); | ||
| driver_return_code_t (*audio_set_volume) | ||
| ( void *p_env, cdio_audio_volume_t *p_volume ); | ||
| driver_return_code_t (*audio_stop) ( void *p_env ); | ||
| driver_return_code_t (*eject_media) ( void *p_env ); | ||
| void (*free) (void *p_env); | ||
| const char * (*get_arg) (void *p_env, const char key[]); | ||
| int (*get_blocksize) ( void *p_env ); | ||
| cdtext_t * (*get_cdtext) ( void *p_env ); | ||
| uint8_t * (*get_cdtext_raw) ( void *p_env ); | ||
| char ** (*get_devices) ( void ); | ||
| char * (*get_default_device) ( void ); | ||
| lsn_t (*get_disc_last_lsn) ( void *p_env ); | ||
| discmode_t (*get_discmode) ( void *p_env ); | ||
| void (*get_drive_cap) (const void *p_env, | ||
| cdio_drive_read_cap_t *p_read_cap, | ||
| cdio_drive_write_cap_t *p_write_cap, | ||
| cdio_drive_misc_cap_t *p_misc_cap); | ||
| track_t (*get_first_track_num) ( void *p_env ); | ||
| bool (*get_hwinfo) | ||
| ( const CdIo_t *p_cdio, /* out*/ cdio_hwinfo_t *p_hw_info ); | ||
| driver_return_code_t (*get_last_session) | ||
| ( void *p_env, /*out*/ lsn_t *i_last_session ); | ||
| int (*get_media_changed) ( const void *p_env ); | ||
| char * (*get_mcn) ( const void *p_env ); | ||
| track_t (*get_num_tracks) ( void *p_env ); | ||
| int (*get_track_channels) ( const void *p_env, track_t i_track ); | ||
| track_flag_t (*get_track_copy_permit) ( void *p_env, track_t i_track ); | ||
| lba_t (*get_track_lba) ( void *p_env, track_t i_track ); | ||
| lba_t (*get_track_pregap_lba) ( const void *p_env, track_t i_track ); | ||
| char * (*get_track_isrc) ( const void *p_env, track_t i_track ); | ||
| track_format_t (*get_track_format) ( void *p_env, track_t i_track ); | ||
| bool (*get_track_green) ( void *p_env, track_t i_track ); | ||
| bool (*get_track_msf) ( void *p_env, track_t i_track, msf_t *p_msf ); | ||
| track_flag_t (*get_track_preemphasis) | ||
| ( const void *p_env, track_t i_track ); | ||
| off_t (*lseek) ( void *p_env, off_t offset, int whence ); | ||
| ssize_t (*read) ( void *p_env, void *p_buf, size_t i_size ); | ||
| int (*read_audio_sectors) ( void *p_env, void *p_buf, lsn_t i_lsn, | ||
| unsigned int i_blocks ); | ||
| driver_return_code_t (*read_data_sectors) | ||
| ( void *p_env, void *p_buf, lsn_t i_lsn, uint16_t i_blocksize, | ||
| uint32_t i_blocks ); | ||
| int (*read_mode2_sector) | ||
| ( void *p_env, void *p_buf, lsn_t i_lsn, bool b_mode2_form2 ); | ||
| int (*read_mode2_sectors) | ||
| ( void *p_env, void *p_buf, lsn_t i_lsn, bool b_mode2_form2, | ||
| unsigned int i_blocks ); | ||
| int (*read_mode1_sector) | ||
| ( void *p_env, void *p_buf, lsn_t i_lsn, bool mode1_form2 ); | ||
| int (*read_mode1_sectors) | ||
| ( void *p_env, void *p_buf, lsn_t i_lsn, bool mode1_form2, | ||
| unsigned int i_blocks ); | ||
| bool (*read_toc) ( void *p_env ) ; | ||
| mmc_run_cmd_fn_t run_mmc_cmd; | ||
| int (*set_arg) ( void *p_env, const char key[], const char value[] ); | ||
| driver_return_code_t (*set_blocksize) ( void *p_env, | ||
| uint16_t i_blocksize ); | ||
| int (*set_speed) ( void *p_env, int i_speed ); | ||
| } cdio_funcs_t; | ||
|
|
||
| // lib/driver/cdio_private.h | ||
| typedef struct { | ||
| uint16_t u_type; | ||
| uint16_t u_flags; | ||
| } cdio_header_t; | ||
|
|
||
| // lib/driver/cdio_private.h | ||
| struct _CdIo { | ||
| cdio_header_t header; | ||
| driver_id_t driver_id; | ||
| cdio_funcs_t op; | ||
| void* env; | ||
| }; | ||
|
|
||
| // lib/driver/generic.h | ||
| typedef struct { | ||
| char *source_name; | ||
| bool init; | ||
| bool toc_init; | ||
| bool b_cdtext_error; | ||
| int ioctls_debugged; | ||
| void *data_source; | ||
| int fd; | ||
| track_t i_first_track; | ||
| track_t i_tracks; | ||
| uint8_t u_joliet_level; | ||
| iso9660_pvd_t pvd; | ||
| iso9660_svd_t svd; | ||
| CdIo_t *cdio; | ||
| cdtext_t *cdtext; | ||
| track_flags_t track_flags[CDIO_CD_MAX_TRACKS+1]; | ||
| unsigned char scsi_mmc_sense[263]; | ||
| int scsi_mmc_sense_valid; | ||
| char *scsi_tuple; | ||
| } generic_img_private_t; | ||
|
|
||
| #endif |
There was a problem hiding this comment.
Could you move everything under the APPLE ifdef into pregap_internal_osx.c, and everything else into pregap_internal_mmc.c. Then add a pregap_internal.h which provides the signature for a generic get_lba_index(). pregap.c can just retain the cyanrip_get_track_pregap_lba() function.
There's so little code shared. Just make meson switch between which version to compile based on the platform.
UltraFuzzy
force-pushed
the
cd-pregap-detection
branch
3 times, most recently
from
9bba1a1 to
8ad62fe
Compare
|
Just a quick note that libcdio accepted my pull request that will allow us to avoid the hacky part of the macOS implementation. They say they want to do a new release by the end of the year. If it ends up that the new libcdio release with that fix comes out before we put out a new release then I'd say we could just ditch the hacky part of the macOS implementation. I don't think there's much need for us to support old versions of libcdio on macOS when we don't even currently have a cyanrip macOS package. |
UltraFuzzy
force-pushed
the
cd-pregap-detection
branch
from
98c9e2a to
f22c3d3
Compare
2 participants
This one still needs a little more time in the oven so that I can add error checking with the Q subchannel CRC16s but I wanted to get the ball rolling on the pull request.
I wrapped the cdio internal structs needed on macOS in an ifdef like we discussed.
Also, libcdio says they'll accept a pull request for a function to get the underlying file descriptor from a CdIo_t which would eventually allow the macOS implementation to be hack-free (libcdio/libcdio#35). How do you want to handle that? Can we go ahead for now with the hacky macOS implementation? Or would you prefer to wait for me to submit a pull request with libcdio and get it merged?