Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use an odd number of taps for PAL/NTSC LPFs #424

Merged
merged 2 commits into from
Jan 23, 2020

Conversation

atsampson
Copy link
Collaborator

I'd wondered why the LPF view in ld-analyse didn't quite line up with the other two views - it's because the filters had an even number of taps, so FIRFilter can't compensate for the leftover 0.5-sample delay.

Switch from 6 to 7 taps, and add an assert to FIRFilter to catch this in the future.

@simoninns - this might affect the DoD stuff you're working on; 5 taps would also work if you'd prefer that to 7.

FIRFilter is only intended to work with an odd number of taps -- with an
even number of taps, the output is delayed by half a sample.

Fixing this means ld-analyse's LPF mode is now aligned with the other
two modes, and produces very (very!) slightly better output from
ld-dropout-correct.
@atsampson atsampson added enhancement ld-decode-tools An issue only affecting the ld-decode-tools labels Jan 22, 2020
Copy link
Collaborator

@simoninns simoninns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that explains it :) I did wonder why it was a bit off. Thanks for fixing it - as for the multi-disc tools, they are still very experimental, so I'll roll through them shortly once ld-discmap is ready to roll (the new NTSC and PAL version)

@simoninns simoninns merged commit 80ed075 into happycube:master Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ld-decode-tools An issue only affecting the ld-decode-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants