flac dev - Jan 2016 - Lets do a 1.3.2 release

lvqcl wrote:
>>> Win_utf8 stuff should not be included in libflac since it's
only to be
>>> used by the flac.exe frontend. It is not needed by other programs
nor
>>> would they benefit from it without doing the extra work of
converting
>>> their ansi filenames and functions to utf-8.
>>>
>>>> Version 2 of my patch attached, which fixes the problem for the
>>>> Makefile.lite and Visual Studio build systems without moving
>>>> win_utf8_io.c or touching the Autotools system.
>>
>> When I compile flac project with MSYS/MinGW-w64, I can see two files:
>> libFLAC.a and libFLAC-static.a. The only difference between them
>> is that libFLAC.a contains functions from win_utf8_io.
>> But 'make install' adds libFLAC.a into /local/lib, not
libFLAC-static.a.
>
>
> P.S.: Visual Studio creates libFLAC_static.lib and libFLAC_dynamic.dll, and
> none of them contain win_utf8_io stuff.
I was wrong.

In Visual Studio, 'flac' project depends on 'libFLAC_static'
project
and all the projects from src/share: getopt_static, grabbag_static,
replaygain_analysis_static, replaygain_synthesis_static,
utf8_static, win_utf8_io_static

Both 'libFLAC_static' and 'libFLAC_dynamic' projects depend
(only)
on 'win_utf8_io_static'.
I tried to remove this dependency and libFLAC_dynamic failed to compile
since metadata_iterators.c uses e.g. flac_fopen, which is defined in
share/compat.h as fopen_utf8.

   metadata_iterators.obj : error LNK2001: unresolved external symbol
unlink_utf8
   metadata_iterators.obj : error LNK2001: unresolved external symbol fopen_utf8
   metadata_iterators.obj : error LNK2001: unresolved external symbol utime_utf8
   metadata_iterators.obj : error LNK2001: unresolved external symbol
_stat64_utf8
   metadata_iterators.obj : error LNK2001: unresolved external symbol chmod_utf8
   metadata_iterators.obj : error LNK2001: unresolved external symbol
rename_utf8

So currently libFLAC on _WIN32 does depend on win_utf8_io.


BTW, share/compat.h includes share/win_utf8_io.h (if _WIN32 is defined),
and is included into many libFLAC source files. As a result, some libFLAC
source files unnecessarily include share/win_utf8_io.h, windows.h, etc.

lvqcl wrote:
> When I compile flac project with MSYS/MinGW-w64, I can see two files:
> libFLAC.a and libFLAC-static.a. The only difference between them
> is that libFLAC.a contains functions from win_utf8_io.
> But 'make install' adds libFLAC.a into /local/lib, not
libFLAC-static.a.
Thank you for checking this. I've always had trouble with Autotools on
Windows, even when running them from a non-Windows machine first.
> So currently libFLAC on _WIN32 does depend on win_utf8_io.
Does my patch fix this issue on your end?
> BTW, share/compat.h includes share/win_utf8_io.h (if _WIN32 is defined),
> and is included into many libFLAC source files. As a result, some libFLAC
> source files unnecessarily include share/win_utf8_io.h, windows.h, etc.
It looks like win_utf8_io.h only includes windows.h in order to have
the proper types to declare CreateFile_utf8, which is used in
src/flac/decode.c and src/share/grabbag/file.c. I suppose those two
files could #define FLAC__INCLUDE_WINAPI and win_utf8_io.h could guard
the include and CreateFile_utf8 declaration with it, if we wanted to
avoid windows.h. If we wanted to avoid share/win_utf8_io.h completely,
we would need to do something similar in the 34 files that use
flac_(printf|fprintf|vfprintf|fopen|chmod|utime|stat|unlink|rename),
which would be counterproductive to the purpose of compat.h.

On the topic of header minimalism, we may want to #define
WIN32_LEAN_AND_MEAN and NOGDI before every #include <windows.h> in the
codebase.


