fix reading with negative offsets by MmAaXx500 · Pull Request #62 · libcdio/libcdio-paranoia

MmAaXx500 · 2026-05-30T18:26:24Z

When --sample-offset is negative, toc_offset can push i_first_lsn below sector 0, resulting in an invalid READ CD command.

INFO: ioctl CDROM_SEND_PACKET for command READ CD (0xbe) failed:
     Input/output error

clamp i_first_lsn to 0 (unless force_overread)
fill missing samples with silence in the pregap

I tested the changes with 2 drives

LG CRD-8522B with an offset of -419
LG GSA-4163B with an offset of +667

Test with libcdio-paranoia

Ripped the 1st, 2nd, and last track of the same CD with the drives. The results are identical.

Commands:
libcdio-paranoia --verbose -d /dev/sr0 -O +667 1-1 # or 2-2, 16-16
libcdio-paranoia --verbose -d /dev/sr1 -O -491 1-1 # or 2-2, 16-16

Result:
71bc6d0c5548ad0cf3a31d346448d5323911211b99fc08854ebda520106d2ee4  cdda_track_01_CRD-8522B_-491.wav
71bc6d0c5548ad0cf3a31d346448d5323911211b99fc08854ebda520106d2ee4  cdda_track_01_GSA-4163B_+667.wav
453ca3714eb809c9037aa82b1878ff65cf2e40d3fb39edf63ad4d43d5a5c8f41  cdda_track_02_CRD-8522B_-491.wav
453ca3714eb809c9037aa82b1878ff65cf2e40d3fb39edf63ad4d43d5a5c8f41  cdda_track_02_GSA-4163B_+667.wav
f3b9c388118a51378381e4dde855661490850bcd3a96fbe4b8e12746a1b72cc2  cdda_track_16_CRD-8522B_-491.wav
f3b9c388118a51378381e4dde855661490850bcd3a96fbe4b8e12746a1b72cc2  cdda_track_16_GSA-4163B_+667.wav

Test with whipper

Tested if whipper can correctly identify the drive's offset, and it was able to determine the correct offset for LG CRD-8522B as -491.

Command:
whipper offset find -d /dev/sr1

Result:
[...]
INFO:whipper.command.offset:trying read offset -24...
INFO:whipper.command.offset:trying read offset 704...
INFO:whipper.command.offset:trying read offset 572...
INFO:whipper.command.offset:trying read offset 1182...
INFO:whipper.command.offset:trying read offset 688...
INFO:whipper.command.offset:trying read offset -491...
INFO:whipper.command.offset:offset of device is likely -491, confirming...

Read offset of device is: -491.
INFO:whipper.command.offset:adding read offset to configuration file

Also ripped the CD with both drives and accuraterip reported correct rip in both cases.

Commands:
whipper cd -d /dev/sr0 rip -O ~/cd-out_GSA-4163B
whipper cd -d /dev/sr1 rip -O ~/cd-out_CRD-8522B

Result:
track  1: rip accurate     (max confidence    200) v1 [5cc31a92], v2 [adf22fe0], DB [5cc31a92, adf22fe0]
track  2: rip accurate     (max confidence    200) v1 [da2e00cf], v2 [b8a1cbe3], DB [da2e00cf, b8a1cbe3]
track  3: rip accurate     (max confidence    200) v1 [ed8c10c1], v2 [bdd5ef65], DB [ed8c10c1, bdd5ef65]
track  4: rip accurate     (max confidence    200) v1 [165aebbd], v2 [79b45054], DB [165aebbd, 79b45054]
track  5: rip accurate     (max confidence    200) v1 [b038d8c7], v2 [99c9b088], DB [b038d8c7, 99c9b088]
track  6: rip accurate     (max confidence    200) v1 [a9fd9865], v2 [e657ecb6], DB [a9fd9865, e657ecb6]
track  7: rip accurate     (max confidence    200) v1 [82ee1fde], v2 [a7bd0c24], DB [82ee1fde, a7bd0c24]
track  8: rip accurate     (max confidence    200) v1 [05da34cf], v2 [9cf3c8a5], DB [05da34cf, 9cf3c8a5]
track  9: rip accurate     (max confidence    200) v1 [694b8fe7], v2 [96240b8f], DB [694b8fe7, 96240b8f]
track 10: rip accurate     (max confidence    200) v1 [70708bb4], v2 [fdab62ed], DB [70708bb4, fdab62ed]
track 11: rip accurate     (max confidence    200) v1 [c1c9875c], v2 [31a69537], DB [c1c9875c, 31a69537]
track 12: rip accurate     (max confidence    200) v1 [55876e52], v2 [808245d8], DB [55876e52, 808245d8]
track 13: rip accurate     (max confidence    200) v1 [22f371fd], v2 [bf750bd3], DB [22f371fd, bf750bd3]
track 14: rip accurate     (max confidence    200) v1 [22d75da6], v2 [1f0cc90f], DB [22d75da6, 1f0cc90f]
track 15: rip accurate     (max confidence    200) v1 [3df7dbd2], v2 [98635782], DB [3df7dbd2, 98635782]
track 16: rip accurate     (max confidence    200) v1 [3932bc35], v2 [114463e0], DB [3932bc35, 114463e0]

