-
Notifications
You must be signed in to change notification settings - Fork 39
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
New Handling of delayline and fractional delay #50
Conversation
Conflicts: SFS_general/general_least_squares.m
Interesting, I guess the goal behind this is to save some time, especially for local WFS? In some driving functions you replaced the old call to Two comments regarding the code:
|
Yes, there is a significant overhead in the delayline procedure especially for the "resample" method. The reasons for splitting the two functions are:
Regarding your comments:
|
OK, I see. One question: as a normal user I don't care and I'm happy that it is faster now. Should we maybe still have the function |
I will merge the two functions |
What is the status of this? % use fractional delays for delay lines
conf.usefracdelay = false; % boolean
conf.fracdelay_method = 'resample'; % string In The |
Actually the indention is correct, as it should point out, which entry of the conf is used, if a particular condition is fulfilled. Any suggestions to make that clearer are welcome.
|
OK, in the meantime I was able to understand the help entry. I will try to update it in order to make it easier to understand. I will wait until you have updated Before testing I would also like to ask a few more questions: 1.) In the old code, there was an 2.) In the old code there was a handling of the case that a common weight, delay is given for more than one channel. This is removed in the new version, will it still work? 3.) The purpose of the new code is still to avoid recursive calling of the |
Regarding your questions: |
Could you make sure that the intention of code is always 4 and not 2 spaces. |
Should I include the MIT license or will you do it during the merge? |
I'm just saying, that it is not fully transparent what MATLAB's The "best" setting for fractional delay filters in the context of sound field synthesis is ongoing research. |
* master: Fix dos line endings Update comments for SSR BRS
OK, I agree and will include all settings and methods for resampling. |
I found the code of |
iseven() is used in time critical delayline(). isodd() is modified as well for symmetrical reasons
end | ||
|
||
li = zeros(Norder,Norder); | ||
for idx=1:N |
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 guess instead of for idx=1:N
you mean for idx=1:Norder
?
As N
is not defined before.
In |
fixed |
Cool, indeed much shorter now ;) |
Maybe this was not so well phrased. Beside this the only questions which remains is if we should ensure backward compatibility with the old |
Seems okay to me |
I found it to messy to include backward compatibility code. In addition, the problem is relatively easy to fix by updating the conf settings. So I only included a |
delayline:
master:
|
OK, perfect, this should be fast enough. And we will remove the check in a few month anyhow. |
* master: Update copyright owner to SFS Toolbox Developers Update README for online documentation Remove wavread and wavwrite (#81) Return delay offset from delayline() (#82) New Handling of delayline and fractional delay (#50) Fix dos line endings Update comments for SSR BRS Update license in missing files Change license to MIT (#80) Make direction_vector() work with matrix => vector add modal weighting to time-domain nfchoa (#77) Fix comment of tapering_window() Add automatic scaling to loudspeaker weights for plotting (#78) Update comment of interpolation() Update NEWS Fix interpolation calling bug in interpolation_ir() Remove debugging parameter Fix normalization for 2D interpolation Further improvements to description of interpolation() Enhance description for interpolation() Conflicts: SFS_general/direction_vector.m
rfactor = delay.resamplingfactor; | ||
a = [1 1 0 0]; | ||
f = [0.0 0.9/rfactor 1/rfactor 1.0]; | ||
b = firpm(delay.resamplingorder,f,a); |
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 don't remember how it got there, but having a fixed order for the filter design independent from the used resampling order is definitely a bad idea. I has to scale with the resampling factor.
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.
Can you provide a fix for this?
Introduces
delayline_write
anddelayline_read
as an alternative todelayline
. This allows to split the handling of fractional delays into a pre- and post-processing stage. Duringdelayline_write
the data may be oversampled and stored with the new sampling rate in order to reuse the oversampled signal for different delays indelayline_read
.A second technique, called the Farrow-Structure, is implemented. It allows
delayline_write
to pre-process the data with a parallel filter structure and stores the outputs in an interleaved manner. Individual delays are applied to the data using the Horner-scheme.TODOs: