Is POSIX API necessary? about audacity-noise-reduction HOT 30 CLOSED

You're right - I originally added it to speed up read performance and it was just really easy to do it this way, but there isn't a strong justification for that.

sf_open_virtual is for implementing a callback-based io (similar to what you had implemented - should've mentioned it but it has been a long while since I've touched it), which uses mmap to do the IO.

This was a lot faster than the built-in sf_open when you're things sample-by-sample instead of chunks (which audacity did), which induces lots of read(2) calls.
Better fixes would've been to implement a buffered reader, or to modify the audacity code to work in chunks.

some possible solutions:

• take a performance hit and just return to sf_open()
• implement a buffered reader
• add buffering the audacity code
• only use the mmap version of the reader when you're under linux, otherwise revert back to sf_open (would be slow on windows, but maybe thats acceptable for your situation)

btw, thank you for all those improvements you've been contributing. That's awesome :)

from audacity-noise-reduction.

I think the last approach is probably the easiest one, I will try that out. I would compile it with Visual Studio under Windows. It should be possible to create a Makefile project in Visual Studio. If I need to make some changes to the Makefile, should I also commit an alternative Makefile for Windows?

You're welcome, it is a fun project 👍🏻 Let me know if you need help with some other projects :)

from audacity-noise-reduction.

I dont think the Makefile will work on windows, actually 😅

You can use CMake or Bazel to have a cross-platform build. They're both a little weird but not too hard to use.
They also have a better story around conditional things so you wouldn't need to duplicate stuff like a Makefile

from audacity-noise-reduction.

There's a couple of options :)

• If you want to pick up the external library (like the Makefile does), you can use find_library + target_link_libraries commands so its not too bad.
• git submodule
• The "modern" way, using a package manager like Hunter or Conan (I've only used Hunter, and I remember it being pretty decent).

from audacity-noise-reduction.

yeah its common to have multiple targets :)

you'd create a (static) library (with add_library) and link it once to the executable, and again to the tests

from audacity-noise-reduction.

I'd leave it out.

I wouldn't try to port the makefile because it is hairy and doesn't add much value :)

from audacity-noise-reduction.

it's the same as -I. You can usually get away with specifying the relative path, as you've said, but not always.

For example maybe the relevant header is generated during build time (RPC frameworks like to do this), so the location where the header is going to end up isn't known ahead of time.
Sometimes people use it for including external libraries instead of a find_library like system, which is what the Makefile did.

I think there's other stuff but thats off the top of my head :)

from audacity-noise-reduction.

I think it might be an artifact of the abstraction layers - you have multiple types of targets (executable and various libs), and you want to link stuff to them.
The linking mechanism is unified, so no matter the target type, you get to use the same machinery.
And if you're linking stuff, it makes sense to expose these settings to the user, but when you put everything together, you get oddballs like the ones you've described.

There's probably some esoteric use cases for it, but I think the real reason it's there is because it fit their design better.

from audacity-noise-reduction.

yup those all have a bunch of header dirs as a first class concept (like /usr/include/), so tools tend to pick up on those.

It's a little hard to understand from the cmake docs, but apparently find_library doesn't hook up the include path?
https://gitlab.kitware.com/cmake/community/-/wikis/doc/tutorials/How-To-Find-Libraries#using-external-libraries

from audacity-noise-reduction.

Definitely I learn a lot about compilers by fixing it for Windows :D It seems to me that clang is far more tolerant than msvc. E.g., msvc requires stdexcept include and definition of M_PI, this is implicit in clang.

from audacity-noise-reduction.

You could overflow the stack sure, but you can overflow lots of things :)

In general you don't want to allocate a ton there because the default sizes aren't huge (like 512kb on mac/linux and 1mb in Windows. you can request that your thread has more stack size).

But just for the technical "allocation" aspect of the stack, you don't need to know the size ahead of time since you're just using a pointer to manage a preallocated slab of memory

from audacity-noise-reduction.

Yeah. On Windows a lib can sometimes just contain dynamic linking information, providing little value.
You can tell that is the case by just examining the size of the lib file and seeing that it’s quite small (you can splunk into it with objdump and its ilk).

