|
From: justnope <spa...@te...> - 2019-01-29 03:32:26
Attachments:
msvc.patch
|
Hi, This is a first attempt to compile the library with msvc. The changes are: * ssize_t -> size_t * vasprintf -> vsnprintf, malloc, sprintf * removed unavailable headers * added cmake file To compile it, use cmake <source dir>. This will generate a visual studio solution file which you can open and build the project. In the cmake file I've added the option to make all symbols in the library public (or whatever the right term is), so it behaves like gcc. This avoids polluting the source code with DLL_EXPORT defines (__declspec(dllexport/dllimport)). It's possible I missed some things which configure generates. I'll add those to CMakeLists.txt in the future. Currently it only builds the library and not the tools. I'm hoping to add those later. There's also a message in the source code "type_info::raw_name() demangling has not been tested yet with Microsoft compiler! Feedback appreciated!" I'm working on lmms porting the code to msvc and to be honest I'm not that familiar with libgig, but if you let me know what triggers that code, I'll gladly try it out. |
|
From: justnope <spa...@te...> - 2019-01-29 03:32:40
Attachments:
msvc.patch
|
Hi, This is a first attempt to compile the library with msvc. The changes are: * ssize_t -> size_t * vasprintf -> vsnprintf, malloc, sprintf * removed unavailable headers * added cmake file To compile it, use cmake <source dir>. This will generate a visual studio solution file which you can open and build the project. In the cmake file I've added the option to make all symbols in the library public (or whatever the right term is), so it behaves like gcc. This avoids polluting the source code with DLL_EXPORT defines (__declspec(dllexport/dllimport)). It's possible I missed some things which configure generates. I'll add those to CMakeLists.txt in the future. Currently it only builds the library and not the tools. I'm hoping to add those later. There's also a message in the source code "type_info::raw_name() demangling has not been tested yet with Microsoft compiler! Feedback appreciated!" I'm working on lmms porting the code to msvc and to be honest I'm not that familiar with libgig, but if you let me know what triggers that code, I'll gladly try it out. |
|
From: justnope <spa...@te...> - 2019-02-11 13:49:48
Attachments:
msvc.patch
|
Hi, Could you check the attached patch for possible mistakes or things you'd like to see implemented differently? About the directory functions (opendir,...), I'm currently also porting zynaddsubfx to msvc and have a partial implementation ready. As soon as it's complete, tested and approved, I'll copy the code into libgig and send you a patch. |
|
From: Christian S. <sch...@li...> - 2019-02-12 16:37:12
|
On Montag, 11. Februar 2019 14:49:22 CET justnope wrote: > Could you check the attached patch for possible mistakes or things you'd > like to see implemented differently? Looks fine to me. Just one last thing: You added CMake files and I wonder what's better, placing those CMake files directly at the individual locations like you did, or rather putting them into a dedicated subdir like "msvc" altogether. The thing is, obviously right now we do not support cmake on Linux etc., so it is not a real issue yet. But one day somebody will ask for compiling libgig with cmake on other platforms as well. Obviously the best way would be sharing the CMake files among all platforms and just wrapping the architecture/compiler specific portions with conditions. Would that be easily possible? Last time I looked into this with cmake I think there were some substantial limitations, but I can't remember anymore what exactly. CU Christian |
|
From: justnope <spa...@te...> - 2019-02-13 17:58:37
Attachments:
msvc.patch
|
I found a mistake which resulted in a compile error when I tried it with gcc, added an install option in the cmake file and split it into libgig & libakai so it's the same as the makefile. On 12/02/2019 17:37, Christian Schoenebeck wrote: > Looks fine to me. Just one last thing: You added CMake files and I wonder what's > better, placing those CMake files directly at the individual locations like you > did, or rather putting them into a dedicated subdir like "msvc" altogether. > The thing is, obviously right now we do not support cmake on Linux etc., so it > is not a real issue yet. But one day somebody will ask for compiling libgig > with cmake on other platforms as well. It's very easy to put it in a separate dir, let me know if you prefer that. > Obviously the best way would be sharing the CMake files among all platforms and > just wrapping the architecture/compiler specific portions with conditions. > Would that be easily possible? Last time I looked into this with cmake I think > there were some substantial limitations, but I can't remember anymore what > exactly. I also work on lmms and there we compile for windows, mac, linux. For windows we can cross-compile with mingw or use msvc directly. This is done with shared cmake files for all combinations. If there's a limitation, I haven't encountered it so far. It's easy to do with conditionals in cmake. What's more tricky is using the right conditionals. For example, you have to make a distinction between platform and compiler. So if the variable WIN32 is defined you can't assume it's msvc that's being used since we can also cross-compile. There's also several combinations possible of which systems you want to support for finding the libraries you link to (pkg-config, cmake config,...) . You have very few dependencies so that won't be a problem. It depends which combinations you want to support and how far you are prepared to go into the rabbit hole. |
|
From: Christian S. <sch...@li...> - 2019-02-14 16:04:00
|
On Mittwoch, 13. Februar 2019 18:58:25 CET justnope wrote: > I also work on lmms and there we compile for windows, mac, linux. For > windows we can cross-compile with mingw or use msvc directly. This is > done with shared cmake files for all combinations. If there's a > limitation, I haven't encountered it so far. Ok, then just leave the cmake files at their locations as you did. Then later on we try to make them shared ones for other architectures as well. CU Christian |
|
From: justnope <spa...@te...> - 2019-02-15 14:05:03
|
On 14/02/2019 17:03, Christian Schoenebeck wrote: > Ok, then just leave the cmake files at their locations as you did. Then later > on we try to make them shared ones for other architectures as well. I can adapt the cmake files to build for other systems now if you'd like. This means I'll have to duplicate what's in the configure files. If that's ok by you, I don't mind putting some effort into it. Give me a list of what's done in configure and I'll replicate it in cmake. I know for starters I have to configure the .in files. I can test on ubuntu and gentoo, if you have other distros to test on that would be handy. |
|
From: Christian S. <sch...@li...> - 2019-02-15 14:15:35
|
On Freitag, 15. Februar 2019 15:04:47 CET justnope wrote: > On 14/02/2019 17:03, Christian Schoenebeck wrote: > > Ok, then just leave the cmake files at their locations as you did. Then > > later on we try to make them shared ones for other architectures as well. > I can adapt the cmake files to build for other systems now if you'd > like. This means I'll have to duplicate what's in the configure files. > If that's ok by you, I don't mind putting some effort into it. Give me a > list of what's done in configure and I'll replicate it in cmake. I know > for starters I have to configure the .in files. I can test on ubuntu and > gentoo, if you have other distros to test on that would be handy. I suggest that I commit your msvc patch first, and then sure, if you like to adapt the cmakes for other architectures, very much appreciated of course! So your previous patch sent to the list is the latest one, or should I wait for an updated patch from your side? CU Christian |
|
From: justnope <spa...@te...> - 2019-02-15 15:00:38
|
On 15/02/2019 15:15, Christian Schoenebeck wrote: > I suggest that I commit your msvc patch first, and then sure, if you like to > adapt the cmakes for other architectures, very much appreciated of course! > > So your previous patch sent to the list is the latest one, or should I wait > for an updated patch from your side? um... wait for a bit? I tried to integrate it in vcpkg and noticed some problems. I know, I should've done this earlier! They are minor changes. But once I'm done and you've released a new version, I can use vcpkg cmake magic to download the source tarball, extract, compile and easily include it in my projects. |
|
From: justnope <spa...@te...> - 2019-02-18 15:09:27
Attachments:
msvc.patch
|
On 15/02/2019 16:00, justnope wrote: > On 15/02/2019 15:15, Christian Schoenebeck wrote: >> I suggest that I commit your msvc patch first, and then sure, if you >> like to >> adapt the cmakes for other architectures, very much appreciated of >> course! >> >> So your previous patch sent to the list is the latest one, or should I >> wait >> for an updated patch from your side? > um... wait for a bit? > I tried to integrate it in vcpkg and noticed some problems. I know, I > should've done this earlier! This is the final version. I did several tests and they all seem to work. If you remove the MSVC check in CMakeLists.txt it will also compile the library and some tools in linux. |
|
From: Christian S. <sch...@li...> - 2019-02-19 19:22:56
|
On Montag, 18. Februar 2019 16:09:11 CET justnope wrote: > This is the final version. I did several tests and they all seem to work. > > If you remove the MSVC check in CMakeLists.txt it will also compile the > library and some tools in linux. I'm currently processing Ivan's patch, but I am optimistic to have both patches committed in the next two days. CU Christian |
|
From: Christian S. <sch...@li...> - 2019-02-20 19:14:55
|
On Dienstag, 19. Februar 2019 20:22:42 CET Christian Schoenebeck wrote: > On Montag, 18. Februar 2019 16:09:11 CET justnope wrote: > > This is the final version. I did several tests and they all seem to work. > > > > If you remove the MSVC check in CMakeLists.txt it will also compile the > > library and some tools in linux. > > I'm currently processing Ivan's patch, but I am optimistic to have both > patches committed in the next two days. I just committed your MSVC changes (SVN r3476). Thanks for your patch! CU Christian |
|
From: Christian S. <sch...@li...> - 2019-01-31 14:51:39
|
On Dienstag, 29. Januar 2019 04:14:41 CET justnope wrote: > Hi, Hi! > This is a first attempt to compile the library with msvc. > The changes are: > * ssize_t -> size_t size_t is an unsigned type, whereas ssize_t is signed. So these two types are used in the code for purpose and simply replacing one with the other would break things, especially replacing ssize_t by size_t is not a good idea. It would make more sense to find an equivalent type available with msvc and to add an appropriate typedef for msvc instead. > * vasprintf -> vsnprintf, malloc, sprintf If vasprintf() is missing then it would be preferable to add an implementation of vasprintf() for msvc (e.g. in helper.h/.cpp) instead of bloating all the calling code locations in RIFF.cpp and Serialization.cpp. > #if defined(WIN32) && !HAVE_CONFIG_H > +#if _MSC_VER > +//PACKAGE and VERSION are defined using the cmake file > +#else > > # include "../win32/libgig_private.h" // like config.h, automatically > generated by Dev-C++ # define PACKAGE "libgig" > # define VERSION VER_STRING // VER_STRING defined in libgig_private.h > > +#endif // _MSC_VER > #endif // WIN32 Well, you know the problem here is who would update all the individual version numbers at separated places, for each combination of platform and build tool on every release. I rather would like to reduce the number of places where we need to bump version numbers instead of adding yet another one. Does msvc not ship with autoconf at all? If not, maybe a small script triggered by the msvc project or by cmake might be an alternative that would grep the latest version number from configure.ac? > * added cmake file > > To compile it, use cmake <source dir>. This will generate a visual > studio solution file which you can open and build the project. In the > cmake file I've added the option to make all symbols in the library > public (or whatever the right term is), so it behaves like gcc. This > avoids polluting the source code with DLL_EXPORT defines > (__declspec(dllexport/dllimport)). The term is "exporting" symbols. And what you did is adding a compiler option which exports all symbols automatically to the generated DLL. Essentially exporting means that e.g. functions and global variables are written to a table in the DLL file. The table contains the symbol name (function name, variable name) and how it can be found (address) by applications using the DLL. Usually one would pick a compiler option to export only global symbols, not all symbols. If you pick all symbols it would also export private (static), implementation internal functions and variables, not meant to be accessed from outside. > There's also a message in the source code "type_info::raw_name() > demangling has not been tested yet with Microsoft compiler! Feedback > appreciated!" I see that you actually disabled demangling completely for msvc. I am not sure, but isn't there an alternative function for mscv that's called undecorate symbols "unDec" or something? At least that's what I found somebody suggested on the net. CU Christian |
|
From: justnope <spa...@te...> - 2019-02-02 01:53:32
|
> size_t is an unsigned type, whereas ssize_t is signed. So these two types are > used in the code for purpose and simply replacing one with the other would > break things, especially replacing ssize_t by size_t is not a good idea. > > It would make more sense to find an equivalent type available with msvc and to > add an appropriate typedef for msvc instead. Or if c++11 is acceptable I could just use auto. >> * vasprintf -> vsnprintf, malloc, sprintf > > If vasprintf() is missing then it would be preferable to add an implementation > of vasprintf() for msvc (e.g. in helper.h/.cpp) instead of bloating all the > calling code locations in RIFF.cpp and Serialization.cpp. My bad, I noticed that there was already a replacement for vasprintf in the code. It wasn't included because the preprocessor #if wasn't triggered. > Does msvc not ship with autoconf at all? If not, maybe a small script > triggered by the msvc project or by cmake might be an alternative that would > grep the latest version number from configure.ac? I added this to the cmake file. It now gets the version info from configure.ac. > I see that you actually disabled demangling completely for msvc. I am not > sure, but isn't there an alternative function for mscv that's called > undecorate symbols "unDec" or something? At least that's what I found somebody > suggested on the net. I've disabled it temporarily. I did some searching and I think I can use dbghelp.dll it's supposed to be included in every version of windows. Another option was to use boost, I thought it included a demangler. The advantage is that it is cross platform, but maybe it's a bit too heavy handed. I don't know what your thoughts are on using boost. If it's ok, I've made a project on github to track the changes I made: https://github.com/justnope/libgig It's easier for me to work on and maybe more convenient than to send the patches each time. If rather receive the patches each time, let me know. It's currently a work in progress, but I rather receive criticism early on than potentially having to redo things. |
|
From: Christian S. <sch...@li...> - 2019-02-03 15:30:53
|
On Saturday, 2. Februar 2019 02:53:14 CET justnope wrote: > > size_t is an unsigned type, whereas ssize_t is signed. So these two types > > are used in the code for purpose and simply replacing one with the other > > would break things, especially replacing ssize_t by size_t is not a good > > idea. > > > > It would make more sense to find an equivalent type available with msvc > > and to add an appropriate typedef for msvc instead. > > Or if c++11 is acceptable I could just use auto. Adding a C++11 dependency just for this minor issue is really not neccessary. I just googled "ssize_t windows" and the solution seems to be: #if defined(_MSC_VER) #include <BaseTsd.h> typedef SSIZE_T ssize_t; #endif As suggested here: https://stackoverflow.com/questions/22265610/why-ssize-t-in-visual-studio-2010-is-defined-as-unsigned And here is a list of Microsofts's predeclared data types, which also lists SSIZE_T along with its precise definition and associated header file on Windows: https://docs.microsoft.com/en-us/windows/desktop/winprog/windows-data-types However maybe you have to fine tune that solution above by also checking the MSC version, because as far as I can see it, in previous MSVC versions there used to be POSIX header files where ssize_t had been declared on Windows, which was later removed, and I don't know if SSIZE_T was always there. > > Does msvc not ship with autoconf at all? If not, maybe a small script > > triggered by the msvc project or by cmake might be an alternative that > > would grep the latest version number from configure.ac? > > I added this to the cmake file. It now gets the version info from > configure.ac. Great! One place less to care about version info! > > I see that you actually disabled demangling completely for msvc. I am not > > sure, but isn't there an alternative function for mscv that's called > > undecorate symbols "unDec" or something? At least that's what I found > > somebody suggested on the net. > > I've disabled it temporarily. I did some searching and I think I can use > dbghelp.dll it's supposed to be included in every version of windows. > > Another option was to use boost, I thought it included a demangler. The > advantage is that it is cross platform, but maybe it's a bit too heavy > handed. I don't know what your thoughts are on using boost. As you already assumed, I would like to avoid new dependencies as much as possible. > If it's ok, I've made a project on github to track the changes I made: > https://github.com/justnope/libgig > It's easier for me to work on and maybe more convenient than to send the > patches each time. If rather receive the patches each time, let me know. Well, once your changes are ready to be committed on our side, I would appreciate if you'd just send one patch with your changes or a link to that patch. CU Christian |
|
From: justnope <spa...@te...> - 2019-02-04 04:34:14
Attachments:
leak.patch
|
Hi, This patch has nothing directly to do with the msvc development. I noticed it when I was trying out the GigWriteTest. Depending on the constructor, RIFF:File can be allocated by the constructor or passed on. In case DLS::File allocates RIFF::File itself, it never gets destroyed. This results on windows in an open file handle and the next test case can't properly open the file. I'm not that familiar with the code base so feel free to correct if I got anything wrong. I've made the patch with git, let me know if there are problems applying it. (used --no-prefix) |
|
From: Christian S. <sch...@li...> - 2019-02-05 12:40:31
|
On Montag, 4. Februar 2019 05:34:07 CET justnope wrote: > This patch has nothing directly to do with the msvc development. I > noticed it when I was trying out the GigWriteTest. > > Depending on the constructor, RIFF:File can be allocated by the > constructor or passed on. In case DLS::File allocates RIFF::File itself, > it never gets destroyed. This results on windows in an open file handle > and the next test case can't properly open the file. Yes, that makes sense. Would you also share your real name so I can credit you for your patch? CU Christian |
|
From: justnope <spa...@te...> - 2019-02-08 00:04:12
|
> Yes, that makes sense. Would you also share your real name so I can credit you > for your patch? I'm fine with being uncredited or using justnope. Another thing I noticed but I'm unable to test is that in Serialization.cpp the return value of abi::__cxa_demangle isn't freed. On 03/02/2019 16:30, Christian Schoenebeck wrote: >> Or if c++11 is acceptable I could just use auto. > > Adding a C++11 dependency just for this minor issue is really not neccessary. > I just googled "ssize_t windows" and the solution seems to be: I've used the typedef and a check for msvc 2015 or higher. If it's a lower version an preprocessor error gets displayed. Installing 2013 only to check if it works seems overkill. >>> I see that you actually disabled demangling completely for msvc. I am not >>> sure, but isn't there an alternative function for mscv that's called >>> undecorate symbols "unDec" or something? At least that's what I found >>> somebody suggested on the net. >> >> I've disabled it temporarily. I did some searching and I think I can use >> dbghelp.dll it's supposed to be included in every version of windows. >> >> Another option was to use boost, I thought it included a demangler. The >> advantage is that it is cross platform, but maybe it's a bit too heavy >> handed. I don't know what your thoughts are on using boost. > > As you already assumed, I would like to avoid new dependencies as much as possible. I've implemented demangling and it seems to work. I've used dbghelp.dll. It's also possible to use an undocumented crt function, but I rather add a dependency on dbghelp (which is always installed on windows) than using that undocumented crt function. When you use raw_name it gives you a string that you can demangle but it adds a "." in the beginning of the string which you must skip. The question I have is that in the documentation it states that function I call to demangle is not thread safe. So the question I have is: can DataType::customTypeName be called from different threads? I'm also able to build all the tools except for: akaiextract, gig2mono, gig2stereo. The reason is I have to write a replacement for opendir. Maybe at a later date it can be implemented if there's demand for it. The other tools compiled without any modification. |
|
From: Christian S. <sch...@li...> - 2019-02-10 20:56:03
|
On Freitag, 8. Februar 2019 01:03:59 CET justnope wrote: > > Yes, that makes sense. Would you also share your real name so I can > > credit you > > > for your patch? > > I'm fine with being uncredited or using justnope. Committed, slightly modified. Thanks! I figured we should fix the same issue with DLS::File::ExtensionFiles one day, so I added a FIXME comment there for now. > Another thing I noticed but I'm unable to test is that in > Serialization.cpp the return value of abi::__cxa_demangle isn't freed. Yep, good catch! I just committed a fix for that as well, thanks! > > Adding a C++11 dependency just for this minor issue is really not > > neccessary. > > I just googled "ssize_t windows" and the solution seems to be: > I've used the typedef and a check for msvc 2015 or higher. If it's a > lower version an preprocessor error gets displayed. Installing 2013 only > to check if it works seems overkill. That's fine with me. > > As you already assumed, I would like to avoid new dependencies as much as > > possible. > I've implemented demangling and it seems to work. > > I've used dbghelp.dll. It's also possible to use an undocumented crt > function, but I rather add a dependency on dbghelp (which is always > installed on windows) than using that undocumented crt function. If dbghelp.dll is part of the system anyway then I would favour it as well. I just made a quick lookup on the net if that DLL was probably introduced by some Windows version, but could not find anything useful. The Microsoft docs just say "available on all supported systems", so not sure if XP,Vista,7 users would have it as well, but I guess we will find out. > When you use raw_name it gives you a string that you can demangle but it > adds a "." in the beginning of the string which you must skip. > > The question I have is that in the documentation it states that function > I call to demangle is not thread safe. So the question I have is: can > DataType::customTypeName be called from different threads? Well, at the moment the Serialization API is only used by gigedit which calls it always on the main thread. But it is a good point and I will add an API doc comment for that issue to be aware of on Windows to avoid potential future issues. > I'm also able to build all the tools except for: akaiextract, gig2mono, > gig2stereo. The reason is I have to write a replacement for opendir. > Maybe at a later date it can be implemented if there's demand for it. > The other tools compiled without any modification. You might have a look at InstrumentEditorFactory::LoadPlugins(), here is the link to the relevant code portion: http://svn.linuxsampler.org/cgi-bin/viewvc.cgi/linuxsampler/trunk/src/plugins/InstrumentEditorFactory.cpp?revision=3034&view=markup#l158 That might be a candidate for borrowing the required code for the opendir() implementation, which uses FindFirstFile() and FindNextFile() there. CU Christian |