New adaptive mode for ntsc3d decoder #522
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This survives from the original version of this code; it looks like the intent was to use the current and previous lines, not the current line twice. Because they're usually very similar, the difference to the result is tiny -- the cross-colour in the S&W zoneplate is slightly better in a couple of pixels.
This matches the other decoders -- qreal isn't intended as a general floating point type; it's Qt's internal type for coordinates (and doesn't even have to be a floating point type depending on how Qt is configured).
- The 1D filter actually has negative gain. - Explain why split2D uses split1D's result. - Explain how split2D's heuristic works more clearly.
The order of declarations now matches the order in which the functions are called.
Nearly all of the decoding steps are operating on a single frame, so they can be a method of FrameBuffer, rather than a method of Comb that takes a FrameBuffer as a parameter. This simplifies the code quite a bit because it doesn't need to say currentFrameBuffer->... all over the place. The code to load two fields into the framebuffer is also made into a method, and the Get* helpers are renamed to get*.
Previously the filter delay was also hardcoded as 2, so perhaps the + 2 was originally intended to compensate for the chroma filter (now disabled).
This simplifies the code a bit later -- e.g. you can subtract the chroma from the composite signal to get the luma, rather than adding them.
This means we can just create two buffers upfront then cycle between them, which is marginally more efficient than copying the object. For 3D decoding in both directions in the future, we can do exactly the same thing with three buffers. Since the object isn't being copied any more, there's no need for its members to be (efficiently) copyable -- so videoParameters and configuration can be const references, and clpbuffer can be a regular array.
The maximum size was 910x525 or 911x526 in different places before.
The constructor is the real win here (because it's just zero-initialisation), but the operators are short enough to benefit too.
There's no need for a wrapper to make this heap-allocated now, so the buffer can just be an array in Comb::FrameBuffer, and we no longer have the frame size hardcoded anywhere.
It's not necessarily optical flow now.
1D isn't very useful for real decoding, but it's handy when debugging the 2D/3D decoders because it's used as the basis for both of them.
atsampson
added
enhancement
ld-decode-tools
3D decoding looks for candidates in the current frame and two fields in either direction, using a similarity heuristic based on the output of the existing 2D decoder. The heuristic is a quality tradeoff: it can produce worse results than the non-adaptive 3D decoder for absolutely still images (because of spurious chroma returned by the 2D decoder, and content that's similar enough to fool the 3D heuristic), but overall the results are much better, particularly for animation and other material with repeated fields/frames.
atsampson
force-pushed
the
ntsc3dchoice2
branch
from
5685e11
to
5c6406e
Compare
|
@Gamnn pointed out that the adaptive checkbox didn't work - fixed. |
simoninns
approved these changes
Aug 4, 2020
There was a problem hiding this comment.
A checked the ld-analyse parts of this and I have no issue with the changes. I'll leave the NTSC filter part to @happycube as he was the real author of it - although I doubt he remembers that far back into the dim and distance past :)
happycube
approved these changes
Aug 4, 2020
|
Indeed - approved and merged. Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
3D decoding looks for candidates in the current frame and two fields in either direction, using a similarity heuristic based on the output of the existing 2D decoder. The heuristic is a quality tradeoff: it can produce worse results than the non-adaptive 3D decoder for absolutely still images, but overall the results are much better, particularly for animation and other material with repeated fields/frames.
This is a much-tidied-up version of the branch that @Gamnn has been testing, so there are a couple of known problems that can fool the 3D heuristic: especially bad results from the 2D decoder (e.g. dotcrawl.ldf, Video Essentials frame 29108), and low-saturation colours (e.g. scarf.ldf, VE frame 36430). Any future improvements to ntsc2d will generally benefit ntsc3d as well.
Most of the commits are refactoring Comb; the 3D stuff is all in the last commit. Performance-wise, ntsc2d is a few percent faster than it was before, and ntsc3d is pretty slow (~10 FPS on one core; it would benefit from some vectorisation).
I'll update the docs for ld-chroma-decoder and ld-analyse once this is merged.