Janne Hyv?rinen wrote:
> Win_utf8 stuff should not be included in libflac since it's only to be
used
> by the flac.exe frontend. It is not needed by other programs nor would they
> benefit from it without doing the extra work of converting their ansi
> filenames and functions to utf-8.
libFLAC.a(metadata_iterators.release.o): In function
`simple_iterator_prime_input_':
flac/src/libFLAC/metadata_iterators.c:427: undefined reference to
`fopen_utf8'
flac/src/libFLAC/metadata_iterators.c:424: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `set_file_stats_':
flac/src/libFLAC/metadata_iterators.c:3355: undefined reference to
`chmod_utf8'
flac/src/libFLAC/metadata_iterators.c:3356: undefined reference to
`utime_utf8'
flac/src/libFLAC/metadata_iterators.c:3355: undefined reference to
`chmod_utf8'
flac/src/libFLAC/metadata_iterators.c:3356: undefined reference to
`utime_utf8'
flac/src/libFLAC/metadata_iterators.c:3355: undefined reference to
`chmod_utf8'
flac/src/libFLAC/metadata_iterators.c:3356: undefined reference to
`utime_utf8'
libFLAC.a(metadata_iterators.release.o): In function
`simple_iterator_prime_input_':
flac/src/libFLAC/metadata_iterators.c:427: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `get_file_stats_':
flac/src/libFLAC/metadata_iterators.c:3343: undefined reference to
`_stat64_utf8'
libFLAC.a(metadata_iterators.release.o): In function `set_file_stats_':
flac/src/libFLAC/metadata_iterators.c:3355: undefined reference to
`chmod_utf8'
flac/src/libFLAC/metadata_iterators.c:3356: undefined reference to
`utime_utf8'
libFLAC.a(metadata_iterators.release.o): In function
`simple_iterator_prime_input_':
flac/src/libFLAC/metadata_iterators.c:424: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `open_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3284: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `cleanup_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
libFLAC.a(metadata_iterators.release.o):flac/src/libFLAC/metadata_iterators.c:3306:
more undefined references to `unlink_utf8' follow
libFLAC.a(metadata_iterators.release.o): In function `transport_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3314: undefined reference to
`rename_utf8'
libFLAC.a(metadata_iterators.release.o): In function `cleanup_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
libFLAC.a(metadata_iterators.release.o): In function `set_file_stats_':
flac/src/libFLAC/metadata_iterators.c:3355: undefined reference to
`chmod_utf8'
flac/src/libFLAC/metadata_iterators.c:3356: undefined reference to
`utime_utf8'
libFLAC.a(metadata_iterators.release.o): In function `cleanup_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
libFLAC.a(metadata_iterators.release.o): In function `chain_read_':
flac/src/libFLAC/metadata_iterators.c:1532: undefined reference to
`fopen_utf8'
flac/src/libFLAC/metadata_iterators.c:1532: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `chain_rewrite_file_':
flac/src/libFLAC/metadata_iterators.c:1394: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `open_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3284: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `cleanup_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
libFLAC.a(metadata_iterators.release.o): In function `get_file_stats_':
flac/src/libFLAC/metadata_iterators.c:3343: undefined reference to
`_stat64_utf8'
libFLAC.a(metadata_iterators.release.o): In function
`chain_rewrite_metadata_in_place_':
flac/src/libFLAC/metadata_iterators.c:1369: undefined reference to
`fopen_utf8'
libFLAC.a(metadata_iterators.release.o): In function `set_file_stats_':
flac/src/libFLAC/metadata_iterators.c:3355: undefined reference to
`chmod_utf8'
flac/src/libFLAC/metadata_iterators.c:3356: undefined reference to
`utime_utf8'
libFLAC.a(metadata_iterators.release.o): In function `transport_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3306: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3314: undefined reference to
`rename_utf8'
libFLAC.a(metadata_iterators.release.o): In function `cleanup_tempfile_':
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
flac/src/libFLAC/metadata_iterators.c:3333: undefined reference to
`unlink_utf8'
libFLAC.a(stream_decoder.release.o): In function `init_file_internal_':
flac/src/libFLAC/stream_decoder.c:609: undefined reference to `fopen_utf8'
flac/src/libFLAC/stream_decoder.c:609: undefined reference to `fopen_utf8'

include/share/compat.h, #ifdef _WIN32:

#define flac_fopen fopen_utf8
#define flac_chmod chmod_utf8
#define flac_utime utime_utf8
#define flac_unlink unlink_utf8
#define flac_rename rename_utf8
#define flac_stat _stat64_utf8

The members of the libFLAC API that result in calls to these functions
include FLAC__stream_decoder_init_file,
FLAC__stream_decoder_init_ogg_file, and a variety of metadata
functions. These functions either take a char* filename or use one
stored in a struct. Fortunately, UTF-8 is a superset of 7-bit ASCII.

Best,
Evan

Evan Ramos wrote:
>> So currently libFLAC on _WIN32 does depend on win_utf8_io.
>
> Does my patch fix this issue on your end?
I don't use libFLAC, so it's not an *issue* for me. So I didn't test
your patch.

Previously all win_utf8_io stuff was intended to be MSVC-only:
the use of win_utf8_io was controlled by FLAC__STRINGS_IN_UTF8 preprocessor
macro (which was defined only in MSVC *.vcproj files) but then it was replaced
by simple _WIN32 (in the commit "Enable utf8 I/O stuff when compiling with
MinGW",
<https://git.xiph.org/?p=flac.git;a=commitdiff;h=2199d086921eb37d249cae0731f334556ec6209d>)

>> BTW, share/compat.h includes share/win_utf8_io.h (if _WIN32 is
defined),
>> and is included into many libFLAC source files. As a result, some
libFLAC
>> source files unnecessarily include share/win_utf8_io.h, windows.h, etc.
>
> It looks like win_utf8_io.h only includes windows.h in order to have
> the proper types to declare CreateFile_utf8, which is used in
> src/flac/decode.c and src/share/grabbag/file.c. I suppose those two
> files could #define FLAC__INCLUDE_WINAPI and win_utf8_io.h could guard
> the include and CreateFile_utf8 declaration with it, if we wanted to
> avoid windows.h. If we wanted to avoid share/win_utf8_io.h completely,
> we would need to do something similar in the 34 files that use
> flac_(printf|fprintf|vfprintf|fopen|chmod|utime|stat|unlink|rename),
> which would be counterproductive to the purpose of compat.h.
Another idea: it's possible to have share/compat_lib.h that doesn't
include win_utf8_io.h and doesn't have definitions of *_utf8 functions
and share/compat.h that includes share/compat_lib.h, share/win_utf8_io.h
and defines all *_utf8 functions.
Source files outside libFLAC use share/compat.h, and libFLAC sources
include only share/compat_lib.h (and share/win_utf8_io.h when necessary).

> On the topic of header minimalism, we may want to #define
> WIN32_LEAN_AND_MEAN and NOGDI before every #include <windows.h> in
the
> codebase.
Maybe, although I'm not sure that it will noticeably reduce compile time.

Evan Ramos wrote:
>> So currently libFLAC on _WIN32 does depend on win_utf8_io.
>
> Does my patch fix this issue on your end?

BTW, your patch also changes MSVC solution/projects. What's the
problem with them? I built flac/metaflac/libFLAC libraries
with Visual Studio many times, and haven't noticed anything wrong.