End of media output in remote mode
According to https://developer.arm.com/documentation/101754/0616/armclang-Reference/armclang-Command-line-Options/-mbranch-protection this build I did on my ARMv8.2 phone should enable BTI: CFLAGS='-Os -mbranch-protection=bti -march=armv8-a" ./configure --with-optimization=0 --enable-nagging make I don't see any issue being reported. It do see linker warnings at runtime regarding unused DT entry that may be related to the alluded no-ops on this older CPU ... those warnings are only in libout123 and...
(With instructiuons, I mean including tool calls to verify if a binary has branch protection enabled or not … my memory is rusty….)
aarch64 assembler files are missing PAC/BTI flags
I was busy with this BTI stuff before and figured that, at least for x86-64, all is well by just ensuring that the assembly is not called via indirect branching (aka function pointers). When resarching the issue back then, I came to the conclusion that wrapping all calls to the assembly into C functions that then get all compiler ornamentation for BTI and whatnot enables the compiler hardening without having to mess with the assembly files yet again (like with executable stack before). Can you give...
@sobukus, reviving this. So AFAICT the assembly routines for aarch64 do not store the LR (x30) to the stack. So they don't need pac instructions. However, they will need bti instructions, specifically 'bti c' at the entry points. Here is a link to a PR I did for Pixman: https://gitlab.freedesktop.org/pixman/pixman/-/merge_requests/105/diffs But the steps are similair: For each assembly file: - ensure the gnu notes section is added (#include usually the easiest) This will mark that BTI is enabled...
Erroneous `-D-I/usr/include` added via the CMake buildsystem port
was fixed in 1.32.5
aarch64 assembler files are missing PAC/BTI flags
Closing for now … I built using clang and branch protection in termux on an Android aarch64 device. It only complained about some prototype-less functions that I'll also fix, but nothing about it still wanting landing pads for the wrapped assembly.
Build fail with portable mode
[PATCH] [Cmake] link libsyn123 with -lm (for exp())
warning in readers.c with debugging enabled
Ok, been a long time. Stuff happened in real life. I finally integrated this. Thanks (though: gcc 11.4 saw no issue with this).
Documentation fix for mpg123_volume_change_db
warning in readers.c with debugging enabled
The point is that there are no indirect branches into the asm code anymore and hence no landing pads needed. At least that seems to fix things on x86. Is that different on arm? Is there an actual issue left, or do you just want those instructions there for themselves? sent from mobile device, trustworthy or not
The warning is gone with 1.32.5 but the original issue is still open since *.S files are not fixed.
I settled the build issue (std=c99 surfaced missing defines in the getcpuflags code for siglongjmp stuff. I'll close this, assuming things are fine now after the correction to use the wrappers.
You're building --with-cpu=aarch64? Choosing plain neon64 would not use a wrapper, but … neon64) ADD_CPPFLAGS="$ADD_CPPFLAGS -DOPT_NEON64 -DREAL_IS_FLOAT" more_sources="$s_neon64" ;; aarch64) ADD_CPPFLAGS="$ADD_CPPFLAGS -DOPT_MULTI -DOPT_GENERIC -DOPT_GENERIC_DITHER -DOPT_NEON64 -DREAL_IS_FLOAT" more_sources="$s_neon64 $s_dither $s_arm_multi" ;; So this is a multi build that should use the wrapper. And … oh … dear … what stupid thing: I simply didn't complete the wrapper usage in INT123_dct36_choose()....
missing symbol INT123_dct36_sse_wrap when configured with --with-cpu=sse_alone
Yeah, confirmed. I included libm now in current svn trunk. Thanks.
OK, this can be closed.
(sorry)
Yes, I reverted that change. The indirect branch on aarch64 is the bogus thing here. No OPT_THE_DCT36 and thus no wrappers for single optimizations.
Wait, this is warped. We should not need the wrappers without OPT_MULTI. There should be no indirect banches. On other platforms, this issue does not arise as we do not call into asm via function pointers. Can you be more specific as to which assembly function is to blame? Where is the indirect branch? Or is the linker just doing this blindly?
This seems to be because of r5396, and there is an OPT_MULTI confusion: OPT_MULTI is not defined in this build, but OPT_THE_DCT36 and thus DCT36_WRAP are only defined if OPT_MULTI is defined.
[PATCH] [Cmake] link libsyn123 with -lm (for exp())
missing symbol INT123_dct36_sse_wrap when configured with --with-cpu=sse_alone
Yeah, that was stupid of me. Please check current trunk or https://mpg123.org/snapshot . I should release 1.32.5 shortly.
Build fail with portable mode
So you have OPT_NEON64 defined so that the wrapper function is created, but it's not assigned to fr->cpu.opts.the_dct36 later on in layer3.c? Oh, of course. The case without OPT_MULTI didn't use the wrappers yet. Please check current https://mpg123.org/snapshot (or svn trunk).
This is with version 1.32.4. Maybe related, I have this warning: [ 32s] src/libmpg123/layer3.c:1491:12: warning: 'INT123_dct36_neon64_wrap' defined but not used [-Wunused-function] [ 32s] 1491 | DCT36_WRAP(INT123_dct36_neon64) [ 32s] | ^~~~~~~~~~~~~~~~~~~ [ 32s] src/libmpg123/layer3.c:1467:13: note: in definition of macro 'DCT36_WRAP' [ 32s] 1467 | static void asmfunc ## _wrap(real *inbuf,real *o1,real *o2,const real *wintab,real *tsbuf) \ [ 32s] | ^~~~~~~
Wich exact version are you referring to and which assembler function, specifically? I hoped to have that issue avoided via this change noted in NEWS for 1.32.4: libmpg123: -- Avoid indirect branches into the assembly routines by using C wrappers also for dct36, relieving us of the need to care for bti / endbr instructions for control flow integrity.
aarch64 assembler files are missing PAC/BTI flags
Oh, darn. Did I miss that for the current release? I fixed in svn now, thanks.
command-line argument -b causes current track output to be invalid
cmake build fixes, mostly related to O_LARGEFILE redefinition
You're right. That's wrong for all modules covered there.
Erroneous `-D-I/usr/include` added via the CMake buildsystem port
Oh, I just realized that I did 'fix' that behaviour in the meantime. With 1.29.3 (Ubuntu 22.04) you already have buffer draining after each song. This has the effect of the new track info only appearing once the new track starts. If you want absolutely no pause between tracks, you can give --smooth to avoid the draining. This has the downside of restoring the early printout of track info. But you can choose: Properly timed information or absolutely smooth playback. The use of --index is probably...
It looks like --index is a good way of avoiding the problem. I really like the --buffer option though and will continue to use that with an older mpg123 release (due to an unrelated NFS client incompatibility I ran into that leads to update problems). Thanks! Feel free to close this.
Using the default -o alsa was the cause of audio distortion and using -o pulse instead avoids that. Thanks! Will try using --index more instead of --buffer .
Pulseaudio? There's (or was) a bug in the ALSA wrapper that results in static noise at stream beginning (or interruption). Maybe -o pulse to use pulse directly fixes it in this case.
When I tried -i / --index with 1.25.13, when using the command-line: mpg123 --index --smooth -C FILE.mp3 I experienced audio distortion at the beginning before it jumps into the single file, later in its contents. The version might be too old, so I might try --index in a future version after I update the Linux install in the future. The file I was experimenting with is larger, (41863756 bytes, i.e., Jethro Tull - Thick As a Brick) if that is relevant.,
Thank you for the suggestion. I will try using -i / --index instead of -b for NFS delays.
Regardless of fixing the display issue or not ... for your case --index might do the trick. It should fill the filesystem cache with the track, in exchange for a small pause beween tracks. But for typical file sizes, this should be really small.
It seems like you would prefer using "--preload 1" which must be maximizing the input buffer. The reason I was using "-b 65536" was to avoid any potential NFS delays that would impact playback. The man page for version 1.25.13 (dated 29 Feb 2016) says "If you suffer from short network outages, you should try the -b option (buffer) to bypass such outages.". So, based on the man page "-b" should be more appropriate for network trouble. I understand this current track output problem has existed for...
On 11/4/23 00:50, Thomas Orgis wrote: True. The positiion info takes that somewhat into account, but mpg123 prints new track info as it appears from the decoder. It's not cached in the player. The decoder library moved on to the next track while the buffer still plays the old one. Considering that this has been that way for rather many years, since the buffer first entered, we might call it a feature request to do otherwise. It can be done, but people weren't bothered enough by the current state...
It seems like you would prefer using "--preload 1" which must be maximizing the input buffer. The reason I was using "-b 65536" was to avoid any potential NFS delays that would impact playback. The man page for version 1.25.13 (dated 29 Feb 2016) says "If you suffer from short network outages, you should try the -b option (buffer) to bypass such outages.". So, based on the man page "-b" should be more appropriate for network trouble. I understand this current track output problem has existed for...
True. The positiion info takes that somewhat into account, but mpg123 prints new track info as it appears from the decoder. It's not cached in the player. The decoder library moved on to the next track while the buffer still plays the old one. Considering that this has been that way for rather many years, since the buffer first entered, we might call it a feature request to do otherwise. It can be done, but people weren't bothered enough by the current state yet. What is your use case for the buffer,...
command-line argument -b causes current track output to be invalid
Documentation fix for mpg123_volume_change_db
This can and should be closed now.
ABI break in 1.32.2 compared to 1.31.x
Well, then, I hope 1.32.3 does the trick.
Cool. So it should become a release in the coming days. This then is hopefully the sensible upgrade from 1.31 series.
The new snapshot works for me. I get the _64 symbols everywhere and the _32 symbols on 32 bit builds.
And there's config.log
Ah, I got it. The previous snapshot should have been fine for 32 bit, can you confirm? For 64 bit systems, where LFS_LARGEFILE_64 is not defined, the aliases were still missing. This is the current respective code block: #if SIZEOF_OFF_T == 8 resample_total_alias(off_t, syn123_resample_total, syn123_resample_total64) resample_total_alias(off_t, syn123_resample_intotal, syn123_resample_intotal64) resample_total_alias(off_t, syn123_resample_total_64, syn123_resample_total64) resample_total_alias(off_t,...
Yes, for mpg123_... they are there: nm -D ./src/libmpg123/.libs/libmpg123.so ./src/libsyn123/.libs/libsyn123.so | grep _64 0000000000038720 T mpg123_decode_frame_64 0000000000038780 T mpg123_feedseek_64 0000000000038710 T mpg123_framebyframe_decode_64 00000000000387b0 T mpg123_framelength_64 00000000000387f0 T mpg123_framepos_64 00000000000387d0 T mpg123_index_64 00000000000387c0 T mpg123_length_64 00000000000386d0 T mpg123_open_64 00000000000386f0 T mpg123_open_fd_64 00000000000386e0 T mpg123_open_fixed_64...
Hm, now I see it, too. Sometimes they are there, sometimes not?!
OK, can you give me a full config.log, config.h? I don't see right now what's missing. The mpg123_*_64 and _32 are there, right? sent from mobile device, trustworthy or not
No. syn123_resample_total_32, syn123_resample_total_64, syn123_resample_intotal_32, and syn123_resample_intotal_64 are still missing.
OK, so a 1.32.3 release is close. Can you check if https://mpg123.org/snapshot/mpg123-1.32.3-dev+20231001112336.tar.bz2 fixes things?
I should've known what mess this ABI reworking becomes. But somehow I feel a normal call for testing wouldn't have catched these cases. Sorry for puting you at the test bench. syn123_resample_total64() is new API. I need to re-enable exporting of the _32and _64 wrappers.
And here from the 32 bit build: --- debian/libmpg123-0.symbols (libmpg123-0_1.32.2-1_i386) +++ dpkg-gensymbolsz76akb 2023-10-01 11:10:24.666405848 +0000 @@ -7,6 +7,7 @@ mpg123_copy_string@Base 1.6.2 mpg123_current_decoder@Base 1.7.2 mpg123_decode@Base 1.6.2 + mpg123_decode_frame64@Base 1.32.2-1 mpg123_decode_frame@Base 1.6.2 (arch-bits=32|arch=!kfreebsd-any !x32)mpg123_decode_frame_32@Base 1.13.7 mpg123_decode_frame_64@Base 1.13.7 @@ -15,6 +16,7 @@ mpg123_delete@Base 1.6.2 mpg123_delete_pars@Base...
And here from the 32 bit build: --- debian/libmpg123-0.symbols (libmpg123-0_1.32.2-1_i386) +++ dpkg-gensymbolsz76akb 2023-10-01 11:10:24.666405848 +0000 @@ -7,6 +7,7 @@ mpg123_copy_string@Base 1.6.2 mpg123_current_decoder@Base 1.7.2 mpg123_decode@Base 1.6.2 + mpg123_decode_frame64@Base 1.32.2-1 mpg123_decode_frame@Base 1.6.2 (arch-bits=32|arch=!kfreebsd-any !x32)mpg123_decode_frame_32@Base 1.13.7 mpg123_decode_frame_64@Base 1.13.7 @@ -15,6 +16,7 @@ mpg123_delete@Base 1.6.2 mpg123_delete_pars@Base...
ABI break in 1.32.2 compared to 1.31.x
macosx build with autotools generates output modules with .so extension
Well, works-as-designed, then.
Hm, cmake port does the same too. Found this https://gitlab.kitware.com/cmake/cmake/-/issues/21189 I guess this should possibly be ignored and closed.
macosx build with autotools generates output modules with .so extension
Yes. (And not as enum, if that's what you're worried about.)
Are errno values macros under Windows? I guess defining to the documented value won't hurt ... sent from mobile device, trustworthy or not
The current trunk seems to work around the GetThreadErrorMode fine. I suggest the following, to make things final. diff --git a/src/libmpg123/lfs_wrap.c b/src/libmpg123/lfs_wrap.c index 1474de7..e082bb7 100644 --- a/src/libmpg123/lfs_wrap.c +++ b/src/libmpg123/lfs_wrap.c @@ -48,4 +48,8 @@ #include "debug.h" +#if defined(_WIN32) && !defined(EOVERFLOW) +#define EOVERFLOW 132 +#endif + // We do not want to expose this publicly, but it is cleaner to have it also defined // as portable API to offer the...
The toolchain I have is really outdated, so don't worry about it. Feel free to close this ticket.
GStreamer x86_64 FTBFS since mpg123 1.32
Also sorry for shouting … sf.net does that when replying by mail, apparently. 1.32.2 hopefull carries the fix for you.
CMake: Calls to check_type_size require CheckTypeSize included
Missing man1 folder in 1.32.0?
Errors fixes in ports/cmake plus Android Cross Compiling
Fix released with 1.32.2.
I pushed 1.32.2 release now with the 4 points … about CMake build not being on par with the rest, that's known. Feature parity is no goal that I work on. The autoconf build is the main one. About missing checks and broken build … do you have such a toolchain at hand and can check the current trunk if it disables modules?
I am painfully aware of this. The fix is there, to be committed to svn trunk and also released asap. In the meantime... holding off the upgrade ro 1.32 is sensible. Sorry for that. sent from mobile device, trustworthy or not
GStreamer x86_64 FTBFS since mpg123 1.32
Svn-trunk looks good. The cmake side doesn't reflect autotools wrt --disable-components & co: configure with -DBUILD_LIBOUT123=OFF (I couldn't see other options) for windows with a toolchain w/o GetThreadErrorMode/SetThreadErrorMode, it doesn't check for those two funcs and doesn't disable modules either, and linkage fails as a result. (Just reporting though, I won't fight for it, I use autotools with ease.)