-
Notifications
You must be signed in to change notification settings - Fork 47
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
Test-case: Add to check-alsabat.sh additional audio test #1016
base: main
Are you sure you want to change the base?
Conversation
019c165
to
a34f96e
Compare
tools/check_wav_file_snr.m
Outdated
%wf = 'bat.wav.3UiPoZ'; % good | ||
%fn = fullfile(fp, wf); | ||
|
||
param.t_ignore_from_start = 30e-3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be possible to enter the ignorefrom begin and end times from command line. And the test criteria should be printed to see them in log.
y = abs(filter(b, a, x)); | ||
z = y(i1:i2); | ||
thr = mean(z) + std(z); | ||
[~, locs]= findpeaks(z, 'MinPeakHeight', thr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this criteria with thr too easily claims some analog noise is periodic glitch. Should improve this to not unnecessarily report the finding. On the other hand finds quite well a periodic buffer issue.
a34f96e
to
8dc6052
Compare
The new output text and graphics can be seen in sof-ci/jenkins Details link. I'm now trying to condense the graphics to combine all channels info into single picture. A successful run would produce one spectrogram plot and one levels plot. If glitches there's a glitch zoom plot per channel for first seen glitch. I hope to have it done by tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you !!
Initial review notes were added. I will go over the algorithm and detection algorithms again.
8dc6052
to
9062b59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a few more suggestions and shared my thoughts on the "findpeaks()
" method; please take a look. I'll keep assessing the code.
Much obliged
z = y(i1:i2); | ||
thr = mean(z) + std(z); | ||
[~, locs]= findpeaks(z, 'MinPeakHeight', thr); | ||
dlocs = diff(locs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to get rid of the location difference, think about using the following code.
dlocs = ismember(1:size(z,1),locs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find differential lot easier to understand. But I'll try and consider.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more ideas on the check_glitch_periodicity
function have been added.
Thanks
y = abs(filter(b, a, x)); | ||
z = y(i1:i2); | ||
thr = mean(z) + std(z); | ||
[~, locs]= findpeaks(z, 'MinPeakHeight', thr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is acceptable to have pks along with locs because you can use the pks to obtain the outliners.
|
||
i1 = round(param.fs * param.t_ignore_from_start); | ||
i2 = length(x) - round(param.fs * param.t_ignore_from_end); | ||
[b, a] = butter(2, 0.95, 'high'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because you have a high pass design that looks for peaks with minHeight
, do you believe the transfer function coefficients of a 2nd-order highpass digital Butterworth filter butter
with a normalised cutoff frequency of 0.95 should be placed after the findpeaks
function? This indicates you've discovered your peaks and have applied a filter
to each one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the alsabat test signal is always a low frequency sine wave, the high-pass filter does a good job in attenuating the test signal, only glitches appear at high level after that. It could be a notch too for the test frequency, but the low order high-pass converges faster. I didn't much experiment with 0.95 vs other cut-off.
The periodicity reporter still gets quite confused from noise, this part is not working well yet. The text should not be printed if the result is not reliable. Maybe I could disable this for for now until there's a better way.
9062b59
to
b416aad
Compare
I changed the SNR, signal, noise plot appearance to this The glitch find/plot can now also find glitches during gain ramp with added time domain glitch detect detect. It's not as good as FFT for tiny glitches but it can find coarse ones like the error in thesofproject/sof#7298 . |
b815bfa
to
dc3579b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it is assumed based on the software one chooses, I added a review remark on epsilon, emphasising the difference between Octave and Matlab interpretation.
dc3579b
to
e2aabb4
Compare
This patch adds run of check_wav_file_snr.m script for the alsabat capture file to analyze with STFT spectra the audio quality. A drop in signal-to-noise ratio in a single FFT indicates presence of glitch. A low SNR result triggers run of additional glitch periodicity finder. Signed-off-by: Seppo Ingalsuo <[email protected]>
e2aabb4
to
9bf202f
Compare
One more push, fixed a green failure with TGLU_RVP_NOCODEC_IPC4ZPH / sh-tglu-rvp-nocodec-ci-02 https://sof-ci.01.org/softestpr/PR1016/build251/devicetest/index.html, there was a glitch plotted but script didn't return error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we make this new check optional at first? Then we could adjust the test plans to run check-alsabat twice: once in the old and lax mode, then a second time with this new and stricter check. This will give us a transition phase where the old and lax mode will pass and the new stricter mode will not yet in many configurations. The old mode will catch regressions while the new mode will show progressions.
If agreed then the easiest way to implement these two modes is probably to run the new octave check only when the user passes -q SNR
.
An invalid 0dB SNR default value can detect when then user passes -q.
This patch adds run of check_wav_file_snr.m script for alsabat capture file to analyze with STFT spectra the audio quality. Dropped signal-to-noise ratio in a single FFT indicates presence of glitch. A low SNR result triggers run of additional glitch periodicity finder.