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

Default value for frame overlap #307

Closed
towsey opened this issue Mar 24, 2020 · 6 comments
Closed

Default value for frame overlap #307

towsey opened this issue Mar 24, 2020 · 6 comments

Comments

@towsey
Copy link
Contributor

towsey commented Mar 24, 2020

The default value for frame/window overlap is currently set at 0.5. (See class SonogramConfig, line 24.)
This should be set = zero (0.0).
The frame step can be set directly or indirectly through windowOverlap. Both means are useful at different times. However currently setting one does not always set the other. In particular, setting the frame step in Generic Profiles does not reset the windowOverlap.
Changing the default windowOVerlap to zero may cause some tests to fail.

@atruskie
Copy link
Member

atruskie commented Mar 24, 2020

I don't think this will have much effect.

We generally set window overlap to 0 almost everywhere. I doubt that default is ever actually used.

Change the default, run the tests, see if there are any failures, and we can see what the right thing to do after that it is.

As for the overlap/frameStep duality: a couple of unit tests for the SonogramConfig class should be enough to ensure they are always set consistently.

@towsey
Copy link
Contributor Author

towsey commented Mar 24, 2020

The default is used which is how I picked up the problem.

@atruskie
Copy link
Member

Where?

@towsey
Copy link
Contributor Author

towsey commented Mar 24, 2020

Line 142 in the same class.

@atruskie
Copy link
Member

atruskie commented Mar 24, 2020

As in:

config.SetPair(ConfigKeys.Windowing.Key_WindowOverlap, DEFAULT_WINDOW_OVERLAP.ToString());

Then yes, the default is set. But we override it almost everywhere. Have you found a case where that default is actually used in real-world code?

towsey added a commit that referenced this issue Mar 24, 2020
Issue #297 THis also deals with Issue #307 Restore line that calculates the overlap given the provided framestep. THis must be done because code depends on the overlap rather than the framestep.
@atruskie
Copy link
Member

@atruskie atruskie closed this as completed Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants