dpwe / solafs

Audio Time Scale Modification via Synchronous OverLap-Add with Fixed Synthesis hop
MIT License
8 stars 2 forks source link

"Hop size" and "overlap size" are used interchangeably #2

Open MachFour opened 3 years ago

MachFour commented 3 years ago

Hi, thank you for your SOLAFS implementation.

While looking through the code, I noticed that the hop time variable in the main function is multiplied by the audio sample rate and then passed as the 'overlap size' argument of the solafs function.

But, aren't hop size and overlap size complementary values? I.e. overlap size = window size - hop size. So I think either 'hop' should be renamed to 'overlap' in the main function, or the above conversion should be done.

Happy to make the changes and submit a pull request if you agree with my reasoning. Thanks.

dpwe commented 3 years ago

You’re completely right. I think the right argument to the solafs should be hop, I find overlap a confusing way to parameterize it. But it looks like overlap is useful in the function, so I guess we should immediately convert hop to overlap at the top of the function. If you submit a pull request, I’d be very glad to incorporate it.

By the way, I have absolutely no memory of writing this code!

DAn.

On Wed, Mar 3, 2021 at 19:00 Max notifications@github.com wrote:

Hi, thank you for your SOLAFS implementation.

While looking through the code, I noticed that the hop time variable in the main function is multiplied by the audio sample rate and then passed as the 'overlap size' argument of the solafs function.

But, aren't hop size and overlap size complementary values? I.e. overlap size = window size - hop size. So I think either 'hop' should be renamed to 'overlap' in the main function, or the above conversion should be done.

Happy to make the changes and submit a pull request if you agree with my reasoning. Thanks.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dpwe/solafs/issues/2, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEGZUMMZJTQXL7HBQWYZ4LTB3EQBANCNFSM4YSGM5KA .