Bug 451082 - libksane should not expose "source" for the Brother DS-640
Summary: libksane should not expose "source" for the Brother DS-640
Status: RESOLVED FIXED
Alias: None
Product: libksane
Classification: Frameworks and Libraries
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Kåre Särs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-03-03 12:22 UTC by Tobias Leupold
Modified: 2022-03-26 09:02 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tobias Leupold 2022-03-03 12:22:57 UTC
Using libksane 21.12.2, the "source" option for the DS-640 scanner from Brother is exposed (Vendor: Brother, Model: DS-640).

It's a portable pullthrough scanner. The only option to choose is "Automatic Document Feeder(left aligned)".

This alone is not a problem, but after having scanned a page, libksane tries to scan the next one, as it thinks the source would be a real document feeder. This leads to an error message after each scanned page ("Document feeder is empty").

I think that the "source" option should be inactive for this model, as there's no other way to feed documents into the scanner, and there should be no attempt to scan another document after having scanned one, as there's no way to do a continuous scan as with a "real" document feeder.
Comment 1 Tobias Leupold 2022-03-03 15:51:23 UTC
The problem seems to be that, as soon as the "source" option contains something that looks like the device had an ADF, multi page scanning is activated:

    void KSaneCorePrivate::determineMultiPageScanning(const QVariant &value)
    {
        const QString sourceString = value.toString();

        m_executeMultiPageScanning = sourceString.contains(QStringLiteral("Automatic Document Feeder")) ||
        sourceString.contains(sane_i18n("Automatic Document Feeder")) || 
        sourceString.contains(QStringLiteral("ADF")) ||
        sourceString.contains(QStringLiteral("Duplex"));
    }

And here, it's started automatically:

    void KSaneCorePrivate::imageScanFinished()
    {
        Q_EMIT q->scanProgress(100);
        if (m_scanThread->frameStatus() == KSaneScanThread::ReadReady) {
            Q_EMIT q->scannedImageReady(*m_scanThread->scanImage());
            // now check if we should have automatic ADF batch scanning
            if (m_executeMultiPageScanning && !m_cancelMultiPageScan) {
                // in batch mode only one area can be scanned per page
                Q_EMIT q->scanProgress(-1);
                m_scanThread->start();
                return;
            }
    ...

m_cancelMultiPageScan can, as far as I could see it, only be set to true by calling KSaneCore::stopScan().

Can we even fix this within libksane? Or is this something upstream should care about?
Comment 2 Tobias Leupold 2022-03-09 07:52:17 UTC
What about some option like "Start ADF batch scans automatically" or such, which is checked by default if m_executeMultiPageScanning is evaluated to be true? This way, this could be unchecked for e.g. the Brother DS-640, and the underlying infrastructure would not have to be changed?
Comment 3 Kåre Särs 2022-03-09 10:25:37 UTC
I think this is a bug in the backend, if it is not an "Automatic Document Feeder" it should not advertise itself as one.

My preferred solution would be to have the brother backend "fixed". Maybe it should state only "Document feeder"?

Then again this is not a thing that is governed by the sane-standard and only heuristics in libksane... :( 

Adding an option like you said is not impossible either, but a bit ugly workaround for something that could be "fixed" in the backend... Then again there might be code shared between multiple brother scanners and that would add extra burden there...

You seem to have coding skills ;) maybe you want to help?
1) My first choice would be to ask brother to fix it.  Seems to be proprietary :( 
2) Add a public API to libksane and use that in Skanlite.
....
3) A bit risky variant would be to "blacklist" ADF for that model 

Are you willing to do it?
Comment 4 Tobias Leupold 2022-03-09 11:50:26 UTC
My coding skills are quite limited, but of course, I'll help if I can :-)

I didn't dive into the sources too much yet. The question is where this could or should be fixed preferably. Where does the "Automatic Document Feeder" string come from? Is this something SANE decides, or does it come directly from the backend? Libksane simply gets it from SANE, doesn't it?

The brscan backend is proprietary, this is right. I fear we're lost here. I can try to find out if I can report this somewhere, but I wouldn't rely on this.

Well, after all, the scanner does arguably have an ADF, with the limitation that it can only feed one document at a time automatically.

I only thought that having an additional "only one scan at a time" option for devices with an ADF would possibly be a benefit for everbybody not wanting to trigger batch scans, even if multiple documents are provided by the ADF.

So, leaving Brother's upstream out: We can blacklist this very device or add an option. If you want, I can try to add it and create a PR if I succeed. What option would you prefer? As said, I think that an additional option would possibly be a benefit for all ADF devices (and when set to true, the current behavior also would remain unchanged).

Also, I think that for devices with only one "source" option, this option should not be exposed, as it can't be changed anyway. What do you think?

I'll see how far I can get ;-)
Comment 5 Tobias Leupold 2022-03-09 20:56:09 UTC
Here we are, https://invent.kde.org/graphics/libksane/-/merge_requests/60 is how I would do it :-)