Per-frame registration for the strip preview, mono TIFF, and no more DNG output - #1042
Per-frame registration for the strip preview, mono TIFF, and no more DNG output#1042marcinz606 wants to merge 15 commits into
Conversation
|
nkscan does have a greyscale mode, but only through the ICC output path as nikon scan had some profile for this already that did RGB->Greyscale. |
i want something generic for all scanner backends, i think just averaging rgb reads that claude suggested makes a lot of sense |
537aced to
4843c11
Compare
|
I fixed a zillion little bugs with the USB transport so hopefully this is more stable. Next release i'll once again rework the frame detection |
|
I tried this + nkscan 0.10 earlier today on an LS-50, and it would let me set the individual offsets. But when it actually did the scan, it seemed to just ignore them. I didn’t have much time to look into it, but hopefully I’ll have some time to do a few more scans soon. |
|
Yeah, something is a bit off, not yet sure what. Offset is applied but it seems less than requested (or reported detected frame has some mismatch vs what I draw in strip preview). It needs some tweaking. When extreme per-frame offset is applied it is certainly there |
|
I think i found the issue |
|
The image link is broken, so I can’t see it. I also made the frame offset deliberately large just to see if it was working, but when I scanned it, it wasn’t. |
|
|
this is max per frame offset, i think I've made it slightly better but still something is a bit off :| |
|
ah yes, I also got one strange frame like that. But without setting to max as I remember it.. But on the other frames it was like nothing happened. I might have time to do some scans tomorrow. |
|
Ok, did some new scans with the latest commit from 15 min ago. I’m still getting weird results. It’s like it’s not physically repositioning the film, but only telling the scanner to read further back in the mask or something like that. |
|
|
|
Like in the last example, we should see parts of frame 4 entering in the scan of frame 3, as shown in the preview. Instead, it seems like the film isn’t being repositioned, and the stepping motor is simply being told to read further out. We see the SA-21 mask (where the black narrows in on the right side) and eventually the motor reaches its endpoint and then just scans that same line over and over. That is my quick take. |
|
I think i narrowed it down to nkscan behaviour and fix belongs there |
|
@thetalkingdrum can you verify with nkscan python wheels built from https://github.com/marcinz606/nkscan/tree/fix/perforation-lead ? |
|
I’ll try later tonight |
|
|
Success! |
|
Can y'all try with the wheel from the tip of main? Trying to clean things up a bit. |
|
@activexray Works:) just needed dropping the frames= arg locally to match the new scan_frame() signature |
|
Gonna make a few more tweaks to the python API btw |
|
Correction. Turns out no mod was needed. NegPy's scan_frame() wrapper already checks inspect.signature(session.scan_frame) and only forwards frames= if the installed build actually accepts it, so it silently no-ops against your new API. Ran it unmodified just now, works fine. But found another bug or regression. Not sure what happened here but seems like it added the offset in the wrong direction. This is with the current main on nkscan: |
|
OK, something even weirder happened. I scanned the whole strip, but instead it scanned the fist frame six times: 
|
|
that part might a hallucination as i tried some headless tests this morning and it is mixing that up. Right now we only tested with negpy |
|
Can you point me to the fork on negpy you're working with? |
|
Claude is retracting part of the statement:) To clarify the mix-up: the "raw nkscan.Session" testing was real, but it was against an earlier build (6094418, before your logging change) and done by directly calling nkscan.Session.scan_frame() in a Python shell, bypassing NegPy — not through this PR's code. Everything on 0.11.0/9f855bc was tested through NegPy's own UI only. My earlier comment blurred those two together, sorry for the confusion. On the fork question: the patch wasn't pushed anywhere when you asked — it was a local, uncommitted change on top of pr-1042-nkscan-improve. I've now pushed it as its own branch so you can check it out directly: https://github.com/thetalkingdrum/NegPy/tree/debug/nkscan-offset-no-op (diff against pr-1042-nkscan-improve: thetalkingdrum/NegPy@pr-1042-nkscan-improve...debug/nkscan-offset-no-op) It's the two files adapted to your Python API changes, plus the init_logging("trace") call and [bleed-debug] logging left in so the trace I posted is reproducible as-is. |
I think Nikon scan uses the quick thumbnail pass to build a map of all the frames, and might be beneficial when doing full roll scanning. I think negpy's preview (and nkscans thumbnail?) is similar to this in spirit. The function actually called "preview" in Nikon scan is just a lower res pass I believe, not anything like the preview function in Negpy |
|
Ok I see what's happening. I need to de-slop some stuff. |
|
fyi I'm busy with other stuff so I'll probably revisit this during the weekend |
|
I looked a bit into the preview/scan mismatch thing in case it is helpful: Two separate sources of mismatch between the preview and the real scan, not one:
|
|
boy I hate llm prose so much. I'm ripping out a lot of slop and fixing more things, although I won't be able to test until I get home later this evening. |
|
Lol, sorry about that. I know it's annoying. |
you are not alone hahaha |
|
Seems to be working again now with the latest commit |
The strip preview corrected registration with two global sliders: Offset,
applied to every frame, and Drift, a linear ramp across frame positions.
Neither reaches a single frame that sits off on its own — a splice, a
mis-fired advance, a boundary the detector placed a hair late.
Each tile now carries its own slider, adding a per-frame correction on top
of that ramp:
offset(N) = frame_offset_mm + (N-1) * drift + frame_offsets[N]
An absent key is 0.0, so nothing existing changes. The delta rides
frame_offset_mm, which already slides the nkscan rect and writes the
coolscan3 subframe option, so no backend work was needed. A measured strip
re-addresses an absolute rect and may go either way; a feeder clamps the
total as before, because it cannot back up.
The correction registers one strip rather than the transport, so eject
drops it alongside the frame selection and crops.
Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
Two changes to what a scan leaves on disk. Film that carries a single record reads the same off one plane as off three, so Format gains "TIFF (mono)": one 16-bit grey plane, minisblack, at a third of the size. The plane is the mean of the three, summed in uint32 so nothing overflows and rounded rather than floored. Mono film meters with its channels locked, so the three are one density read three times and their mean is the least noisy of them. Deriving it host-side rather than asking the transport keeps one code path: nkscan offers no grey mode at all. The loader restacks a 2-plane TIFF to three, so NegPy reads its own mono scans back unchanged. DNG output is retired. write_dng_linear and its encoder are gone, and a saved "DNG" preference lands on TIFF, the other 16-bit master — the same mapping RETIRED_EXPORT_FORMATS already applies on the export side. Reading LinearRaw DNGs from SilverFast and VueScan is untouched; only writing them is dropped. TestScanRoundTripParity went with it: it existed to prove a scan TIFF and its DNG twin decode alike, and there is no twin now. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
A per-frame correction reached the scan only while its tile existed. frame_offsets() read the sliders and nothing else, and a measured strip opens with no tiles at all — they appear when the strip is detected. Reopening the strip preview and accepting it without detecting therefore answered with an empty map and wiped every saved correction, so the batch scanned unshifted. A frame with no tile now keeps what was saved; a tile the operator can see overrides it, and zeroing a visible slider still clears it. The arithmetic was right the whole way down — dialog, preview fractions, batch millimetres and the stage rect all agree — so the tests that covered it all passed. They shared one blind spot: every dialog test built a device with an adapter capacity, which is a feeder, whose tiles are built eagerly. Nothing constructed a transport that measures the strip. There is a _discovery_device() helper now, and the end-to-end test drives the real sidebar values rather than a hand-built request. Alongside it, two things the strip preview was missing: Tiles get a size slider (90-340 px, remembered as strip_tile_height). The label letterboxes its kept pixmap at paint time, so resizing costs no scanning. The grid no longer wraps at a fixed six: columns are computed from the width, which is what lets a large tile stay on screen, and the dialog reflows when resized. Every previewed frame gets a thin outline. That box is the boundary the offset is measured from, and without it there was nothing to judge the offset against on a dark scan. It is drawn last so neither the shaded band nor a crop taken to the edge dims it, and it lives in the shared label, so the prescan and quick-scan previews get it too. The clamp warnings now name the frame's own slider, which they could not have caused before it existed. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
Whether a per-frame correction reached the scan was only answerable by reading the written file and the settings blob back. One line per frame says it outright, which is what a report of "the offset did nothing" needs first. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
The offset reaches the scanner, and the delivered file moves the opposite way from the preview. Separating the two halves needs the rect that was actually sent, next to the one discovery reported, which no file or setting records. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
Every per-frame control lives on a tile, and a measured strip had no tiles until the operator pressed Detect frames, so reopening the strip preview to adjust one frame's offset offered nothing to adjust. The search now runs as the dialog opens, once, and only for a transport that measures the strip: previewing a feeder costs a scanner pass per frame, which is not something to start unasked. The rects and the strip pass are cached until the film is ejected and previews are cut from that pass, so only the first open per film costs anything. Detect frames stays, for a film that has moved. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
…ar out The per-frame correction cut exactly what it was given, and still left a clear strip on two frames of a test roll: their boundaries sat 876 and 1339 stage units off the picture, 5.6 mm and 8.5 mm at 4000 dpi, and the control stopped at 2.5 mm. A feeder already reached 10 mm on the same slider, while a measured strip, the transport that can re-address a rect in either direction, was held to a quarter of that. Both span 10 mm now, still far short of the frame pitch, so the offset stays a boundary correction rather than a way to the next frame. Claude-Session: https://claude.ai/code/session_01MVoVDPPMqeh65VwEUCp4m7
0.10 rewrites the USB/SCSI transport's busy handling: it polls a busy unit's phase rather than resending the command, paces the retries, gives a session's first command a warm-up budget, and keeps every READ on a granule boundary. It also fixes low-DPI scans on the LS-5000. The Python API is unchanged. Claude-Session: https://claude.ai/code/session_01XqiQLys9TYeFmJSguFHgmV
A thumbnail column is one line pitch of film, and the pitch is a whole number of stage addresses. Deriving the scale from the rect and the row count gave 41.09 where the pass uses 41, so every tile sat further off the film it names the further down the strip it was, reaching 0.43 mm by the last frame of a six-frame strip. Every frame top a real LS-50 reported divides by 41 exactly. The fake modelled the same wrong formula, so it agreed with the bug; it now lays its strip out the way a unit does. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7
The boundary every offset control is measured from was a grey box at partial alpha, which is hard to pick out against a negative's own low contrast. It is the reference the operator judges the offset against, so it gets the accent colour at full opacity, and stays drawn last so neither the offset band nor a crop taken to the edge dims it. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7
A moved offset slid the pixmap the tile already held under a fixed frame box, so what the operator set the offset against was a simulation of the move rather than the film the scan would take. A measured strip's whole pass is already in memory, so the tile can be re-cut from it instead: 250 ms after any offset slider stops, the frames whose offset no longer matches what is on show are read again at the rect the batch will scan. No scanning, and the debounce keeps a drag to one request. Closing the dialog cancels a pending re-cut, since the timer holds it. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7
…measured selected_frames() and frame_windows() read the live tiles alone, so accepting the dialog before detection had built any returned an empty selection and no crops, wiping what a previous pass had saved. Stopping the preview and pressing Apply was enough to lose the lot. Both now merge against what was saved, the way frame_offsets() already did; a crop cleared on a tile the operator can see still drops. Claude-Session: https://claude.ai/code/session_012UXETEHMjMAqRhQ1sZdNA7
A crop drawn on a measured strip's tile went to the backend as drawn, but the backend's window has y along the feed and x along the sensor, whose high addresses are the top of the tile. A crop over a tile's top-left came back as the frame's bottom-left. The rotated feeder tile already applies that transform, so the measured strip takes the same one; the rotation only decides which raster had to be turned. Verified on an LS-50 over nkscan: the crop matches the full scan's bottom-left quadrant at 0.987 correlation and its own quadrant at -0.16.
A perforation-framed Coolscan positions the film by its frame table and honours that table only as a whole, and the fine scan runs in a session that never measured the strip. nkscan's scan_frame now takes the detected frames back for that, so a shifted or cropped rect lands where the preview shows it instead of being read as an offset into the frame it fell under. Bindings without the argument are called as before. Measured on an LS-50 through the roll session and the batch path: frame 3 at +8 and -8 mm came back whole and aligned with its tile, where before both blacked out past the gate.
nkscan main (0.11.0) drops `positive` from discover_frames, which made our call raise TypeError, and registers a moved or cropped rect with the unit itself. Stop passing `positive` and the `frames=` table (never in an upstream release), and drop the now-unused film_type from detect_frames. Cut strip-preview tiles at Discovery.addresses_per_column instead of optical_dpi / thumbnail_dpi: an LS-50 reports 97 dpi (41 addresses a column) where the film moves about 41.9, so tiles drifted towards the next frame down the strip. Shorten the comments, docstrings and USER_GUIDE prose this branch adds. Claude-Session: https://claude.ai/code/session_01BijGE62D7xF5J3oPwr16FR
7667590 to
55c495e
Compare
|
all good against |
|
Offset works fine now, but I'm still getting mismatch between the preview/thumbnail and the scan. The preview has been perfect for the strips I have scanned so far, with no film base at either side of the frame. But all the scanned frames are shifted somewhat to the right from the preview and shows some black border. So everything looks fine in the preview, but the scan is always slightly off. So setting an offset isn't reliable as you are flying blind. At least on my LS-50. |
|
Thinking about this some more I'm starting to wonder if the SA-21/30 are simply not accurate enough to match the preview with the scan? Nikon scan always overscans and includes film base on both sides of the frame so there is a much bigger margin for error. I think it might be a bit more accurate as well, but there are visible mismatches between thumbnail and scan there as well on some frames. So it is not like it manages to position the film perfectly for every frame either. But the small errors gets absorbed because of the overscan. |
4 participants
Three rounds of work on the strip preview and the scan writer, on top of the nkscan backend.
Per-frame offset
Offset and Drift correct a strip whose gaps creep uniformly. Neither reaches a single frame that sits off on its own — a splice, a mis-fired advance, a boundary the detector placed a hair late. Each tile now carries its own slider, adding a correction on top of that ramp:
An absent key is 0.0, so nothing existing changes behaviour. No backend work was needed: the correction rides
frame_offset_mm, which already slides the nkscan rect and writes the coolscan3subframeoption. A measured strip re-addresses an absolute rect and may go either way; a feeder clamps the total as before, because it cannot back up.The bug this PR also fixes. The correction reached the scan only while its tile existed.
frame_offsets()read the sliders and nothing else, and a measured strip opens with no tiles — they appear when the strip is detected. Reopening the preview and accepting without detecting answered with an empty map and wiped every saved correction, so the batch scanned unshifted. A frame with no tile now keeps what was saved.The arithmetic was correct the whole way down, and the tests covering it all passed. They shared one blind spot: every dialog test built a device with an adapter capacity — a feeder, whose tiles are built eagerly — so nothing ever constructed a transport that measures the strip. There is a
_discovery_device()helper now, and the end-to-end test drives real sidebar values instead of a hand-built request.Tile size, and something to judge the offset against
Tiles get a size slider (90–340 px, remembered). The label letterboxes its kept pixmap at paint time, so resizing costs no scanning. The grid no longer wraps at a fixed six — columns follow the available width, which is what keeps a large tile on screen — and it reflows when the dialog is resized.
Every previewed frame gets a thin outline. That box is the boundary the offset is measured from, and without it there was nothing to judge against on a dark scan. Drawn last, so neither the shaded band nor a crop taken to the edge dims it. It lives in the shared label, so the prescan and quick-scan previews get it too.
Scan output: TIFF only, colour or mono
Film carrying a single record reads the same off one plane as off three, so Format gains TIFF (mono): one 16-bit grey plane at a third of the size. The plane is the mean of the three, summed in uint32 so nothing overflows and rounded rather than floored — mono film meters with its channels locked, so the three are one density read three times. Derived host-side rather than asked of the transport, because nkscan offers no grey mode; the loader restacks a 2-plane TIFF, so NegPy reads its own mono scans back unchanged.
DNG output is retired.
write_dng_linearand its encoder are gone, and a saved"DNG"preference lands on TIFF — the same mappingRETIRED_EXPORT_FORMATSalready applies on the export side. Reading LinearRaw DNGs from SilverFast and VueScan is untouched; only writing them is dropped.Verification
make allgreen: format, lint, ty, and 5417 passed, 13 skipped.The per-frame chain was checked numerically rather than by reading, at every hop. With base 1.0 mm, drift +0.2 mm/frame and −0.7 mm on frame 3 alone, the dialog, the preview request and the batch all produce
1.0, 1.2, 0.7, 1.6, 1.8mm, and the rect handed to nkscan shifts by exactly 110 stage units (0.7 mm at 4000 dpi) on the feed axis only, with the frame's length preserved and its neighbours untouched.Known limitation
The per-tile slider spans ±2.5 mm on every transport, but a feeder clamps any negative total to zero — it cannot back up. Consistent with how Drift already behaves, and the clamp notice says so, but a negative correction is only genuinely useful on a measured strip.