Allow replacement of GSM/WAV49 audio archives with other formats #492

jxmx · 2025-02-15T23:31:25Z

Addresses #485

Allan-N · 2025-02-15T23:33:34Z

apps/app_rpt.c

@@ -2962,7 +2962,7 @@ static inline void log_keyed(struct rpt *myrpt)
 		time(&myt);
 		strftime(mydate, sizeof(mydate) - 1, "%Y%m%d%H%M%S", localtime(&myt));
 		sprintf(myfname, "%s/%s/%s", myrpt->p.archivedir, myrpt->name, mydate);
-		myrpt->monstream = ast_writefile(myfname, "wav49", "app_rpt Air Archive", O_CREAT | O_APPEND, 0, 0644);
+		myrpt->monstream = ast_writefile(myfname, "wav", "app_rpt Air Archive", O_CREAT | O_APPEND, 0, 0644);


While I support this change I will ask whether this going to [significantly] change the size of the archived audio files?

Yes, but I'm not sure there's a better format to choose if we want "native" tools to work with the archive data. Ideally you could do an MP3 or OGG file but that isn't supported. From looking at the code, we can pick from what's in core show file format. Nothing else on that list would be generally readable by Windows and Mac commonly-installed software I don't believe.

mp3 can be supported (I did some work to patch the mp3 support in Asterisk to allow this for a client), but the problem with any compressed formats is it increases the transcoding load, hence why raw formats like wav tend to get used.

Obviously the question there is whether to optimize for space or CPU, and I think the latter is more common on production systems but I could see the value in the other way as well. "Compatibility with consumer devices" is certainly another factor and not one I usually see come up. For that, I agree you can't go wrong with wav

If the size of WAV files become a real problem, we could create a systemd.timer that compresses any WAV files older than 5 minutes.

@jxmx asterisk -x 'module load format_mps' 😄

One suggestion made in the issue discussion was to add a configuration file setting that would specify the file format. By adding that we can keep the current .wav49 and, at the same time, allow other formats (.wav, .mp3, ...)

Adding a configuration option to change the filetype seems unnecessary given that archivedir isn't a commonly-used command to begin with that I see. My understanding of why MP3 and OGG isn't loaded by default is there's problem with them, perhaps performance-related? If we're going to change this to mp3, then we'll also need to change modules.conf and we'll have to add logic to asl3-asterisk.postinst to edit customized modules.conf to load mp3.

Allan brings up a good point. Now would be as easy a time as ever to just do away with a hardcoded format and turn it into a variable that can be set from the config. Defaulting to wav would make sense, but users could easily override it.

My $0.02 :

make the format choice a configuration variable

have the new "archiveformat" default value continue to be "wav49"

This way, all remains the same for running configurations yet folks will have the ability to change the format to one that works better for them. And, if folks do change the format then they can deal with any increased size, CPU requirements, compression issues, etc.,

@InterLinked1 wrote:
mp3 can be supported (I did some work to patch the mp3 support in Asterisk to allow this for a client), but the problem with any compressed formats is it increases the transcoding load, hence why raw formats like wav tend to get used.

Q? Does Asterisk have/use "a" native raw format? If not, then wouldn't we need to perform some transcoding to whatever format is chosen?

I think slin is technically the "raw format" but the PCM formats (ulaw/alaw) are pretty close I believe.
core show translations shows some of the appx. costs in converting between different formats

ulaw/alaw are raw telephony formats and wav isn't compressed so that's why these are usually good choices. gsm and mp3 are compressed and lossy, so they are more computationally intensive.

InterLinked1

Code wise this seems fine, but how about a more useful commit message?

app_rpt: Use wav for recordings instead of wav49

Resolves #485

or something like that?

InterLinked1 · 2025-02-15T23:43:54Z

Code wise this seems fine, but how about a more useful commit message?
app_rpt: Use wav for recordings instead of wav49

Resolves #485
or something like that?

Slightly better but still not in the suggested format. The app_rpt prefix and the reference to the issue number are critical.
Also, it's GSM, not GMS.

mkmer · 2025-02-16T15:20:20Z

Maybe while tinkering right here we should fix the file name generation to snprintf?

app_rpt/apps/app_rpt.c

Line 2964 in b5b110a

sprintf(myfname, "%s/%s/%s", myrpt->p.archivedir, myrpt->name, mydate);

Allan-N · 2025-02-19T21:46:17Z

For those following this PR, I had my hopes up that we might be able to create "mp3" audio archives. Sadly :

[2025-02-19 16:42:48.386] ERROR[193754]: format_mp3.c:277 mp3_rewrite: I Can't write MP3 only read them.
[2025-02-19 16:42:48.386] WARNING[193754]: file.c:512 fn_wrapper: Unable to rewrite format mp3
[2025-02-19 16:42:48.386] WARNING[193754]: file.c:1514 ast_writefile: Unable to rewrite /var/spool/asterisk/monitor/491301/20250219164248.mp3

InterLinked1 · 2025-02-19T22:06:07Z

For those following this PR, I had my hopes up that we might be able to create "mp3" audio archives. Sadly :

