diff --git a/NAPS2.Sdk/Platform/ISystemCompat.cs b/NAPS2.Sdk/Platform/ISystemCompat.cs index e745d6bb79..98cae99ecb 100644 --- a/NAPS2.Sdk/Platform/ISystemCompat.cs +++ b/NAPS2.Sdk/Platform/ISystemCompat.cs @@ -44,6 +44,8 @@ internal interface ISystemCompat string SaneLibraryName { get; } + bool IsLibUsbReliable { get; } + IntPtr LoadLibrary(string path); IntPtr LoadSymbol(IntPtr libraryHandle, string symbol); diff --git a/NAPS2.Sdk/Platform/LinuxSystemCompat.cs b/NAPS2.Sdk/Platform/LinuxSystemCompat.cs index 68dd6cf6d9..f56dccb31a 100644 --- a/NAPS2.Sdk/Platform/LinuxSystemCompat.cs +++ b/NAPS2.Sdk/Platform/LinuxSystemCompat.cs @@ -53,6 +53,8 @@ internal class LinuxSystemCompat : ISystemCompat public string SaneLibraryName => "libsane.so.1"; + public bool IsLibUsbReliable => true; + public IntPtr LoadLibrary(string path) => LinuxInterop.dlopen(path, RTLD_LAZY | RTLD_GLOBAL); public IntPtr LoadSymbol(IntPtr libraryHandle, string symbol) => LinuxInterop.dlsym(libraryHandle, symbol); diff --git a/NAPS2.Sdk/Platform/MacSystemCompat.cs b/NAPS2.Sdk/Platform/MacSystemCompat.cs index adc4c93c17..e92bf1decf 100644 --- a/NAPS2.Sdk/Platform/MacSystemCompat.cs +++ b/NAPS2.Sdk/Platform/MacSystemCompat.cs @@ -64,6 +64,8 @@ public string[] LibrarySearchPaths public string SaneLibraryName => "libsane.1.dylib"; + public bool IsLibUsbReliable => false; + public IntPtr LoadLibrary(string path) => MacInterop.dlopen(path, RTLD_LAZY | RTLD_GLOBAL); public IntPtr LoadSymbol(IntPtr libraryHandle, string symbol) => MacInterop.dlsym(libraryHandle, symbol); diff --git a/NAPS2.Sdk/Platform/WindowsSystemCompat.cs b/NAPS2.Sdk/Platform/WindowsSystemCompat.cs index 35a0e4bfcb..e33e080cd1 100644 --- a/NAPS2.Sdk/Platform/WindowsSystemCompat.cs +++ b/NAPS2.Sdk/Platform/WindowsSystemCompat.cs @@ -47,6 +47,8 @@ internal abstract class WindowsSystemCompat : ISystemCompat public string SaneLibraryName => "sane.dll"; + public bool IsLibUsbReliable => true; + public IntPtr LoadLibrary(string path) => Win32.LoadLibrary(path); public string GetLoadError() => Marshal.GetLastWin32Error().ToString(); diff --git a/NAPS2.Sdk/Scan/Internal/Sane/Native/SaneClient.cs b/NAPS2.Sdk/Scan/Internal/Sane/Native/SaneClient.cs index 341b323467..edbeb6a412 100644 --- a/NAPS2.Sdk/Scan/Internal/Sane/Native/SaneClient.cs +++ b/NAPS2.Sdk/Scan/Internal/Sane/Native/SaneClient.cs @@ -11,7 +11,6 @@ private static SaneNativeLibrary GetNativeLibrary(ISaneInstallation saneInstalla { lock (SaneLock) { - saneInstallation.Initialize(); return new SaneNativeLibrary(saneInstallation.LibraryPath, saneInstallation.LibraryDeps); } } diff --git a/NAPS2.Sdk/Scan/Internal/Sane/SaneScanDriver.cs b/NAPS2.Sdk/Scan/Internal/Sane/SaneScanDriver.cs index 9a2a29edb3..5adee29f45 100644 --- a/NAPS2.Sdk/Scan/Internal/Sane/SaneScanDriver.cs +++ b/NAPS2.Sdk/Scan/Internal/Sane/SaneScanDriver.cs @@ -9,7 +9,7 @@ namespace NAPS2.Scan.Internal.Sane; internal class SaneScanDriver : IScanDriver { private static string? _customConfigDir; - + private readonly ScanningContext _scanningContext; public SaneScanDriver(ScanningContext scanningContext) @@ -22,15 +22,6 @@ public SaneScanDriver(ScanningContext scanningContext) : File.Exists("/.flatpak-info") ? new FlatpakSaneInstallation() : new SystemSaneInstallation(); - if (_customConfigDir == null && !OperatingSystem.IsWindows()) - { - // SANE caches the SANE_CONFIG_DIR environment variable process-wide, which means that we can't willy-nilly - // change the config dir. However, if we use a static directory name and only create the actual directory - // when we want to use it, SANE will (without caching) use the directory when it exists, and fall back to - // the default config dir otherwise. - _customConfigDir = Path.Combine(_scanningContext.TempFolderPath, Path.GetRandomFileName()); - Installation.SetCustomConfigDir(_customConfigDir); - } #else Installation = null!; #endif @@ -53,20 +44,39 @@ void MaybeCallback(SaneDeviceInfo device) return Task.Run(() => { - // TODO: This is crashing after a delay for no apparent reason. - // That's okay because we're in a worker process, but ideally we could fix it in SANE. - using var client = new SaneClient(Installation); - // TODO: We can use device.type and .vendor to help pick an icon etc. - // https://sane-project.gitlab.io/standard/api.html#device-descriptor-type - if (Installation.CanStreamDevices) + Installation.Initialize(); + string? tempConfigDir = MaybeCreateTempConfigDirForSingleBackend(options.SaneOptions.Backend); + try { - client.StreamDevices(MaybeCallback, cancelToken); + // TODO: This is crashing after a delay for no apparent reason. + // That's okay because we're in a worker process, but ideally we could fix it in SANE. + using var client = new SaneClient(Installation); + // TODO: We can use device.type and .vendor to help pick an icon etc. + // https://sane-project.gitlab.io/standard/api.html#device-descriptor-type + if (Installation.CanStreamDevices) + { + client.StreamDevices(MaybeCallback, cancelToken); + } + else + { + foreach (var device in client.GetDevices()) + { + MaybeCallback(device); + } + } } - else + finally { - foreach (var device in client.GetDevices()) + if (tempConfigDir != null) { - MaybeCallback(device); + try + { + Directory.Delete(tempConfigDir, true); + } + catch (Exception ex) + { + _scanningContext.Logger.LogDebug(ex, "Error cleaning up temp SANE config dir"); + } } } }); @@ -110,147 +120,101 @@ private static string GetName(SaneDeviceInfo device) return null; } - private static string GetBackend(string saneDeviceName) - { - return saneDeviceName.Split(':')[0]; - } + public static string GetBackend(ScanDevice device) => GetBackend(device.ID); + + private static string GetBackend(string saneDeviceName) => saneDeviceName.Split(':')[0]; public Task Scan(ScanOptions options, CancellationToken cancelToken, IScanEvents scanEvents, Action callback) { return Task.Run(() => { + bool hasAtLeastOneImage = false; try { - ScanWithSaneDevice(options, cancelToken, scanEvents, callback, options.Device!.ID); - } - catch (DeviceOfflineException) - { - // Some SANE backends (e.g. airscan, genesys) have inconsistent IDs so "device offline" might actually - // just mean "device id has changed". We can query for a "backup" device that matches the name of the - // original device, and assume it's the same physical device, which should generally be correct. - string? backupDeviceId = QueryForBackupSaneDevice(options.Device!); - if (backupDeviceId == null) - { - throw; - } - ScanWithSaneDevice(options, cancelToken, scanEvents, callback, backupDeviceId); - } - }); - } - - void ScanWithSaneDevice(ScanOptions options, CancellationToken cancelToken, IScanEvents scanEvents, - Action callback, string deviceId) - { - bool hasAtLeastOneImage = false; - try - { - using var client = new SaneClient(Installation); - if (cancelToken.IsCancellationRequested) return; - _scanningContext.Logger.LogDebug("Opening SANE Device \"{ID}\"", deviceId); - using var device = client.OpenDevice(deviceId); - if (cancelToken.IsCancellationRequested) return; - var optionData = SetOptions(device, options); - var cancelOnce = new Once(device.Cancel); - cancelToken.Register(cancelOnce.Run); - try - { - if (!optionData.IsFeeder) - { - var image = ScanPage(device, scanEvents, optionData) ?? - throw new DeviceException("SANE expected image"); - callback(image); - } - else + Installation.Initialize(); + using var client = new SaneClient(Installation); + if (cancelToken.IsCancellationRequested) return; + _scanningContext.Logger.LogDebug("Opening SANE Device \"{ID}\"", options.Device!.ID); + using var device = client.OpenDevice(options.Device.ID); + if (cancelToken.IsCancellationRequested) return; + var optionData = SetOptions(device, options); + var cancelOnce = new Once(device.Cancel); + cancelToken.Register(cancelOnce.Run); + try { - while (ScanPage(device, scanEvents, optionData) is { } image) + if (!optionData.IsFeeder) { - hasAtLeastOneImage = true; + var image = ScanPage(device, scanEvents, optionData) ?? + throw new DeviceException("SANE expected image"); callback(image); } - } - } - finally - { - cancelOnce.Run(); - } - } - catch (SaneException ex) - { - switch (ex.Status) - { - case SaneStatus.Good: - case SaneStatus.Cancelled: - return; - case SaneStatus.NoDocs: - if (!hasAtLeastOneImage) + else { - throw new DeviceFeederEmptyException(); + while (ScanPage(device, scanEvents, optionData) is { } image) + { + hasAtLeastOneImage = true; + callback(image); + } } - - break; - case SaneStatus.DeviceBusy: - throw new DeviceBusyException(); - case SaneStatus.Invalid: - // TODO: Maybe not always correct? e.g. when setting options - throw new DeviceOfflineException(); - case SaneStatus.Jammed: - throw new DevicePaperJamException(); - case SaneStatus.CoverOpen: - throw new DeviceCoverOpenException(); - default: - throw new DeviceException($"SANE error: {ex.Status}"); - } - } - } - - private string? QueryForBackupSaneDevice(ScanDevice device) - { - // If we couldn't get an ID match, we can call GetDevices again and see if we can find a name match. - // This can be very slow (10+ seconds) if we have to query every single backend (the normal SANE behavior for - // GetDevices). We can hack this by creating a temporary SANE config dir that only references the single backend - // we need, so it ends up being only ~1s. - _scanningContext.Logger.LogDebug( - "SANE Device appears offline; re-querying in case of ID change for name \"{Name}\"", device.Name); - string? tempConfigDir = MaybeCreateTempConfigDirForSingleBackend(GetBackend(device.ID)); - try - { - using var client = new SaneClient(Installation); - var backupDevice = client.GetDevices() - .FirstOrDefault(deviceInfo => GetName(deviceInfo) == device.Name); - if (backupDevice.Name == null) - { - _scanningContext.Logger.LogDebug("No matching device found"); - return null; - } - return backupDevice.Name; - } - finally - { - if (tempConfigDir != null) - { - try + } + finally { - Directory.Delete(tempConfigDir, true); + cancelOnce.Run(); } - catch (Exception ex) + } + catch (SaneException ex) + { + switch (ex.Status) { - _scanningContext.Logger.LogDebug(ex, "Error cleaning up temp SANE config dir"); + case SaneStatus.Good: + case SaneStatus.Cancelled: + return; + case SaneStatus.NoDocs: + if (!hasAtLeastOneImage) + { + throw new DeviceFeederEmptyException(); + } + + break; + case SaneStatus.DeviceBusy: + throw new DeviceBusyException(); + case SaneStatus.Invalid: + // TODO: Maybe not always correct? e.g. when setting options + throw new DeviceOfflineException(); + case SaneStatus.Jammed: + throw new DevicePaperJamException(); + case SaneStatus.CoverOpen: + throw new DeviceCoverOpenException(); + default: + throw new DeviceException($"SANE error: {ex.Status}"); } } - } + }); } - private string? MaybeCreateTempConfigDirForSingleBackend(string backendName) + private string? MaybeCreateTempConfigDirForSingleBackend(string? backendName) { - if (!Directory.Exists(Installation.DefaultConfigDir) || _customConfigDir == null) + if (string.IsNullOrEmpty(backendName)) + { + return null; + } + if (!Directory.Exists(Installation.DefaultConfigDir)) { // Non-typical SANE installation where we don't know the config dir and can't do this optimization return null; } - // SANE normally doesn't provide a way to only query a single backend - it's all or nothing. - // However, there is a workaround - if we use the SANE_CONFIG_DIR environment variable, we can specify a custom - // config dir, which can have a dll.conf file that only has a single backend specified. + if (_customConfigDir == null) + { + // SANE caches the SANE_CONFIG_DIR environment variable process-wide, which means that we can't willy-nilly + // change the config dir. However, if we use a static directory name and only create the actual directory + // when we want to use it, SANE will (without caching) use the directory when it exists, and fall back to + // the default config dir otherwise. + _customConfigDir = Path.Combine(_scanningContext.TempFolderPath, Path.GetRandomFileName()); + Installation.SetCustomConfigDir(_customConfigDir); + } + // By using a custom config dir with a dll.conf file that only has a single backend specified, we can force SANE + // to only check that backend Directory.CreateDirectory(_customConfigDir); // Copy the backend.conf file in case there's any important backend-specific configuration var backendConfFile = $"{backendName}.conf"; @@ -264,7 +228,7 @@ void ScanWithSaneDevice(ScanOptions options, CancellationToken cancelToken, ISca File.WriteAllText(Path.Combine(_customConfigDir, "dll.conf"), backendName); // Create an empty dll.d dir so SANE doesn't use the default one Directory.CreateDirectory(Path.Combine(_customConfigDir, "dll.d")); - _scanningContext.Logger.LogDebug("Create temp SANE config dir {Dir}", _customConfigDir); + _scanningContext.Logger.LogDebug("Created temp SANE config dir {Dir}", _customConfigDir); return _customConfigDir; } diff --git a/NAPS2.Sdk/Scan/Internal/ScanOptionsValidator.cs b/NAPS2.Sdk/Scan/Internal/ScanOptionsValidator.cs index 65f44473c5..c45da80c9f 100644 --- a/NAPS2.Sdk/Scan/Internal/ScanOptionsValidator.cs +++ b/NAPS2.Sdk/Scan/Internal/ScanOptionsValidator.cs @@ -8,8 +8,7 @@ internal class ScanOptionsValidator { public ScanOptions ValidateAll(ScanOptions options, ScanningContext scanningContext, bool requireDevice) { - // Easy deep copy. Ideally we'd do this in a more efficient way. - options = options.ToXml().FromXml(); + options = options.Clone(); if (options.Device != null && options.Driver != Driver.Default) { diff --git a/NAPS2.Sdk/Scan/SaneOptions.cs b/NAPS2.Sdk/Scan/SaneOptions.cs index 5908e84004..7ed1ce6a49 100644 --- a/NAPS2.Sdk/Scan/SaneOptions.cs +++ b/NAPS2.Sdk/Scan/SaneOptions.cs @@ -5,4 +5,8 @@ /// public class SaneOptions { + /// + /// Limit the devices queried by GetDevices/GetDeviceList to the given SANE backend. + /// + public string? Backend { get; set; } } \ No newline at end of file diff --git a/NAPS2.Sdk/Scan/ScanController.cs b/NAPS2.Sdk/Scan/ScanController.cs index 95e5ca6658..14e6e8ac28 100644 --- a/NAPS2.Sdk/Scan/ScanController.cs +++ b/NAPS2.Sdk/Scan/ScanController.cs @@ -1,6 +1,9 @@ using System.Threading; +using Microsoft.Extensions.Logging; using NAPS2.Ocr; +using NAPS2.Scan.Exceptions; using NAPS2.Scan.Internal; +using NAPS2.Scan.Internal.Sane; namespace NAPS2.Scan; @@ -138,17 +141,61 @@ void PageProgressCallback(double progress) => ScanStartCallback(); return AsyncProducers.RunProducer(async produceImage => { - try + var bridge = _scanBridgeFactory.Create(options); + + async Task DoScan(ScanOptions actualOptions) { - var bridge = _scanBridgeFactory.Create(options); - await bridge.Scan(options, cancelToken, new ScanEvents(PageStartCallback, PageProgressCallback), + await bridge.Scan(actualOptions, cancelToken, new ScanEvents(PageStartCallback, PageProgressCallback), (image, postProcessingContext) => { - image = _localPostProcessor.PostProcess(image, options, postProcessingContext); + image = _localPostProcessor.PostProcess(image, actualOptions, postProcessingContext); produceImage(image); PageEndCallback(image); }); } + + try + { + try + { + await DoScan(options); + } + catch (DeviceOfflineException) when + (options.Driver == Driver.Sane && + (_scanningContext.WorkerFactory != null || PlatformCompat.System.IsLibUsbReliable)) + { + // Some SANE backends (e.g. airscan, genesys) have inconsistent IDs so "device offline" might actually + // just mean "device id has changed". We can query for a device that matches the name of the + // original device, and assume it's the same physical device, which should generally be correct. + // + // TODO: Ideally this would be contained within SaneScanDriver, but due to libusb's unreliability on + // macOS, we have to make sure each call is in a separate worker process. Makes me wonder if the + // scanning pipeline could be redesigned so that drivers have more control over worker processes. + _scanningContext.Logger.LogDebug( + "SANE Device appears offline; re-querying in case of ID change for name \"{Name}\"", + options.Device!.Name); + var getDevicesOptions = options.Clone(); + getDevicesOptions.SaneOptions.Backend = SaneScanDriver.GetBackend(options.Device!); + ScanDevice? matchingDevice = null; + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancelToken); + await bridge.GetDevices(getDevicesOptions, cts.Token, device => + { + if (device.Name == options.Device!.Name) + { + matchingDevice = device; + cts.Cancel(); + } + }); + if (matchingDevice == null) + { + _scanningContext.Logger.LogDebug("No matching device found"); + throw; + } + var actualOptions = options.Clone(); + actualOptions.Device = matchingDevice; + await DoScan(actualOptions); + } + } catch (Exception ex) { scanError = ex; diff --git a/NAPS2.Sdk/Scan/ScanOptions.cs b/NAPS2.Sdk/Scan/ScanOptions.cs index 3c1d9807be..23a837d0e5 100644 --- a/NAPS2.Sdk/Scan/ScanOptions.cs +++ b/NAPS2.Sdk/Scan/ScanOptions.cs @@ -1,4 +1,5 @@ using NAPS2.Ocr; +using NAPS2.Serialization; namespace NAPS2.Scan; @@ -143,4 +144,10 @@ public class ScanOptions public bool FlipDuplexedPages { get; set; } public KeyValueScanOptions? KeyValueOptions { get; set; } + + public ScanOptions Clone() + { + // Easy deep copy. Ideally we'd do this in a more efficient way. + return this.ToXml().FromXml(); + } } \ No newline at end of file