Thanks, Erik. This is the delta that should fix everything up. I hope that #include "share/compat.h" is acceptable from the test_streams binary. If not, you can probably change it to FLAC/ordinals.h instead Also don't forget to commit my earlier VERSION="1.3.0" patch. You can add the "pre1" in a normal text editor without risk. I didn't include those changes in the big patch you committed already. -Ben> Ben Allison wrote: > >> Erik, et al. >> >> This fixes the entire library and does it more cleanly. It puts most of >> the guts into share/compat.h > > Thanks Ben. > > I applied a slightly tweaked version of the patch. The patch had to be > tweaked because it broke the Linux build :-). Specifically, the file > include/FLAC/ordinals.h is a public header file and should not include > the project internal header "share/compat.h". I also moved some of the > #ifdef stuff to "share/compat.h". > > What is in git now may not build on windows but should be really, really > close. I'd appreciate it if you could test it and report the error > messages. It might take a couple of iterations but we should get it > working for all the platforms. > > Cheers, > Erik > -- > ---------------------------------------------------------------------- > Erik de Castro Lopo > http://www.mega-nerd.com/ > _______________________________________________ > flac-dev mailing list > flac-dev at xiph.org > http://lists.xiph.org/mailman/listinfo/flac-dev >-------------- next part -------------- A non-text attachment was scrubbed... Name: FLAC-1-3-0-MSVC-round-two.patch Type: application/octet-stream Size: 1472 bytes Desc: not available Url : http://lists.xiph.org/pipermail/flac-dev/attachments/20130306/554b4c80/attachment.obj
Hi Ben, Can you please remove the _MSC_VER >= 1600 check? _MSC_VER 1600 is set for Visual Studio 2010, which means that Visual Studio 2012 will get a lot of errors because _MSC_VER is defined as 1700. Cheers, Cristian. On Wed, Mar 6, 2013 at 5:53 PM, Ben Allison <benski at winamp.com> wrote:> Thanks, Erik. > > This is the delta that should fix everything up. > I hope that #include "share/compat.h" is acceptable from the test_streams > binary. If not, you can probably change it to FLAC/ordinals.h instead > > Also don't forget to commit my earlier VERSION="1.3.0" patch. You can add > the "pre1" in a normal text editor without risk. I didn't include those > changes in the big patch you committed already. > -Ben > > > Ben Allison wrote: > > > >> Erik, et al. > >> > >> This fixes the entire library and does it more cleanly. It puts most of > >> the guts into share/compat.h > > > > Thanks Ben. > > > > I applied a slightly tweaked version of the patch. The patch had to be > > tweaked because it broke the Linux build :-). Specifically, the file > > include/FLAC/ordinals.h is a public header file and should not include > > the project internal header "share/compat.h". I also moved some of the > > #ifdef stuff to "share/compat.h". > > > > What is in git now may not build on windows but should be really, really > > close. I'd appreciate it if you could test it and report the error > > messages. It might take a couple of iterations but we should get it > > working for all the platforms. > > > > Cheers, > > Erik > > -- > > ---------------------------------------------------------------------- > > Erik de Castro Lopo > > http://www.mega-nerd.com/ > > _______________________________________________ > > flac-dev mailing list > > flac-dev at xiph.org > > http://lists.xiph.org/mailman/listinfo/flac-dev > > > > _______________________________________________ > flac-dev mailing list > flac-dev at xiph.org > http://lists.xiph.org/mailman/listinfo/flac-dev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.xiph.org/pipermail/flac-dev/attachments/20130306/e1797a1c/attachment.htm
Those checks account for compiler changes/improvements introduced in Visual Studio 2010 that are not present in Visual Studio 2008. I will grab Visual Studio 2012 off of MSDN and make sure everything is OK there. -Ben> Hi Ben, > > Can you please remove the _MSC_VER >= 1600 check? > > _MSC_VER 1600 is set for Visual Studio 2010, which means > that Visual Studio 2012 will get a lot of errors because _MSC_VER > is defined as 1700. > > Cheers, > Cristian. > > On Wed, Mar 6, 2013 at 5:53 PM, Ben Allison <benski at winamp.com> wrote: > >> Thanks, Erik. >> >> This is the delta that should fix everything up. >> I hope that #include "share/compat.h" is acceptable from the >> test_streams >> binary. If not, you can probably change it to FLAC/ordinals.h instead >> >> Also don't forget to commit my earlier VERSION="1.3.0" patch. You can >> add >> the "pre1" in a normal text editor without risk. I didn't include those >> changes in the big patch you committed already. >> -Ben >> >> > Ben Allison wrote: >> > >> >> Erik, et al. >> >> >> >> This fixes the entire library and does it more cleanly. It puts most >> of >> >> the guts into share/compat.h >> > >> > Thanks Ben. >> > >> > I applied a slightly tweaked version of the patch. The patch had to be >> > tweaked because it broke the Linux build :-). Specifically, the file >> > include/FLAC/ordinals.h is a public header file and should not include >> > the project internal header "share/compat.h". I also moved some of the >> > #ifdef stuff to "share/compat.h". >> > >> > What is in git now may not build on windows but should be really, >> really >> > close. I'd appreciate it if you could test it and report the error >> > messages. It might take a couple of iterations but we should get it >> > working for all the platforms. >> > >> > Cheers, >> > Erik >> > -- >> > ---------------------------------------------------------------------- >> > Erik de Castro Lopo >> > http://www.mega-nerd.com/ >> > _______________________________________________ >> > flac-dev mailing list >> > flac-dev at xiph.org >> > http://lists.xiph.org/mailman/listinfo/flac-dev >> > >> >> _______________________________________________ >> flac-dev mailing list >> flac-dev at xiph.org >> http://lists.xiph.org/mailman/listinfo/flac-dev >> >> >
Ben Allison wrote:> This is the delta that should fix everything up. > I hope that #include "share/compat.h" is acceptable from the test_streams > binary.Yes, the share/* header files are shared between the library internals, the test suite programs and flac and metaflac binary executables.> If not, you can probably change it to FLAC/ordinals.h insteadAre the changes for FLAC/ordinals.h really necessary? Yhe FLAC/*.h header files specify libFLAC's public API. In a previous thread on this mailing list back in February we decided that for compilers that didn't supply <stdint.h> the developer should supply something suitable. Here's the commit: commit c506b2f43e2fadd70141322c774bc196c2c65cc3 Author: Erik de Castro Lopo <erikd at mega-nerd.com> Date: Fri Feb 10 19:19:11 2012 +1100 include/FLAC/ordinals.h : Remove CPP hackery. This change assumes that a C99 <stdint.h> header is available. For compilers where that is not the case, the user should provide a minimal replacement header. I've also just updated the commemt above the #include <stdint.h> to say: /* If your compiler does not provide <stdint.h> you should provide a replacement * which has suitable replacements for the following intX_T and uintX_t types. * For example: * http://msinttypes.googlecode.com/svn/trunk/stdint.h * http://www.azillionmonkeys.com/qed/pstdint.h */ Is this not an acceptable solution? Its kind of neater and MSVC really is the only compiler I know of that doesn't have <stdint.h>. > Also don't forget to commit my earlier VERSION="1.3.0" patch. You can add> the "pre1" in a normal text editor without risk.Yep applied that as well, modifying the version string as you suggested. Cheers, Erik -- ---------------------------------------------------------------------- Erik de Castro Lopo http://www.mega-nerd.com/
> Are the changes for FLAC/ordinals.h really necessary? Yhe FLAC/*.h > header files specify libFLAC's public API. In a previous thread on > this mailing list back in February we decided that for compilers > that didn't supply <stdint.h> the developer should supply something > suitable. Here's the commit:Yes, they are necessary. Here is the rationale 1) FLAC/ordinals.h is a public header file, so replacing this file with an MSVC-compatible version is not only the responsibility of a developer building libFLAC, but also a developer using libFLAC. 2) Mistakes made by a developer supplying a custom implementation of FLAC/ordinals.h and trying to link against a pre-built libFLAC.dll can lead to ABI incompatibility. 3) I understand requiring developers to provide an implementation for lesser-used niche compilers and development environments, such as embedded systems, but Visual Studio 2008 and prior represent a huge portion of developers. -Ben Allison