[2025-02-19 16:42:48.386] ERROR[193754]: format_mp3.c:277 mp3_rewrite: I Can't write MP3 only read them.
[2025-02-19 16:42:48.386] WARNING[193754]: file.c:512 fn_wrapper: Unable to rewrite format mp3
[2025-02-19 16:42:48.386] WARNING[193754]: file.c:1514 ast_writefile: Unable to rewrite /var/spool/asterisk/monitor/491301/20250219164248.mp3

The ability exists, but the MP3 write patch isn't merged upstream yet - see Asterisk PR 665 / 704

Allan-N · 2025-02-19T22:41:45Z

I have updated the PR to allow you to specify the audio recording format using the new archiveformat variable. We still default to wav49 but you now has options.

... and archivedir and archiveformat are now documented in rpt.conf

InterLinked1 · 2025-02-19T22:42:36Z

configs/rpt/rpt.conf

+; logging can be useful in debugging, policing, or other creative things.
+;
+; The "archiveformat" line can be used to specify the format of the audio
+; recordings.  By default, the format will be "wav49" (GSM in a WAV|wav49


Just say WAV instead of WAV|wav49 (wav49 is gsm in a wav wrapper)

apps/app_rpt.c

This change means that one will be now be able to specify an audio recording format other than "WAV". Resolves #485

jxmx requested review from InterLinked1 and Allan-N February 15, 2025 23:31

jxmx added the enhancement New feature or request label Feb 15, 2025

Allan-N reviewed Feb 15, 2025

View reviewed changes

InterLinked1 requested changes Feb 15, 2025

View reviewed changes

jxmx force-pushed the fix_485 branch from a189504 to acfe580 Compare February 15, 2025 23:41

jxmx requested a review from InterLinked1 February 15, 2025 23:42

jxmx force-pushed the fix_485 branch from acfe580 to b5b110a Compare February 15, 2025 23:45

Allan-N linked an issue Feb 16, 2025 that may be closed by this pull request

Can we move away from .wav files? #485

Closed

Allan-N force-pushed the fix_485 branch 2 times, most recently from 9bc2b8d to 99ea26b Compare February 19, 2025 22:38

Allan-N requested review from mkmer and KB4MDD February 19, 2025 22:42

Allan-N self-assigned this Feb 19, 2025

InterLinked1 reviewed Feb 19, 2025

View reviewed changes

Allan-N force-pushed the fix_485 branch from 99ea26b to e7739c4 Compare February 19, 2025 22:56

Allan-N requested a review from InterLinked1 February 19, 2025 22:58

mkmer approved these changes Feb 19, 2025

View reviewed changes

Allan-N force-pushed the fix_485 branch 2 times, most recently from 4ff798f to 02eb25f Compare February 20, 2025 00:04

InterLinked1 reviewed Feb 20, 2025

View reviewed changes

apps/app_rpt.c Outdated Show resolved Hide resolved

app_rpt: add "archiveformat" to specify the audio recording format

aec8145

This change means that one will be now be able to specify an audio recording format other than "WAV". Resolves #485

Allan-N force-pushed the fix_485 branch from 02eb25f to aec8145 Compare February 20, 2025 00:23

Allan-N requested a review from InterLinked1 February 20, 2025 00:23

InterLinked1 approved these changes Feb 20, 2025

View reviewed changes

Allan-N approved these changes Feb 20, 2025

View reviewed changes

Allan-N changed the title ~~Replace use of GSM/WAV49 with Standard WAV Files~~ Allow replacement of GSM/WAV49 audio archives with other formats Feb 20, 2025

Allan-N merged commit b2763bc into master Feb 20, 2025
3 checks passed

mkmer deleted the fix_485 branch February 20, 2025 15:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow replacement of GSM/WAV49 audio archives with other formats #492

Allow replacement of GSM/WAV49 audio archives with other formats #492

jxmx commented Feb 15, 2025

Allan-N Feb 15, 2025

jxmx Feb 15, 2025

InterLinked1 Feb 15, 2025

jxmx Feb 15, 2025

Allan-N Feb 15, 2025

jxmx Feb 15, 2025

InterLinked1 Feb 15, 2025

Allan-N Feb 16, 2025 •

edited

Loading

Allan-N Feb 16, 2025

InterLinked1 Feb 16, 2025

InterLinked1 left a comment

InterLinked1 commented Feb 15, 2025

mkmer commented Feb 16, 2025

Allan-N commented Feb 19, 2025

InterLinked1 commented Feb 19, 2025

Allan-N commented Feb 19, 2025

InterLinked1 Feb 19, 2025

Allow replacement of GSM/WAV49 audio archives with other formats #492

Allow replacement of GSM/WAV49 audio archives with other formats #492

Conversation

jxmx commented Feb 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Allan-N Feb 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InterLinked1 left a comment

Choose a reason for hiding this comment

InterLinked1 commented Feb 15, 2025

mkmer commented Feb 16, 2025

Allan-N commented Feb 19, 2025

InterLinked1 commented Feb 19, 2025

Allan-N commented Feb 19, 2025

Choose a reason for hiding this comment

Allan-N Feb 16, 2025 •

edited

Loading