Menu

#631 Optical drives are serialised by two global flags; auto-start breaks with more than one drive

New
nobody
None
Medium
Enhancement
1 day ago
1 day ago
No

XLD 20250302 (157.2), macOS 26A428, Apple Silicon, four USB optical drives.

Summary

With more than one drive attached, XLD uses them strictly one at a time, and "Start ripping automatically" quietly stops working. Both come from a single global flag where a per-drive one is needed. The ripping engine itself is already per-disc, so the serialisation sits only in the queue and the controller.

What the user sees

  • A disc cannot be inserted into a second drive until the first has finished reading -- the whole "Open Audio CD" menu is disabled meanwhile.
  • Ripping never overlaps: four discs in four drives are ripped one after another, whatever "Maximum threads" is set to.
  • "Automatically open the disc when inserted" + "Start ripping automatically" appear to do nothing as soon as a second drive is in use.
  • A drive that hangs (a copy-protected disc, say) stalls every other drive too. In my case that cost a full night of unattended ripping.

Cause 1 -- XLDQueue: one rip at a time, globally

XLDConverterTask.m:

- (BOOL)isAtomic
{
    return [NSStringFromClass(decoderClass) isEqualToString:@"XLDCDDARipper"];
}

XLDQueue keeps a single BOOL atomic. Once an atomic task runs, nothing else may start:

if(!atomic) {
    if(threadsOccupied < [delegate maxThreads]) {
        atomic = [task isAtomic];
        [task beginConvert];
        ...
    }
}
else {
    if((threadsOccupied < [delegate maxThreads]) && ![task isAtomic]) { ... }
}

"Atomic" is the right idea, but the resource being protected is a drive, not the application. Tasks reading different drives have no reason to wait for each other.

Cause 2 -- XLDController: driveIsBusy is app-wide

XLDController.h has a single BOOL driveIsBusy. It disables the menu:

else if([menuItem action] == @selector(readCDDA:))
    return (driveIsBusy == NO);

and, more damagingly, suppresses automount for a newly inserted disc:

if(device && !driveIsBusy && !openingFiles && ([o_autoMountDisc state] == NSOnState))
    automount = YES;

Automatic ripping hangs off automount, so this is why the auto-start settings appear dead with several drives: with four drives one of them is nearly always busy.

Suggested fix

Make both per drive. The device path is already at hand: XLDConverterTask has -rippingSession, and XLDCDDARippingSession already stores devicePath (it only needs a getter).

  • XLDConverterTask: add -atomicKey, returning the session's device path for a CDDA task and nil otherwise.
  • XLDQueue: replace BOOL atomic with an NSCountedSet of busy devices. A task may start when there is a free thread and, if it has a key, when no running task holds that key. Tracks of the same disc still run one at a time.
  • XLDController: replace BOOL driveIsBusy with an NSMutableSet of device paths plus -isDriveBusy: / -setDrive:busy:. The menu validation resolves the item's volume to its device and asks about that drive only; the automount check does the same.
- (BOOL)canStartTask:(id)task
{
    if(threadsOccupied >= [delegate maxThreads]) return NO;
    NSString *key = [task atomicKey];
    if(!key) return YES;
    return ![busyDevices containsObject:key];
}

Result

Running with this since 18 September on four drives. Discs can be inserted in any order without waiting, all four rip at the same time, and the existing auto-start settings finally do what they say: put a disc in and it rips.

Two drives overlapping for thirteen minutes, from this morning's run:

08:24 -> 08:38  HL-DT-ST BD-RE BP06LU10    Best Of Smooth Jazz Vol. 4
08:25 -> 08:43  HL-DT-ST DVDRAM GP30NB40   Maxi Power Vol. 3 - CD 1

Both AccurateRip-verified, correct per-drive read offsets, no read errors. Across 74 discs on these four drives: zero read errors and no offset mix-ups. (The per-drive offset needs ticket #629 to be correct at this point; without that fix, parallel ripping would make #629 worse.)

Related: #629 (read offset shared between drives), #630 (crash when a disc is queued twice).

Happy to supply the full patch against trunk.

-- Walter van der Heiden

Discussion


Log in to post a comment.