Activity for manx
-
manx posted a comment on ticket #383
Maybe we need to go to inline asm, after all, at least for active architectures. I would suggest also considering to change to intrinsics instead of inline asm. Intrinsics for SSE and AVX on amd64 (or NEON on arm64) are widely supported and compatible between all major compilers, while inline asm requires compiler-specific syntax. We'd keep the files for 32 bit archs unchanged, I presume. Changing all the x86 32bit asm implementations is very likely not worth the effort, totally agreed.
-
is the combined workflow missing something? As far as I can see, only to old, now removed, noyasm workflow did not successfully build. See https://github.com/madebr/mpg123/actions/runs/12609590335 vs https://github.com/madebr/mpg123/actions/runs/12609590334 . This is because Microsoft removed 32bit ARM support in the latest SDK, and only the combined workflow accounts for that by explicitly selecting an older SDK for 32bit ARM. See https://github.com/madebr/mpg123/blob/b7db2283aa045699c53a788575...
-
They are converted back to wchar_t and passed to e.g. _wfopen. See for example INT123_compat_fopen in src/compat/compat.c. Even though the perfect solution would be to always keep filenames in wchar_t on Windows, this is way too much hassle for an originally POSIX-focused project like mpg123. mpg123 tries to keep everything in char in UTF8 internally. Windows support and conversion handling is not perfect and has known bugs. I am generally pushing for moving towards better Windows support (MinGW...
-
patch attached
-
[MPG123_LINK_DLL] I guess we should put a platform check into mpg123.h to default to a client build on Windows? Setting __declspec(dllimport) unconditionally on Windows would break clients that statically link libmpg123. But apparently it works without? "Modern" (last decades I think, I can did up details if wanted) MSVC toolchains do work without __declspec(dllimport) in client code when calling functions from a linked import library of a DLL, albeit (IIRC) at a small pointer indirection cost for...
-
I think the "demanding to use the _-variants" is Microsoft specific, I am not aware of any situation where the POSIX names do not exist while the _-variants do exist. I really do not know if "_WIN32 implies _-variants" holds true in all situations, so defaulting to the POSIX names by default is probably a more conservative approach. Assuming all Windows C libraries are implementing _-variants sounds somewhat fragile to me. On the other hand, with the current state there is a risk of missing some...
-
Looks like I missing switching to #if defined(MPG123_COMPAT_MSVCRT_IO) in INT123_compat_close. Can you do that as well?
-
Yeah, works for me.
-
I really do not like #define read _read #define close _close at all. That would be redefining names that do not belong to mpg123. But for the plain _lseek(), I rather see the explicit call because it strictly is not the same prototype than lseek(). The r_lseek pointer is using off_t in the prototype, not matching _lseek(). I guess we need a pointless fallback_lseek() wrapper just like fallback_read() to ensure the pointer type matches. Yeah, that was already wrong before for MSVCRT lseek(), which...
-
I honestly do not see how this would work. The old _64-suffixed API functions assume aliasing of off_t and off64_t iff off_t is 64bit, or a distinct 64bit off64_t otherwise. off_t will always be 32bit with MSVC, and libmpg123 should never invent/define any off64_t in a public API header itself. This would break immediately if any other library does the same thing. So, if the public API can neither change nor be extended (with the existing _64-suffixed functions), is there any point in calling _lseeki64...
-
Im kinda sick of this political game of Microsoft, suggesting the POSIX names are in conflict with ISO C. These are not the warnings I am concerned about. I only mentioned that for context and explanation of the work-around/solution. The linker warning is independent of Microsoft's opinions regarding POSIX, and may at the root well be a bug in their ARM64EC UCRT. I would still prefer if libmpg123 would not require patching to be compilable without warnings for ARM64EC. What about building with --enable-portable?...
-
weird warning when building against ARM64EC UCRT
-
warning in readers.c with debugging enabled
-
I/O stuff can be handled, turning non-portable off_t API into wrappers over the new stuff. But for the portable case, there's some more functions that might need suffix-64 variants. So, all funcs with off_t int them someplace sounds fine mpg123_position Given the documentation already deprecates it, maybe make the source also deprecate it. Almost nobody reads documentation, but __attribute__((deprecated)) (and similar) will get noticed. There is of course no need to carry any deprecated function...
-
I am sorry for the late reply. If you want to keep all current ABI and API, MPG123_PORTABLE_API may be a way to go. I still suggest new symbol names for the new functions: The portable API functions would not rely on ssize_t, but instead ptrdiff_t (or anything else that is actually available everywhere). As these types only somewhat guarantee the bit width and in particular not the exact type used (int/long/long long), a change here could cause a mismatch of declared and defined function type between...
-
CMake/YASM change looks good and works for me. The GitHub mirror appears to not update at all at the moment.
-
I am not aware of a compiler-agnostic way to get cpuid information to dispatch SSE or AVX code [...] But porting the actual optimized routines doesn't gain much if one still has to resort to plain assembly to identify the CPU features. cpuid needs some (very limited) compiler-specific implementation, but nothing serious. All compilers have intrinsics or helpers for cpuid: __cpuid/__cpuidex for MSVC, __cpuid/__cpuid_count for GCC/Clang, and for Intel and GCC there are higher level abstractions like...
-
If you want people to use something, you make it work by default without any user intervention required at all. The way to achieve that for MSVC is porting the asm to intrinsics. Expecting people to install yasm just to built an absolutely not performance critical (on modern hardware) library for a single file format is just not realistic at all. I even question if a warning would be that useful. The majority of users will just ignore the warning or silence it, instead of complicating their build....
-
Do you think it would be sensible to have the default build use yasm and cmake error out if it is not found, with a message that one could explicitly choose the generic decoder without assembly? Not really. Almost all users of Visual Studio do not have yasm installed, and they would thus not be able to build a default configuration. That's basically equivalent to not really supporting Visual Studio at all. I think the default build should not rely on additional tools if possible. At best, it could...
-
ports/cmake should not require yasm
-
I agree, this issue can be closed now. Further discussion is a better fit for issue https://sourceforge.net/p/mpg123/bugs/344/.
-
I have to admit I was somewhat puzzled by your claim that the aliases would be available on OpenBSD, because if they were, there would not have been anything to report for me in the first place. Turns out the situation is more complex: While you are correct that a default build of libmpg123 on OpenBSD in fact provides the _64 aliases, that is not true for the libmpg123 that OpenBSD provides. Why? They are building with --disable-lfs-alias (see https://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/audio/mpg123/Makefile?rev=1.114&content-type=text/x-cvsweb-markup),...
-
I am sorry for the late reply. If you want to keep all current ABI and API, MPG123_PORTABLE_API may be a way to go. I still suggest new symbol names for the new functions: The portable API functions would not rely on ssize_t, but instead ptrdiff_t (or anything else that is actually available everywhere). As these types only somewhat guarantee the bit width and in particular not the exact type used (int/long/long long), a change here could cause a mismatch of declared and defined function type between...
-
portability issues and possible libmpg123 v2 API and ABI considerations
-
Part 2/3 I'll reply in 3 parts: 1. kind-of off-topic details 2. reply to your proposal 3. my proposal which I had already mostly finished The header assumes a largefile-sensitive system and reacts to _FILE_OFFSET_BITS being defined even where it should be a no-op. Can we agree that this is fixed by working this fact into mpg123.h.in, using the result stored in largefile_sensitive in configure? I must admit, I have some trouble wrapping my head around all the renaming and symbol availability. Would...
-
Part 1/3 I'll reply in 3 parts: 1. kind-of off-topic details 2. reply to your proposal 3. my proposal which I had already mostly finished This is about some detail remarks, I'm also writing a top-level comment soon on how I'd like us to proceed. I'm trying to separate constructive discussion about the future from complaining about the past. Agreed. I also intend to post a proposal on how to proceed (probably as a separate new issue). Frankly, none of that work was ever necessary at all, and it never...
-
Well … Windows was a bit POSIX for some time … https://brianreiter.org/2010/08/24/the-sad-history-of-the-microsoft-posix-subsystem/ … That is just not native Windows, it never was. That's about as Windows being POSIX as the FreeBSD kernel can run Linux binaries. It is/was a completely separate kernel binary interface. You could never mix a library of the POSIX subsystem with an application of the native Win32 subsystem. Thus, it is completely irrelevant to this whole discussion. it also has a bit...
-
Thinking even more about the issue and also looking into mpg123 code some more, I now do think that revising the API and ABI is the only solution. The current API uses off_t, which is a type not even specified by C - it comes from POSIX. Windows is not POSIX, and the only reason for this type even existing there at all is probably some crude "get things to compile" effort by Microsoft presumably ~30 years ago. off_t on Windows got never updated to 64bit, not even for 64bit Windows. So, if you value...
-
Well, it's a difficult problem, which has been made more difficult by various C libraries implementing not even POSIX, but only something that remotely resembles POSIX. I am basically equally annoyed as you about this situation, and in my library, libopenmpt, I have actually given up to try to solve this at all in our public headers (see https://github.com/OpenMPT/openmpt/commit/8fedeca885445ab1702cae8be92a5a8b81e7fb0d), and defer to client code for choosing a fitting implementation. However unlike...
-
checking for _FILE_OFFSET_BITS is wrong on platforms where it does nothing
-
Thanks, no, nothing else came up.
-
MSVC logic warning in 1.29.1
-
It think it's a bug No. From C99 6.10.6 Pragma directive (1): "A preprocessing directive of the form #pragma pp-tokens opt new-line where the preprocessing token STDC does not immediately follow pragma in the directive (prior to any macro replacement) 174) causes the implementation to behave in an implementation-defined manner." This allows for expanding macros as well as for not expanding macros. MSVC's behaviour is equally standards-compliant as is GCC's. I would consider GCC's behaviour extremely...
-
It think it's a bug No. From C99 6.10.6 Pragma directive (2): "A preprocessing directive of the form #pragma pp-tokens opt new-line where the preprocessing token STDC does not immediately follow pragma in the directive (prior to any macro replacement) 174) causes the implementation to behave in an implementation-defined manner." This allows for expanding macros as well as for not expanding macros. MSVC's behaviour is equally standards-compliant as is GCC's. I would consider GCC's behaviour extremely...
-
Pardon granted. Apologies accepted. :)
-
You know, your buildsystem is not so cool as you think. Every time i update mpg123 Vcpkg port, i have problems with two ports. The fisrt is portaudio. The second is libopenmpt. This is SOLELY the problem of the vcpkg project. They never addressed my concerns with the libopenmpt port. See https://github.com/microsoft/vcpkg/pull/2669. Do your research before asserting things that I did not do. vcpkg is also unfit to be used as a package manager for our dependencies because it has no policy on providing...
-
I added the mpg123_ssize_t thing now. Sounds better than what was there before. Thanks. When things work now … you are not relyiing on mpg123 releases anyway, right, could just update from trunk? So could take some time scheduling 1.28.1. Yes, works for me. I just merged r4942 and r4944 and adjusted our mpg123.h wrappers accordingly.
-
Isn't that OK? It's more flexible, you can opt into the _64 names without configure now. Yes, I can make it work for me. In practice, I guess you can live with support for 32 bit file offsets especially for the use cases of a tracker library. 32 bit offsets are still plenty for instrument samples, eh? It's enough for OpenMPT and libopenmpt currently, as we, as you guessed, are limiting sample length to 32bit anyway. About the ssize_t thing. Would a #ifndef ssize_t #define ssize_t thing be more robust?...
-
Well no, svn trunk does not work as intended because now, #if (!defined MPG123_NO_LARGENAME) && ((defined _FILE_OFFSET_BITS) || (defined MPG123_LARGESUFFIX)) is evaluated even on platforms like MSVC where _FILE_OFFSET_BITS has no relevance. If this is defined by accident, we now get "64"-suffixed names. I would prefer to keep MPG123_NO_CONFIGURE around the whole largefile ifdef-ery, like it was in 1.27. Keeping it exactly as it was would also probably reduce problems for other users that we do not...
-
https://github.com/OpenMPT/openmpt/commit/f14635031dc468426a03aa7bf8db4f032d38cf71
-
Removal of #ifndef MPG123_NO_CONFIGURE breaks build
-
Sure, we could easily distribute object files that would allow linking a different libmpg123. However, we prefer to keep things simple in this regard, and in particular do not want to "trap" downstream users with a default configuration that links libmpg123 statically and would require them to also distribute object files for their binaries as well (yes, rare and remote situation, but we just prefer to not ship statically linked LGPL code in the default configurations). After all, our project is...
-
Well, we caught these in the past (these are not new issues), but I forgot to report them. These are all code changes we currently have in our repository, so no further bug reports for now. We (https://openmpt.org / https://lib.openmpt.org/) are actually only building libmpg123 itself (and that with our own build system), so I have no idea whether the mpg123 utility would still work in DOS ;-). libmpg123 is only a second optional choice for us in DOS anyway because of the license. For platforms with...
-
warnings in debug printf format strings on obscure platforms
-
Sure, it was only a suggestion. Thanks for the quick fix.
-
Yes, works for me. I guess you should probably also update tabinit.c though for consistency. It currently does not break only because "costabs.h" which is included after "debug.h" does not include any system headers.
-
As a minimal work-around. you can move #include "debug.h" to the last #include directive in each .c file (and never ever include it in any .h file). Affected files are: - src/compat/compat_str.c - src/libmpg123/libmpg123.c - src/libmpg123/tabinit.c
-
#define warning clashes with MSVC pragmas, runtime headers, and Windows SDK headers
-
Unclear and/or confusing licensing of src/libmpg123/synth_sse3d.h
-
The download page https://mpg123.de/download.shtml directs to https://mpg123.de/download/win32/1.24.0/...
-
The earlier vrsion http://mpg123.de/download/win32/mpg123-1.23.8-x86.zip just contains...
-
Missing libgcc DLL in the 1.24 Win32 binary package
-
Well, at least as a starting point, the project files were actually useful for m...
-
I can successfully build libmpg123 svn trunk with MSVC 2015 with my own build system...
-
Build failure with VS2015 64bit due to conflicting ssize_t typedef
-
The OpenMPT source code repository has been mig...
-
Delete everything in order to prevent subversio...
-
[Ref] Add CTrackApp::SystemCanRunModernBuilds()...
-
[Fix] build: Really kill ReleaseNoSSE2 configur...
-
[Doc] libopenmpt: Update changelog.
-
[Ref] build: Kill ReleaseNoSSE2 configurations ...
-
[New] build: Add packaging script for libopenmp...
-
[Imp] in_openmpt: Support VS2008.
-
[Mod] build: Enable SSE2 for VS2010 or later Re...
-
[Ref] build: Add ReleaseNoSSE2 MSVC build confi...
-
[Fix] Repair Win10 detection.
-
[Ref] Settings: Add Setting<T>::IsDefault() and...
-
[Fix] libopenmpt: Catch exception types by cons...
-
[Ref] Update Check: Avoid duplicating the setti...
-
[Var] build: Update premake required version to...
-
[Fix] mptOS: Build fix for !WINDOWS.
-
[Ref] openmpt123: Cleanup indentation.
-
[Ref] Detect Windows 10.
-
[Fix] liboepnmpt: Even message_raw should be utf8.
-
[New] libopenmpt: Add "message_raw" metadata wh...
-
[Fix] sounddev: A timeout in the PortAudio-inte...
-
[Fix] Be more defensive with CPUID detection co...
-
[Fix] FindFirstFile can return INVALID_FILE_HAN...
-
[Fix] Advanced settings: We should also call Tr...
-
[Imp] Advanced settings: Add a "Save now" button.
-
[Fix] sounddev: On a fresh install, when playin...
-
[New] openmpt123: Add --end-time command line o...
-
[Ref] Cleanup splash screen startup logic.
-
[Fix] When loading the sample in ReadSampleAsIn...
-
[Ref] std::getenv is cumbersome. Provide a wrap...
-
[Mod] The platform identifier for the update ch...
-
[Var] bladeenc: Add the manual file (which cont...
-
[Var] opus: License text was missing.
-
[Doc] Update path to premake files in include/*...
-
[Var] zlib: Add OpenMPT.txt.
-
[Mod] Add applicatiopn manifest compatibility s...
-
[Fix] Properly tell MFC to not open an empty ne...
-
[Fix] std::getenv returns nullpointer for unset...
-
[Fix] UnRAR: UnRAR 5.1 started actually using W...
-
[Ref] Kill CTrackApp::m_bInitialized. Does not ...
-
[Ref] Move the few bits of init code found in C...
-
[Ref] sounddev: Move migration of old global de...
-
[Fix] About Dialog: Do not associate tab contro...
-
[Ref] More mpt::ustring.
-
[Ref] Cleanup some confusion in Windows version...
1 >
