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 ffmpeg resampling to handle non-standard sample rates #303

Merged
merged 6 commits into from
Aug 8, 2019

Conversation

atsampson
Copy link
Collaborator

@atsampson atsampson commented Jul 31, 2019

This seems to work pretty well - I've tried it with a number of 8fSC captures and it produces better results than running the rest of the decoder at 8fSC. It should simplify future testing as well.

The script i used to test this with a few different input formats is here.

There's probably enough loader code in lddutils to make it worth moving into a module of its own now (lddecode.loaders?).

@simoninns
Copy link
Collaborator

@happycube - can you verify this and pull it in? Doesn't look like a change that is risky

@atsampson atsampson force-pushed the freqconv branch 2 times, most recently from 0d46de9 to 52d0710 Compare August 4, 2019 12:24
When -f is specified, load the input file through ffmpeg using the
aresample filter to convert to 40MHz. This means the rest of
ld-decode can run at its usual sample rate, which produces
better-quality results and makes testing easier in the future.
This avoids a broken-pipe warning from ffmpeg when using -l.

(Strictly this will only work properly in CPython, since other
implementations like PyPy don't guarantee the destructor will actually
be run. Fixing this properly would mean changing the loader interface to
have an explicit close() operation.)
Copy link
Owner

@happycube happycube left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants