Large sample (>587) drive read offset fix

[buddyabaddon]

I recently came across Whipper and wanted to try it out. My drive has a sample offset of +667 and all attempts to use Whipper resulted in a known (and longstanding) issue (#234). I started to investigate using libcdio-paranoia directly with the same commands generated by Whipper and found I was hitting a libcdio-paranoia longstanding issue (#14).

It seems drives with read offsets over 587 samples in length hit this issue. 588 sample (pairs) happens to be the size of an audio disc sector... so I started investigating.

The sector offset implementation in libcdio-paranoia looks to be flawed. The TOC sector positions on the drive object are being updated and the offset values are then being used to make library function calls that shouldn't receive offset values but rather absolute values.

I refactored the logic so it is not modifying the TOC sector start positions. The absolute values are then used when calling library functions to perform operations such as obtaining track numbers from sector indexes (this was previously failing with offset values as they'd often be offset into the next track). Offsets are now applied only when reading and un-applied when absolute sector indexes are needed.

I am now fully able to use both libcdio-paranoia and Whipper with my drive.

I tested this change with my PLEXTOR BD-R PX-B940SA (revision 1.08) drive which uses a +667 sector read offset:

• Ran 'make test' - all tests passed
• Read tracks from an audio CD (comparing results with EAC and AccurateRip v1/v2):
  • Read a single track via --sample-offset=667
  • Read a single track via --sample-offset=667 --force-overread
  • Read multiple tracks via --sample-offset=667 --batch
  • Read multiple tracks via --sample-offset=667 --batch --force-overread
• Used my patched libcdio-paranoia library with Whipper to run:
  • whipper offset find - correctly identified my drive's +667 sector read offset
  • whipper cd rip
  • whipper cd rip --force-overread

Everything produced correct results.

I would appreciate additional verification by the community with other drives with differing sector read offsets (both negative and positive).

So far, this issue is fixed for me.

---

Verified on drives with sample offsets:

• +667 @buddyabaddon
• +594 @mdosch
• +702 @aereaux

[fenugrec]

This sounds awesome. I had briefly looked into this a while back but after testing a few naive fixes, realized it would probably need some more elaborate rework - as this PR certainly seems to confirm : ))

Looking forward to testing this in a few days when I'm able. [EDIT - tested, whipper offset find and whipper cd rip now working !]

[rocky]

LGTM - Code and comments look excellent!

Just as you, I'd like to hear back from others on this. I'll wait a couple of days and then merge.

Again, thanks!

[iMartyn]

I can confirm that master works with my +667 drive, can we get a release soon? Also perhaps get a checked-in ./configure or at least a release tarball that includes it? Autotools is arcane magic for most folks these days.

[rocky]

@iMartyn Thank you for reminding me to set up a sponsor button. ;-)

If you are a packager and need to meet a specific deadline for a distribution release, let me know the release schedule. In the near term I am focusing on an upcoming BlackHat Asia 2024 conference I will be talking at.

There are specific special dates which I plan on making releases of stuff like March 15 (which will probably be just decompiler-related projects), and June 12. However if there is is significant support for this project shown by donations, I will make a release sooner.

[iMartyn]

It's been ~20 years since I was a packager, hence why I spent hours fighting autotools! Enjoy the conference, I'll probably make a docker image at some point soon(ish) and contribute the dockerfile back. I think my message might have come across as entitled, that wasn't my intent, I just didn't want to open an issue for "release soon pls" which felt even more entitled! I'm really happy this exists, and doubly so that @buddyabaddon found a solution for these drives, enabling whipper that to take my CDs via flac to minidisc in an automated way.

[fenugrec]

I can confirm that master works with my +667 drive, can we get a release soon? Also perhaps get a checked-in ./configure or at least a release tarball that includes it?

Eh, I find after enough iterations of autoconf, autoreconf, automake, you eventually get there.

[buddyabaddon]

From scratch, I believe one needs to:
Install: autoconf, libcdio-dev, libtool, pkg-config packages
Run: ./autogen.sh; make; make install

[fenugrec]

For reference, here is the build script on Arch https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=libcdio-paranoia-git#n28

[iMartyn]

for me at least I needed --fix-missing in there somewhere. Took a lot of tries and exactly the right versions of all the autotools. I'm not saying it's impossible but it is a lot of trial and error that could be saved by having configure in the release file.

[rocky]

@iMartyn If your goal is to to save others a lot of time in trial and error, you could spend some time and put in a PR. If you don't want to spend the time, then you could offer to pay someone else to do it instead. (It doesn't have to be, and I would prefer not me). As the issue tracker shows there are platforms for requesting that code be written or worked on). Or you just live the fact that on open-source projects you may have to do trial and error.

There was a time when perhaps 20 years ago in that time when you did more packaging, when open source was more about contributing than observing.

[harry-newton]

I can also confirm the patched version worked for my +667 drive.

What can I do to help ?

/H

[rocky]

I can also confirm the patched version worked for my +667 drive.

What can I do to help ?

/H

Things that come to mind are to star the project, and if you are feeling really generous consider sponsoring or donating to the project.

[harry-newton]

Done and done, though I'm afraid you won't be able to retire on my la'al donation.

/H

[rocky]

Done and done, though I'm afraid you won't be able to retire on my la'al donation.

/H

But nevertheless it is greatly appreciated. Heartfelt thanks.

I just got back from a trip from halfway around the world. I hope to put out a new release this week.