xiph / speex

Speex voice codec mirror - THIS IS A MIRROR, DEVELOPMENT HAPPENS AT https://gitlab.xiph.org/xiph/speex
https://www.speex.org/
Other
433 stars 158 forks source link

added missing function speex_header_free to libspeex.def #7

Closed vividos closed 6 years ago

vividos commented 7 years ago

libspeex.def is missing the function speex_header_free(), so library users on Visual Studio are not able to free speex_header structures, resulting in a (small) memory leak.

erikd commented 7 years ago

LGTM! 👍

vividos commented 6 years ago

It seems the AppVeyor build isn't configured correctly - it doesn't know where to find the .sln file to build. It may help to point it to speex/win32/VS2008/libspeex.sln, but I don't know if it can build libspeex with Visual Studio 2008.

vividos commented 6 years ago

@erikd what's the process to get this pull request into the repository? The only missing step seems to be that someone checks the Appveyor configuration and specifies the .sln file to build.

erikd commented 6 years ago

Have you looked at the appveyor build failure?

Maybe we should ping @tmatth .

tmatth commented 6 years ago

Thanks, will merge once I figure out what's up with this appveyor build failure.

Normally Speex development happens on the Speex-dev mailing list: http://lists.xiph.org/mailman/listinfo/speex-dev

vividos commented 6 years ago

It seems the AppVeyor build fails since it isn't properly configured. The error message of the AppVeyor build prints:

Specify which project or solution file to use because the folder contains more than one project or solution file.

The speex project could use the .appveyor.yml file from another xiph project, e.g. from opus: https://github.com/xiph/opus/blob/master/.appveyor.yml

tmatth commented 6 years ago

@vividos yes, I'm assuming that appveyor works with vs2008 .sln files? That's the most recent one we have in speex.

vividos commented 6 years ago

Visual Studio 2008 Community Edition is supported by AppVeyor, but I saw PR #8, and it seems the build there is not completing successfully.

vividos commented 6 years ago

The patch contained in this PR was recently added to master in commit 0c02121, so I guess this PR can be closed.