Skip to content

add modal weighting to time-domain nfchoa #77

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

Merged
merged 13 commits into from
Apr 28, 2016
Merged

add modal weighting to time-domain nfchoa #77

merged 13 commits into from
Apr 28, 2016

Conversation

fietew
Copy link
Member

@fietew fietew commented Apr 19, 2016

in order to study the effect of different modal windows on the reproduced sound field

@hagenw
Copy link
Member

hagenw commented Apr 25, 2016

I have corrected a small error and changed a little bit the behavior of the default parameters to be in line with the rest of the Toolbox, hopes this is ok with you.

At the moment the modal_weighting() function has only Input parameters and Output parameters in the comments at the beginning. Have you a good idea what we could add for a more detailed explanation, that maybe could roughly explain what it is doing to someone not familiar with the concept of modal weighting?

For adding a reference I guess we have to wait until you publish some results?

@fietew
Copy link
Member Author

fietew commented Apr 26, 2016

Maybe we should use the varargin functionality of MATLAB instead of assigning ndft to conf. I think it's quite hard to understand, what happens, if only two arguments are given.

I could add some reference for the kaiser window function.

@hagenw
Copy link
Member

hagenw commented Apr 27, 2016

I have updated the parameter handling a little bit, but did not change the behavior to varargin as the current behavior is the standard procedure done in SFS, see this example.

There is a TODO left, could you fix this and add the reference you mentioned, this would be nice.

@hagenw
Copy link
Member

hagenw commented Apr 27, 2016

I added a small description, maybe you can check if it is correct.

Then, I was wondering about the usage of the conf.nfchoa.parameter entry. If I see it correctly, a setting of conf.nfchoa.wtype = 'kaiser' and conf.nfchoa.parameter = 0.0 will be the same as using conf.nfchoa.wtpye = 'rect'.
Are you planing to integrate more windows? Otherwise I would propose to use only the Kaiser-Bessel window and provide just one conf parameter, something like conf.nfchoa.modal_window which can go from 0 to 1.

@fietew
Copy link
Member Author

fietew commented Apr 28, 2016

Yes, I would like to add some more windows in the future. conf.nfchoa.parameter is not that intuitive, but many window types have one parameter, but the meaning of the parameter is quite different. Furthermore, the parameter of the Kaiser window can go from 0.0 to infinity. I will add that to the comments of the SFS_config

@hagenw
Copy link
Member

hagenw commented Apr 28, 2016

OK, then I agree to have the setting in conf.
I'm still not completely happy with the naming, what do you think of the following proposal?
conf.nfchoa.wtype => conf.nfchoa.modal_window
conf.nfchoa.wparameter => conf.modal_window_strengths or conf.modal_window_parameter

@fietew
Copy link
Member Author

fietew commented Apr 28, 2016

I think the window parameter should still have conf.nfchoa. in front. conf.modal_window_strengths is misleading.
If I had to chose, I would use conf.nfchoa.modal_window and conf.nfchoa.modal_window_parameter

@hagenw
Copy link
Member

hagenw commented Apr 28, 2016

Sorry, my fault, I meant conf.nfchoa.modal_window_parameter.
I have changed the code accordingly, could you test if it is still working.

Currently the modal_weighting is only applied in the temporal NFC-HOA driving function. Is it also possible to apply it to the mono-frequent driving function or do we have to change the NFC-HOA structure (as you proposed) before?

@fietew
Copy link
Member Author

fietew commented Apr 28, 2016

Yes, I had a look at the current monochromatic driving functions and it would be quite complicated to add the weightings there. With a new structure, i.e.

  1. compute the circular/spherical basic expansion of the sound field,
  2. compute the circular/spherical harmonics coefficients of the driving functions,
  3. convert it to the loudspeakers driving function,

this would be easier. The modal weights could be applied directly in the second step.

@fietew
Copy link
Member Author

fietew commented Apr 28, 2016

I tested it with a few parameter combination and everything looked fine for me.

@hagenw hagenw merged commit be44b6d into master Apr 28, 2016
@hagenw hagenw deleted the nfchoa_weighting branch April 28, 2016 13:58
hagenw added a commit that referenced this pull request May 13, 2016
* master:
  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()

Conflicts:
	SFS_general/delayline.m
hagenw added a commit that referenced this pull request May 26, 2016
* 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
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.

2 participants