from audacity-noise-reduction.

I think nowadays it just tells it what symbols to expect in the dll and what its name is.
You can totally just generate that from the dll, but for some reason its typically done like this in Windows.
I wouldn't be surprised if there's some historical justification for it :)

from audacity-noise-reduction.

Okay, thanks 👍

I think I am understanding now. The .lib is necessary for the linker to resolve the external symbols, but the actual implementation is loaded at runtime from a dll.

I have now created a hack to just copy the dll to the executable dir with a batch file, better than doing it by hand at least :D

from audacity-noise-reduction.

Okay, nice, I will look into CMake. I have a heard from it a lot, I think it is very popular in the C++ community, right?

from audacity-noise-reduction.

CMake is popular, but it's also a pretty prickly system/language so it's not very pleasant to use.
We ended up using it in my last job.

This (pretty cool) project uses both: https://github.com/yse/easy_profiler
so its a good reference point. I find it easier to learn from this than from the official docs

from audacity-noise-reduction.

Hey @tals ,

I would like to finally begin to add CMake support to the project :)

I am wondering about how to deal with the libsndfile library. I basically see two options. The first is to do it as currently done in the Makefile, i.e. linking to an already installed library. The other option would be to include the source of libsndfile, probably add it as a git submodule. The repository contains a CMake file so this should be easy. It seems to me that linking to an external library is not super straight forward with CMake, often you have to write your own find package scripts, am I correct?

What option would you prefer?

from audacity-noise-reduction.

I have now decided for the first method. You are right, it is actually very simple once you know the right commands ;)

I have realized you have two targets in the Makefile, one for the noisereduction driver and one for the tests.
Is it common to add multiple targets for CMake as well or is there some other best practice?
Or is actually the target you pass to the command add_executable the same as the targets in the generated Makefile?

Thanks in advance :)

from audacity-noise-reduction.

Okay, thank you :)

And then you would define the name of the exectuable with set_target_properties, am I correct?

from audacity-noise-reduction.

i think it just defaults to the target name

from audacity-noise-reduction.

I do not really understand what the dist target is used for in the current Makefile. Is it necessary or can I just leave it out in CMake?

from audacity-noise-reduction.

I have another question about CMake?

What is the purpose of include_directories, if the include directory is part of the same project? I have seen it in easy_profiler and I wonder because can't you just include the header files by specifying the relative path in the #include directive?

from audacity-noise-reduction.

Another thing about CMake which i wondered: Are there scenarios where it makes sense to link libraries to an executable publicly? If you link library A to library B publicly, I understand that the interface of library B is propagated to library A. But what would it mean for an executable to have a library propagated to its interface?

from audacity-noise-reduction.

Thank you, I understand :)

from audacity-noise-reduction.

Is the flag -Wextra important? It does not exist in the MSVC compiler which is used by the default generator on Windows.

from audacity-noise-reduction.

you can just remove it, but in general you can gate those things by platform

from audacity-noise-reduction.

Okay, thanks.

Another think I wondered: On Windows, I need to add the include directory of libsndfile, for Linux it worked without (and I guess on Mac as well, because the include dir is not specified in the Makefile). Do you know why it works? Do Linux and Mac just add the include dir to the default include dirs automatically when installing the library?

from audacity-noise-reduction.

I am kind of surprised, that clang even allows to use variable number arguments for stack-allocated arrays :D

As e.g. done in

Couldn't this easily lead to stack overflows? It seems paradox to me to allocate something on the stack if the size is not known at compile time.

from audacity-noise-reduction.

I have almost finished, but have a very strange error still.

It compiles fine, but when I run it under Windows, it tells me "cannot find libsndfile-1.dll". This is strange, because in the CMake build process it definitely finds the file libsndfile-1.lib. So I wonder why it is even looking for a dll at runtime.

Have you heard of a similar issue once?

from audacity-noise-reduction.

Thank you very much :) This should be the reason, the lib only has 10 kb while the dll has 1.7 mb. I am starting to understand why many developers do not like using Windows, the library and package management seems to be much easier on Unix-based systems :D

But then, what is even the purpose of the .lib if you have to link the .dll anyway?

from audacity-noise-reduction.