JuliaDSP / MFCC.jl

Mel Frequency Cepstral Coefficients calculation for Julia
BSD 2-Clause "Simplified" License
33 stars 18 forks source link

Negative `lifterexp` for `lifter` #2

Closed WojciechMigda closed 8 years ago

WojciechMigda commented 9 years ago

Shouldn't the case for the negative lifterexp passed to the lifter have the sign of the lifterexp being negated? liftw = [1, (1 + lift/2*sin([1:ncep-1]π/lift))] compared to https://github.com/jameslyons/python_speech_features/blob/master/features/base.py

davidavdav commented 9 years ago

You might very well be right---I remember that negative values have some kind of HTK interpretation, and I must have followed Dan Ellis's rastamat code at the time, but I might have made an error here.

I'll have to look into this.

davidavdav commented 9 years ago

OK, I had a look at the original Octave code from Dan Ellis. You are right that there a L=-lift is used, but the Julia expression is (1 + lift/2*sin(collect(1:ncep-1)π/lift)), and sin(x) is an asymmetric function, so replacing lift by -lift doesn't change the values of the expression. Phew.

But I am happy to reverse the sign of lift if you think that makes the code more clear.

WojciechMigda commented 9 years ago

Right, the signs due to the asymmetricity cancel each other. I began coding in julia only this week and that might be why some statements do not look so obvious, yet. I think that the usual baseline applies - if the code causes problems with reasoning about it after a time off then some annotation of change should be applied. I'll leave the decision open to you.