-
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
Remove conf.N setting #90
Conversation
* master: Zero-padding of HRIRs to conf.N before delayline() Update NEWS
@VeraE: I have updated |
I started working on removing @VeraE, @fietew: for @fietew: could you have a look at |
I did some updates on |
Mh, too bad, this does not sound like there is an easy solution. Do you still think it is worse trying to remove |
You are right both approaches are not a good idea. To me it doesn't make sense to only remove conf.N in some parts of the calculation, though. As long as there is one step where impulse responses are shortened the user has to know before how to set conf.N sensibly. I was hoping we could come up with a solution that would not make the code much more complicated or slower. Unfortunately, I couldn't think of a better way to implement this so far. So maybe, we have to drop this completely? |
For the impulse responses part we could still think about if it is a good idea to not include it into But I'm still very interested in getting rid of |
Fiete has just helped me to fix this branch as I managed to mess up the three commits you did before my merge. So your idea would be to have truncating to conf.N at a higher level in the code? We can do that. So I'd leave |
Time-domain driving function for WFS looks fine. I just got one question: why is the impulse response zero-padded by The WFS IIR pre-equalization does not work properly without the zero-padding as the Matlab function I had a look at |
With the As it also seems complicated to remove |
It's an IIR-Filter. You theoretically need an infinite number of zeros. Maybe we could truncate the impulse responses if the power/amplitude of the last coefficient is below a given threshold.
There is no |
2048 is the default of conf.N, which was used before for zeropadding
This was covered by conf.N before
@fietew: And do we have to account for the delay offset of the fractional delay filter for specifying the driving signal length? For the normal WFS case I'm considering now only the maximum delay without possible delay offsets. |
I discussed with @trettberg and we somehow agreed, that keeping |
OK, I agree. |
This resulted from a discussion in #89.
The idea is that the user don't have to specify the desired impulse response link with
conf.N
, but that the Toolbox automatically decides what is the best length.This would be beneficial, especially for new users. The only downside I see at the moment is that the resulting impulse responses could be very long if the original data from the SOFA file is long.