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

Tools: Tune: SRC: Update generator to produce similar as edited #8845

Merged

Conversation

singalsu
Copy link
Collaborator

@singalsu singalsu commented Feb 8, 2024

This patch makes to SRC coefficients tables generator same changes as done in recent git commits.

commit 88fdb10 ("audio: src: rename sample rate converter coefficient folder name") moved coefficients files location to src/audio/src/coefs.

commit 90fef5a ("src_lite: add module") removed include of src.h from upper level due to another src-lite version.

'commit 7f4e6ae ("src: make coefficients constant") added const to struct src_stage.

@singalsu singalsu marked this pull request as ready for review February 8, 2024 10:20
fprintf(fh, '#include <stdint.h>\n');
fprintf(fh,'\n');

fprintf(fh, '/* SRC table */\n');
switch ctype
case 'float'
fprintf(fh, '%s fir_one = 1.0;\n', vtype);
fprintf(fh, 'const %s fir_one = 1.0;\n', vtype);
fprintf(fh, 'struct src_stage src_double_1_1_0_0 = { 0, 0, 1, 1, 1, 1, 1, 0, 1.0, &fir_one };\n');
fprintf(fh, 'struct src_stage src_double_0_0_0_0 = { 0, 0, 0, 0, 0, 0, 0, 0, 0.0, &fir_one };\n');
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about these? Aren't they const now too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, you are right!

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Will need this for v2.9

@lgirdwood lgirdwood added this to the v2.9 milestone Feb 14, 2024
@singalsu singalsu force-pushed the src_tune_recent_changes_to_generator branch from d169c50 to baef046 Compare February 23, 2024 10:27
@singalsu singalsu requested review from lyakh and lgirdwood February 23, 2024 10:28
This patch makes to SRC coefficients tables generator same changes
as done in recent git commits.

commit 88fdb10 ("audio: src: rename sample rate converter
coefficient folder name") moved coefficients files location to
src/audio/src/coefs.

commit 90fef5a ("src_lite: add module") removed include of
src.h from upper level due to another src-lite version.

'commit 7f4e6ae ("src: make coefficients constant")
added const to struct src_stage.

Signed-off-by: Seppo Ingalsuo <[email protected]>
@singalsu singalsu force-pushed the src_tune_recent_changes_to_generator branch from baef046 to e035c9d Compare February 23, 2024 11:11
Copy link
Collaborator

@lyakh lyakh left a comment

Choose a reason for hiding this comment

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

We'll see next time when src headers are updated if anything breaks :-)

fprintf(fh, '\t%d, %d, %d, %d, %d, %d, %d, %d, %f,\n\t%s};\n', ...
src.idm, src.odm, src.num_of_subfilters, ...
src.subfilter_length, src.filter_length, ...
src.blk_in, src.blk_out, src.halfband, ...
src.gain, vfn);
case { 'int16' 'int24' 'int32' }
fprintf(fh, 'struct src_stage %s = {\n', sfn);
fprintf(fh, 'const struct src_stage %s = {\n', sfn);
Copy link
Contributor

Choose a reason for hiding this comment

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

A side comment, instead of datestr(now(), 'yyyy'); I suggest currentYear = datestr(datetime('now'), 'yyyy');

And there's a unused code , please consider removing it

%% Plot to compare
        if 0
                f = linspace(0, fs/2, 1000);
                h1 = freqz(src.coefs/src.L, 1, f, fs);
                h2 = freqz(bq0/sref/src.L, 1, f, fs);
                h3 = freqz(bi/sref/src.L, 1, f, fs);
                figure;
                plot(f, 20*log10(abs(h1)), f, 20*log10(abs(h2)), f, 20*log10(abs(h3)));
                grid on;
                fprintf('Original = %4.1f dB, optimized = %4.1f dB, delta = %4.1f dB\n', ...
                        sb1, sb2, sb1-sb2);
        end

@@ -82,45 +82,44 @@
um_tmp = unique_modes(i,:);
if isequal(um_tmp(1:2),[1 1]) || isequal(um_tmp(1:2),[0 0])
else
fprintf(fh, '#include <%ssrc_%s_%d_%d_%d_%d.h>\n', ...
ppath, prof_ctype, um_tmp(1:4));
fprintf(fh, '#include \"src_%s_%d_%d_%d_%d.h\"\n', ...
Copy link
Contributor

Choose a reason for hiding this comment

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

unused variables are there ppath in the function call, please consider ~

Copy link
Contributor

@ShriramShastry ShriramShastry left a comment

Choose a reason for hiding this comment

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

The changes look good to me; I've added a couple side notes. If you believe that the propose changes are beneficial, please include them; otherwise, the current changes are fine to me.
Thank you!

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

@singalsu can you make incremental fixes for @ShriramShastry comments. Thanks !

@lgirdwood lgirdwood merged commit a9a05e2 into thesofproject:main Feb 23, 2024
40 of 44 checks passed
@singalsu
Copy link
Collaborator Author

@singalsu can you make incremental fixes for @ShriramShastry comments. Thanks !

Sure, @ShriramShastry thanks! I'll fix them later. That "if 0" code might be useful, maybe under some debug option. The ppath is clear to remove after the directory locations change.

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.

4 participants