Note: I used AI and Actual Intelligence to make this PR

Fixes #61

rocky · 2026-05-30T22:32:30Z

@buddyabaddon: do you have any thoughts or objections to this?

eshattow · 2026-05-30T22:48:55Z

I am not so sure filling missing samples with silence is the correct approach. I do not have optical drive hardware to test this with the CD Audio album I know that has Track 00 audio, Blind Melon / Soup.

MmAaXx500 · 2026-05-31T06:37:12Z

I don't think I have that specific album, but I will check if I have any with track 0.

MmAaXx500 · 2026-05-31T18:21:29Z

It turns out I don't have a CD that has a track 0 (I only found one with and extra track on the end), so I can't verify it.

Based on cdda_track_firstsector() that handles track 0 case I assume that even if track 0 exist that just shifts the other tracks forward.

libcdio-paranoia/lib/cdda_interface/toc.c

Lines 45 to 54 in 307fa2e

    
              if (i_track == 0) { 
        
                if (d->disc_toc[0].dwStartSector == 0) { 
        
           /* first track starts at lba 0 -> no pre-gap */ 
        
           cderror(d,"402: No initial pregap\n"); 
        
           return(-402); 
        
                } 
        
                else { 
        
           return 0; /* pre-gap of first track always starts at lba 0 */ 
        
                } 
        
              } else if (i_track < i_first_track || i_track > i_last_track) {

So track 0 would still start at LBA 0 instead of being on some negative LBA address. That is the same case with track 1 as you can read LBA 0 fine and insert silence if there is need to read before that as most of the drives are incapable of reading negative LBA addresses. So effectively track 0 and track 1 handling seems same to me.

buddyabaddon · 2026-06-04T09:01:06Z

@buddyabaddon: do you have any thoughts or objections to this?

I was certainly looking for someone who owned a drive with a negative read offset when I originally fixed the large offset bug: #14 (comment) . Unfortunately, no one came forward at the time who owned such a drive. I had suspected there were some issues with the logic for this use case.

Well, it seems @MmAaXx500 has such a drive!

If the --force-overread option is not specified -and- the drive has a negative offset, I do expect that the right thing to do here is to advance i_first_lsn to 0 and fill the unreadable samples with 0's...

You'll certainly want to make sure all the existing tests still pass ('make test'). I'd also look at verifying that things still work as expected with the --force-overread option (that the logic still attempts to read into the leadin/leadout despite your drive not supporting reading into negative sector indices).

Other than some semantic naming clarification, I can't see anything I dislike about this approach. I'll need to do some testing to ensure it still works with positive offset drives. I encourage you to also ask folk to give this fix a try so you can get a variety of results from drives with differing read offsets.

I do worry about unknown corner cases and CDDA shenanigans (hidden first tracks, etc.) but this looks to be a real bug for folk with negative offset drives. Had I had such a drive (or identified someone with one) when I made my changes, I certainly would have considered this case.

Thanks for coming forward with a negative offset drive @MmAaXx500. Give me some time and I can try your changes out and report back with my results.

rocky · 2026-06-04T13:07:21Z

@buddyabaddon: do you have any thoughts or objections to this?

I was certainly looking for someone who owned a drive with a negative read offset when I originally fixed the large offset bug: #14 (comment) . Unfortunately, no one came forward at the time who owned such a drive. I had suspected there were some issues with the logic for this use case.

Well, it seems @MmAaXx500 has such a drive!

If the --force-overread option is not specified -and- the drive has a negative offset, I do expect that the right thing to do here is to advance i_first_lsn to 0 and fill the unreadable samples with 0's...

You'll certainly want to make sure all the existing tests still pass ('make test').

@MmAaXx500 does make test work?

I'd also look at verifying that things still work as expected with the --force-overread option (that the logic still attempts to read into the leadin/leadout despite your drive not supporting reading into negative sector indices).

@MmAaXx500 ?

Other than some semantic naming clarification, I can't see anything I dislike about this approach. I'll need to do some testing to ensure it still works with positive offset drives. I encourage you to also ask folk to give this fix a try so you can get a variety of results from drives with differing read offsets.

I do worry about unknown corner cases and CDDA shenanigans (hidden first tracks, etc.) but this looks to be a real bug for folk with negative offset drives. Had I had such a drive (or identified someone with one) when I made my changes, I certainly would have considered this case.

Thanks for coming forward with a negative offset drive @MmAaXx500. Give me some time and I can try your changes out and report back with my results.

Thanks @buddyabaddon for looking and the information. I'd appreciate it if you try out the changes and let us all know.

MmAaXx500 · 2026-06-04T17:09:54Z

Other than some semantic naming clarification, I can't see anything I dislike about this approach.

@buddyabaddon Please add a review comment on them and suggest a better name. What I also dislike myself is toc_clamp, but I was unable to come up with a better name as also toc_offset is used for similar purposes.

@buddyabaddon: do you have any thoughts or objections to this?

I was certainly looking for someone who owned a drive with a negative read offset when I originally fixed the large offset bug: #14 (comment) . Unfortunately, no one came forward at the time who owned such a drive. I had suspected there were some issues with the logic for this use case.
Well, it seems @MmAaXx500 has such a drive!
If the --force-overread option is not specified -and- the drive has a negative offset, I do expect that the right thing to do here is to advance i_first_lsn to 0 and fill the unreadable samples with 0's...
You'll certainly want to make sure all the existing tests still pass ('make test').

@MmAaXx500 does make test work?

It does locally, it also passed as part of the CI.

PASS: toc
============================================================================
Testsuite summary for libcdio-paranoia 10.3dev+2.1.1
============================================================================
# TOTAL: 1
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
SKIP: testparanoia
PASS: testutils
PASS: get_libcdio_version
PASS: check_paranoia.sh
PASS: endian.sh
SKIP: check_start_track_not_one.sh
============================================================================
Testsuite summary for libcdio-paranoia 10.3dev+2.1.1
============================================================================
# TOTAL: 6
# PASS:  4
# SKIP:  2
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================
SKIP: paranoia
SKIP: paranoia2
============================================================================
Testsuite summary for libcdio-paranoia 10.3dev+2.1.1
============================================================================
# TOTAL: 2
# PASS:  0
# SKIP:  2
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

I'd also look at verifying that things still work as expected with the --force-overread option (that the logic still attempts to read into the leadin/leadout despite your drive not supporting reading into negative sector indices).

@MmAaXx500 ?

Running libcdio-paranoia --verbose -d /dev/sr1 --force-overread -O -491 1-1on the CRD-8522B produces the same IO errors as I showed in #61. I kind of expect that. For the last track (16) the result is the exact same as without --force-overread it produces the correct file.

Running with --force-overread on the GSA-4163B produced exact same file for the first and last track as without --force-overread.

rocky · 2026-06-04T23:12:50Z

Other than some semantic naming clarification, I can't see anything I dislike about this approach.
...
@MmAaXx500 does make test work?

It does locally, it also passed as part of the CI.
...
Running libcdio-paranoia --verbose -d /dev/sr1 --force-overread -O -491 1-1on the CRD-8522B produces the same IO errors as I showed in #61. I kind of expect that. For the last track (16) the result is the exact same as without --force-overread it produces the correct file.

Running with --force-overread on the GSA-4163B produced exact same file for the first and last track as without --force-overread.

Okay. Good to hear. We'll wait a little bit for @buddyabaddon to test, and then this will probably merge as is.

When --sample-offset is negative, toc_offset can push i_first_lsn below sector 0, resulting in an invalid READ CD command. INFO: ioctl CDROM_SEND_PACKET for command READ CD (0xbe) failed: Input/output error - clamp i_first_lsn to 0 (unless force_overread) - fill missing samples with silence in the pregap

rocky · 2026-06-12T00:45:03Z

@buddyabaddon Do you still need more time to test, or should we merge this?

buddyabaddon · 2026-06-12T06:23:58Z

@rocky - Sorry, I haven't had a chance to mess with this locally yet; been one of those weeks. If you're itching to merge, don't wait on me.

rocky · 2026-06-12T10:23:42Z

@rocky - Sorry, I haven't had a chance to mess with this locally yet; been one of those weeks.

I totally understand.

If you're itching to merge, don't wait on me.

Not itching, but letting things go on for a while isn't good either. So let's merge, and if there are changes down the line, we can do that in future PRs.

MmAaXx500 mentioned this pull request May 30, 2026

Read failing with negative offsets: Input/output error #61

Closed

buddyabaddon reviewed Jun 5, 2026

View reviewed changes

Comment thread src/cd-paranoia.c Outdated

Comment thread src/cd-paranoia.c Outdated

MmAaXx500 force-pushed the neg-offset-read branch from d4eb2ff to dcd4ab2 Compare June 5, 2026 18:41

rocky merged commit 384f4da into libcdio:master Jun 12, 2026
2 checks passed

Uh oh!

Uh oh!

Conversation

MmAaXx500 commented May 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test with libcdio-paranoia

Test with whipper

Uh oh!

rocky commented May 30, 2026

Uh oh!

eshattow commented May 30, 2026

Uh oh!

MmAaXx500 commented May 31, 2026

Uh oh!

MmAaXx500 commented May 31, 2026

Uh oh!

buddyabaddon commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rocky commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MmAaXx500 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rocky commented Jun 4, 2026

Uh oh!

Uh oh!

Uh oh!

rocky commented Jun 12, 2026

Uh oh!

buddyabaddon commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rocky commented Jun 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MmAaXx500 commented May 30, 2026 •

edited

Loading

buddyabaddon commented Jun 4, 2026 •

edited

Loading

rocky commented Jun 4, 2026 •

edited

Loading

MmAaXx500 commented Jun 4, 2026 •

edited

Loading

buddyabaddon commented Jun 12, 2026 •

edited

Loading