acarsdec next? :-) #117

f00b4r0 · 2024-08-18T18:19:23Z

Hi,

This large PR proposes what could be acarsdec 4.x

It'll be too long to document each commit separately (that's what the commit log is for anyway, I tried to use meaningful commit messages) so I'll focus on main changes:

This PR includes #116, #107, #106 and bits of #111, in more or less reworked form. It fixes #112, #76 and presumably #100 (through inclusion of #107).

The bulk of this PR consists of:

a large code cleanup and refactor, reducing the number of duplicated (sometimes with slight unnecessary variations) code, deleting dead code and removing unnecessary bandaids; hence deleting more lines than are added (despite adding several header files and comment blocks). The net result is improved memory footprint and CPU usage.
improving error checking and reporting
improving signal handling and exit cleanup
improving build system: all optional libraries are now autodetected (including libasound for ALSA support), the system-wide libcJSON is used instead of embedding a local copy and it builds fine on Linux and macOS (cannot test Windows but I don't expect changes there from previous situation)
enabling an unlimited number of ACARS channels to be decoded simultaneously (within available bandwidth and CPU power). This also means that acarsdec now uses just as much memory it needs for the number of channels actually decoded, no more, no less.
allowing all supported inputs to be built in the same executable (no more "cmake -Drtl=ON") which, together with the removal of the unconditional -march=native compilation flag (which can be added back on cmake invocation, as detailed in README), should make acarsdec packageable by distributions if necessary
allowing all supported output types to be used concurrently and repeatedly, which means it's now possible to display full text will sending JSON (or anything else) via UDP to several feed collectors and updating an MQTT broker at the same time
improving performance-critical common code which translates into e.g. the CPU usage of the SoapySDR backend being all but divided by 4, so that the overhead of using Soapy instead of native backend is now quite negligible
implementing multichannel raw audio input, at multiple sample rates (partly addresses Adding standard input option #109, supersedes Add option to decode raw audio data from STDIN #110).
and more 😁

Oh yes and now there's also a bit of statistics collection and reporting available via a simple StatsD-compatible interface. It's loosely based on how dumpvdl2 reports its own statistics. This helps to e.g. keep track of the performance of the SDR dongle, or to find which ACARS channels are the busiest.

The code reorganization (building all supported input backends together) and the ability to use multiple output combinations prompted me to change quite drastically how command line options are processed. This means in particular that from this point forward, acarsdec is no longer compatible with previous command line parameters (another reason for increasing the version major number, if all the above changes weren't enough already 😉):

inputs now use unambiguous long options (e.g. --rtlsdr instead of -r or --airspy instead of -s, loosely based on dumpvdl2), yet their argument remains unchanged: migration is easy.
outputs are defined using --output FORMAT:DESTINATION:PARAMETERS allowing mix and match of output formats and destinations (e.g. it's now possible to log planeplotter format to file, or send full text via UDP). This is detailed in the README and the supported formats and destinations can be listed via acarsdec --output help.
That aside I tried to keep carried-over short options unchanged.
A side effect of the reorganization is that all options - including frequencies - can be provided in any order. In other words, you can specify options after frequencies and even between frequencies if you so fancy.

I tried to revamp the help text (shown with acarsdec -h) to make everything as clear as possible, as well as correctly handling the cases where not all optional libraries were available at build time.

Finally the things I didn't touch (beyond code cleanup) at this stage are:

output formats syntax (for obvious backward compatibility reasons): I've been testing "acarsdec 4.0" feeding JSON to airframes.io with no problem.
Airspy SDR backend, which does things rather differently than the other 3 (beyond the filter tweak, that is) for reasons I'm not completely clear about: I'd like to make changes but I'd like to get some feedback from the author before I do (e.g. why is it operating entirely on real numbers instead of complex ones, which as far as I can tell the airspy driver can provide?). So the performance of this backend should be essentially unchanged as it doesn't use the new optimized common code.
MSK demodulator
ACARS decoder

The latter two I might revisit later as I see some room for improvement (I'd like to add a noisefloor measurement among other things), but I thought that a 100-commit PR was a nice round number for a start 😅

I have tried to add some meaningful documentation where applicable, I'm by no means an SDR expert so I hope I didn't write any blunder. Should probably still be better than nothing 😊

Sucessfully built: everything except the sdrplay backend (I don't have the required dependencies)

Tested:

rtlsdr input
soapysdr input (both rtlsdr and network modules)
sndfile input
file output
udp output
mqtt output
all output formats
statsd reporting

Needs testing:

sdrplay input
airspy input
ALSA input

(though I shouldn't have brought any functional changes to the latter two)

Tests performed on macOS x86_64, Linux x86_64 and armhf.

I think that sums it up. I'm happy to help maintain acarsdec in the future, if that can help 😊

I hope all of this is useful.

- avoid special-casing IPv6 (AF_UNSPEC already handles it) - avoid keeping multiple copies of the Rawaddr parameter - avoid redundant (and useless) error checks - avoid redundant checks in Netout*() - avoid running strlen() over buffer when snprintf() is guaranteed to return the buffer size - avoid duplicating the prepared JSON buffer, send it natively instead (tested working with airframes.io) - use static buffers for Netoutpp() and Netoutsv() to reduce stack pressure

Fixes: TLeconte#112

This fixes the following problem: Found 3 device(s): 0: Realtek, RTL2838UHIDIR, SN: 00000003 1: Realtek, RTL2838UHIDIR, SN: 00000002 2: Realtek, RTL2838UHIDIR, SN: 00000001 `acarsdec -r 00000002` would select the device with serial 00000001 `acarsdec -r 00000001` would select the device with serial 00000002 This patch disambiguates explicit serials by skipping matching device ID number when the passed argument is exactly 8 characters long, like a fully qualified serial. This patch is designed to be as short a diff as possible while preserving existing behaviour for all but 8-character long arguments.

- move common global variables out of #define scope - exit with an error if more than one input is selected - reformat the usage help text - rename rtlMult -> rateMult (in line with soapy) - adjust rtl and soapy gain routines to work with floats cmake -Drtl=ON -Dsoapy=ON -Dairspy=ON works and builds

This reverts commit 22f0c81. Does this solve an actual problem? dumpvdl2 doesn't have similar code. This runs an extraneous thread and uses nonportable pthread API. -Drtl=ON now builds on macOS

Does this solve an actual problem? Neither dumpvdl2 nor common examples have similar code. This runs an extraneous thread and uses nonportable pthread API. -Dsoapy=ON now builds on macOS Additionally: - add some error checking - retry on SOAPY_SDR_OVERFLOW (previous code wouldn't work either without it) - gracefully retry on zero-reads - don't free in buffer while read() may still be in-flight - remove unnecessary global variables PS: this needs so much more work :/

- No functional change. - Some unused vars have been removed.

- Simplify the logic (use the median value of min and max frequency) - Switch all Fc/minFc/maxFc references to unsigned int This also "fixes" the following warning: warning: taking the absolute value of unsigned type 'unsigned int' has no effect [-Wabsolute-value]

First pass at cleaning the mess, using Linux kernel clang-format rules (without ColumnLimit) and a handful of manual adjustements. No code change.

don't explicitly zero R initializers (zeroing is implicit)

allows decoding an arbitrary number of channels, limited only by CPU and memory. channels.Fr is now unsigned int, adjust casts accordingly until further cleanup.

Remove malloc() casts. No code change.

find it through pkg-config since we're already using that for libacars and make it a hard dependency for now.

We no longer need the various -D<radio>=On since we can build them all simultaneously. Instead, print an informative message when the corresponding library is or isn't found.

also make MQTT support depend on it.

This commit enables the use of multiple concurrent outputs, using the following syntax: acarsdec --output format:destination:parameters [--output ...] Where "format" is one of: - "oneline" for single line text decoding - "full" for full text decoding - "monitor" for live decoding - "pp" for PlanePlotter - "native" for Acarsdec native format With CJSON support enabled: - "json" for JSON output - "routejson" for flight route output in JSON format and "destination" is one of: - "file" for file (including stdout) output - "udp" for network output over UDP With MQTT support enabled: - "mqtt" for MQTT output Not all combinations of format and destination are valid, acarsdec will complain if an invalid combination is chosen. params are described in respective source file (TODO: update usage()) Fixes: TLeconte#76

No functional change

this is completely redundant with the check in blk_thread() and adds extra processing time in the sample processing path.

This reduces memory pressure and cache trashing and speeds up execution. outputmsg() no longers allocates memory unnecessarily and operates on the data passed on from acars blk_thread(), which it only modifies once (nul-termination at suffix position - necessary as cJSON doesn't have primitives to limit string length).

This is faster, especially considering that the source will typically be all-but-last non-nulls.

msg is zeroed on initialization

Use memcpy() instead of for loops and entirely skip parsing if text_len is too short. Avoids redundant tests.

This speeds up execution by not trying each if() in sequence and makes the function operation clearer. Using immediate returns instead of a function pointer and single return point allows aggressive auto-inlining

'bs' is only used once locally in outputmsg()

use explicit size relationships for null-terminated copies

'msn_seq' is only used once locally in outputmsg()

Move one member outside of #ifdef since it's a standard type and this makes packing deterministic.

use sf_readf_float() and close input file on exit

This partly addresses TLeconte#109 until a proper raw IQ input is added, and replaces TLeconte#110 as a more generic implementation use '--sndfile <file>' as previously for all containers supported by libsndfile. use '--sndfile <file>,subtype=<N>' for raw audio processing with <N> being a supported subtype value as specified in sndfile.h. Optionally, 'channels=<M>,endian=<end>' can be added to decode <M> channels (default 1 if not specified) using <end> endianness, where <end> is one of 'big', 'little' or 'cpu' (default 'cpu' if not specified).

use '-m' to change the multiplier from the default '1'

GCC is apparently not as smart as clang to identify uninitialized uses Fixes: d6b48ae

fredclausen · 2025-01-12T08:04:35Z

Thank you so much for this. I maintain a docker container that originally was running TLeconte's acarsdec, and I've switched to using your version 4.0/4.1 for my users. It works wonderfully, and I appreciate your work on this.

f00b4r0 · 2025-01-12T12:10:34Z

Thank you so much for this. I maintain a docker container that originally was running TLeconte's acarsdec, and I've switched to using your version 4.0/4.1 for my users. It works wonderfully, and I appreciate your work on this.

Thanks for the feedback, much appreciated. I have a few more improvements brewing but have left them on the backburner especially considering there was zero feedback here. This gives new motivation to complete that work 👍

BuildTools and others added 30 commits July 11, 2024 19:56

added RTLSDR bias tee support

0b2440a

TLeconte#116

Allow 2.5MHz bandwidth on R2 use without filter optimization if needed.

730c720

TLeconte#107

Add documentation for recent patches

029e2d9

TLeconte#106

Use sendto(2) for udp and retry dns on temporary failure.

30f5b57

TLeconte#111

Prevent clobbering the destination hostname and better error reporting

19fd754

TLeconte#111

CMakeList: fixes

043e4e0

Fixes: TLeconte#112

Revert "add watchDog to detect hanging rtl-sdr"

e2c01e4

This reverts commit 22f0c81. Does this solve an actual problem? dumpvdl2 doesn't have similar code. This runs an extraneous thread and uses nonportable pthread API. -Drtl=ON now builds on macOS

Remove some unused includes and global vars

4e90981

gitignore: add build/ folder

55f8320

code cleanup: regroup all runtime global vars under a single namespace

84721cb

- No functional change. - Some unused vars have been removed.

code cleanup: deduplicate frequencies parsing

560b270

- No functional change. - Some unused vars have been removed.

code cleanup

c906da6

First pass at cleaning the mess, using Linux kernel clang-format rules (without ColumnLimit) and a handful of manual adjustements. No code change.

acarsdec: cleanup usage()

b92e5f1

don't explicitly zero R initializers (zeroing is implicit)

rename R.channel -> R.channels

1e06453

main(): use errx() where applicable

693e143

make number of channels dynamic

396f895

allows decoding an arbitrary number of channels, limited only by CPU and memory. channels.Fr is now unsigned int, adjust casts accordingly until further cleanup.

code cleanup: s/sizeof(type)/sizeof(var)/

4d93619

Remove malloc() casts. No code change.

code cleanup: use system-provided cJSON

01991c8

find it through pkg-config since we're already using that for libacars and make it a hard dependency for now.

code cleanup: improve CMakeLists.txt

6b9632d

We no longer need the various -D<radio>=On since we can build them all simultaneously. Instead, print an informative message when the corresponding library is or isn't found.

code cleanup: make libcJSON optional

5131a27

also make MQTT support depend on it.

netout: add minimal sanity checks

4ce37bd

rtl: fix error message

9cecbdd

fileout: make rotation a per-output parameter

e380549

treewide: code reordering

ba09118

No functional change

f00b4r0 added 27 commits September 26, 2024 17:18

acars: don't check parity during reception

e3bb819

this is completely redundant with the check in blk_thread() and adds extra processing time in the sample processing path.

acars: make parity removal loop vectorizable

ecd1b1d

acars: use stdint types

9855f0a

acars: use an union to access fixed-size members of TXT segment

3e35867

output: use faster mem*() instead of str*()

62f363d

This is faster, especially considering that the source will typically be all-but-last non-nulls.

outputmsg(): remove unnecessary assignments

f23dab8

msg is zeroed on initialization

output: improve downlink msg parsing logic

16903c2

Use memcpy() instead of for loops and entirely skip parsing if text_len is too short. Avoids redundant tests.

label: use switch and immediate return

d79b33f

This speeds up execution by not trying each if() in sequence and makes the function operation clearer. Using immediate returns instead of a function pointer and single return point allows aggressive auto-inlining

DecodeLabel(): do not zero oooi argument

52bb44f

addFlight(): don't use now unnecessary intermediary variable

c166409

fmt_msg(): fix 'Wheels off Tme' typo

0da8306

acarsmsg_t: remove unneeded member

b0fdfd3

'bs' is only used once locally in outputmsg()

move acarsmsg_t from label.h to acarsdec.h

ca44ce2

use explicit size relationships for null-terminated copies

output: remove unnecessary #ifdefs

30a79af

acarsmsg_t: remove unneeded member

4d2bc81

'msn_seq' is only used once locally in outputmsg()

acarsmsg_t: pack

f2a6d73

Move one member outside of #ifdef since it's a standard type and this makes packing deterministic.

soundfile: make inner sample read loop vectorizable

f96c535

use sf_readf_float() and close input file on exit

treewide: add missing catchall error for token parameters

d1ea6b0

netout: add missing parameter check

a47f8b1

treewide: deduplicate token parameter parsing

3f2a61d

parse_params(): allow processing of lone tokens

315f240

soundfile: support sample rates which are multiple of INTRATE

c02dfac

use '-m' to change the multiplier from the default '1'

update README

8fe41b7

output: silence a GCC warning

5a601e9

GCC is apparently not as smart as clang to identify uninitialized uses Fixes: d6b48ae

acarsdec 4.1

59e61a4

f00b4r0 changed the title ~~acarsdec 4.0? :-)~~ acarsdec next? :-) Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

acarsdec next? :-) #117

acarsdec next? :-) #117

f00b4r0 commented Aug 18, 2024 •

edited

Loading

fredclausen commented Jan 12, 2025

f00b4r0 commented Jan 12, 2025

acarsdec next? :-) #117

Are you sure you want to change the base?

acarsdec next? :-) #117

Conversation

f00b4r0 commented Aug 18, 2024 • edited Loading

fredclausen commented Jan 12, 2025

f00b4r0 commented Jan 12, 2025

f00b4r0 commented Aug 18, 2024 •

edited

Loading