Fix stopping voice keyer messages #276

dl1jbe · 2021-09-01T08:07:12Z

Start voice keying in an separate process so TLF can look for ESC key and send the play_vk script an TERM signal.

To allow the script to handle that signal the actual sound playing has to be run in background by the added '&'.

You have to adapt the autocq timing as the time the sound is running does no longer add to that time.

Please review and test that sound is stopped and rig switched back to receive mode.

dl1jbe · 2021-09-03T12:25:22Z

For the SoX version of play this line should be:
play -q $1 &

Thanks for the hint. According to man page you are right.

I do have Sox-14.4.2 here and de facto both version do work. I will change it anyway.

N0NB

Sorry I missed this.

I've yet to try it but will. I noted the syntax for SoX play in the commit for play_vk.

N0NB · 2021-09-03T12:36:17Z

I actually have these lines in my local play_vk:

# play file on the default sound device

# OSS play utility syntax:
#play $1 -q 2>/dev/null

# SoX play utility syntax
play -q $1

N0NB · 2021-09-03T12:38:30Z

scripts/play_vk

@@ -28,7 +28,7 @@ trap 'kill -s INT $PID' TERM INT

 # play file on the default sound device in backgroudn, so signals can be catch
 # by the 'trap' instruction above
-play $1 -q 2>/dev/null &
+play -q $1 2>/dev/null &


Locally I found that redirecting stderr to /dev/null was not necessary with the -q option properly placed. Please test that, Tom.

Sorry I missed this.

Just found the following in 'man play':

"These options can be specified on the command line at any point before the first effect name."

That may explain the behavior. With -q before the file name it is quite more robust. Thanks.

Locally I found that redirecting stderr to /dev/null was not necessary with the -q option properly placed. Please test that, Tom.

It may be left over from former versions of SoX which left an error message when killed. Just tested it with 'aplay -q' - that would need the redirection.

IIRC, aplay is the ALSA ply utility.

If we opt to retain play_vk then all examples should be included but commented. If we choose to specify the play utility in logcfg.dat then the alternatives can be documented in the man page.

I was basing my placement of -q on the output of play -h which provides a usage output. Yes it says that gopts can be specified any place before the first effect. Earlier in the output is this line:

Usage summary: [gopts] [[fopts] infile]... [fopts] [effect [effopt]]...

So I presumed it would be best to place the -q in front of the filename. However, just testing it now I see it is honored in either place on the command line, so disregard my "advice". :)

I just tested the aplay command. But as agreed let us stay with SoX as 'official' supported.

And sure there has to be more documentation. At the moment it is just to test that the sound gets stopped and rig is going back to RX mode.

I am going through the man page and have noted that area as one that will need to be updated.

- avoids leaving zombies after playing the sound - respects VK playtime for auto CQ (start again AFTER end of sound file)

dl1jbe · 2021-09-07T18:14:55Z

Please test and report back again. If no problems we can add some keywords to allow users to specify the commands to use.

zcsahok · 2021-09-09T17:22:22Z

It does not stop playing on my system (Debian 11 i386). I'm using a plain play_vk script with a single line

play $1 -q 2>/dev/null

Upon Esc "Terminated" is written into the call input field (after the current cursor) and this text can't be cleaned by further Esc's, just by typing it over. Playback continues and terminates normally (not interrupted).
Looks like pkill only kills the play_vk process itself, but the actual play command keeps running.
Replacing the pkill with

pkill -SIGTERM -n play 2>/dev/null

fixes both issues (playback is terminated and there is no extra output). But "play" seems to a bit too generic to me, easy to inflict collateral damage...

N0NB · 2021-09-09T17:47:40Z

While it may seem that play is generic, apparently it doesn't conflict in Debian or they would rename it (that happened some years with the node program in the ax25 packages). Also, as it is a command line program, I doubt that it would be used at the same time tlf is used, at least on the same computer.

To be absolutely certain, its PID or the script's PID needs to be known to kill the correct process that tlf spawned and then kill by PID.

zcsahok · 2021-09-09T17:54:22Z

If PID is know then

pkill -9 -P <PID>

seems to do the right job. (tested in a terminal)

dl1jbe · 2021-09-09T18:01:08Z

Hi Zoli, you need to use the changed play_vk from the PR. It catches the sigterm and in turn kills the play command. Am 09.09.2021 19:22 schrieb Zoltan Csahok ***@***.***>: It does not stop playing on my system (Debian 11 i386). I'm using a plain play_vk script with a single line play $1 -q 2>/dev/null Upon Esc "Terminated" is written into the call input field (after the current cursor) and this text can't be cleaned by further Esc's, just by typing it over. Playback continues and terminates normally (not interrupted). Looks like pkill only kills the play_vk process itself, but the actual play command keeps running. Replacing the pkill with pkill -SIGTERM -n play 2>/dev/null fixes both issues (playback is terminated and there is no extra output). But "play" seems to a bit too generic to me, easy to inflict collateral damage... —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.

N0NB · 2021-09-09T18:01:12Z

If I understand things, system() doesn't return the PID. A more manual method is required, I think.

dl1jbe · 2021-09-09T18:05:03Z

It is difficult to get the pid of a process started by system(). Therefore we use the pkill command. There is one switch to kill only the last started command by pkill - i just do not remember...Am 09.09.2021 19:54 schrieb Zoltan Csahok ***@***.***>: If PID is know then pkill -9 -P <PID> seems to do the right job. (tested in a terminal) —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.

N0NB · 2021-09-09T18:09:38Z

It's the -n option, Tom.

N0NB · 2021-09-09T18:10:30Z

https://manpages.debian.org/bullseye/procps/pkill.1.en.html

zcsahok · 2021-09-09T18:14:28Z

OK, with the new play_vk it does stop indeed, all is fine.
Just users need to be reminded to fix their scripts in case they don't use the distributed ones.

dl1jbe · 2021-09-09T18:54:36Z

I will prepare more documentation in next time. I just needed a confirmation for correct function before continuing in these direction.To be honest the play_vk is not really needed - the mechanism can work with scripts or with plain binary programs (e.g. play). More on that later (using just my phone for that conversation is just annoying....)Am 09.09.2021 20:14 schrieb Zoltan Csahok ***@***.***>: OK, with the new play_vk it does stop indeed, all is fine. Just users need to be reminded to fix their scripts in case they don't use the distributed ones. —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.

zcsahok

added 2 comments

scripts/play_vk

src/main.c

N0NB · 2021-09-10T01:36:11Z

There is a slight delay if I hit F1 and then Escape immediately afterward but this much, much better than we had and than what my hack was.

N0NB

LGTM

dl1jbe · 2021-09-10T07:48:33Z

Zoli, Nate, thanks for the spell check.Atomic_bool is a good idea. I fear the small delay in stopping is difficult to avoid as we are running some threads and real scheduling is controlled by the kernel only.Btw, the delay is shorter than for the cw keying here.Am 10.09.2021 03:36 schrieb Nate Bargmann ***@***.***>: There is a slight delay if I hit F1 and then Escape immediately afterward but this much, much better than we had and than what my hack was. —You are receiving this because you authored the thread.Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications on the go with GitHub Mobile for iOS or Android.

zcsahok · 2021-09-10T18:36:48Z

There is a slight delay if I hit F1 and then Escape immediately afterward but this much, much better than we had and than what my hack was.

Try killing with SIGKILL in play_vk:

trap 'kill -s KILL $PID' TERM INT

For me it resulted in an immediate stopping.

N0NB · 2021-09-11T01:51:52Z

Never mind...

I must have goofed something in my play_vk script. I saved it again and it's killing immediately upon the press of Escape.

Try this, hit F1 a few times in a row...

zcsahok

All OK.

zcsahok · 2021-09-20T18:52:23Z

scripts/play_vk

@@ -21,13 +21,14 @@
 # An executable copy of 'play_vk' in your contest directory (containing the
 # logcfg.dat file) can be adapted and has precedence over the standard install

+# accept SIGTERM and SIGINT signals and kill play command as soon as possible


The latest play_vk and code works just fine for me.

dl1jbe · 2021-09-20T19:50:30Z

Very well. Let me do some fine tuning (comments, better naming, maybe documentation,..) before I merge it in, Should be done in next one or two days.

The playback command will be executed in the soundlog directory, so we can directly reference the file by name

dl1jbe · 2021-11-07T17:03:33Z

It was more than the expected one or two days. I took the chance to rewrite most of the audio handling code to cleanup functionality and improve readability. Furthermore the following changes were made.

Dropped the no longer needed SC_DEVICE keyword
Added user configuration for commands to record and play back voice key messages and soundlog.
(see VK_PLAY_COMMAND, VK_RECORD_COMMAND, SOUNDLOG_PLAY_COMMAND and
SOUNDLOG_RECORD_COMMAND)
Added user configuration for the directory containing soundlogs (SOUNDLOG_DIRECTORY).

I would appreciate a review and feedback for the changes so far.

Open points to do and to discuss:

Documentation (my part)
Should we also make the file ending (and therefore the audio file type) configurable? It is still fixed
to Sun au format. Other parts of TLF use different formats.
Soundlog recording could use an automatic filename suggestion (at the moment still done via the
'soundlog' shell script). Do we keep the existing format (ddhhmm) or should we choose a different one?
The actual 'soundlog' script supports starting a new record any hour or so. Should we keep that in the
script or code it into TLF.
Do we really need the command to replay the soundlog files? I would suggest to drop the command at
all.
It seems more appropriate to use an external player which allows to interactively move to interesting
points in the file, to replay some parts again and so on. There are good solutions out there (graphical
and also with text UI).

dl1jbe · 2022-03-02T16:12:14Z

To complete the started PR and to fix the existing conflicts I did a rebase of the code onto master and split it in two parts.

First part enables a really quick stop of running voice keyer messages via stop_tx.
The second part has most of the audio code rewritten and makes all sound recording and playing user configurable. It will also move soundlogs to ./soundlogs directory by default.

I will add pull requests for both, link back to these PR for reference and close these PR afterwards.

dl1jbe · 2022-03-04T18:18:53Z

All code from these PR got moved to PR #317 and PR #318. Therefor closing these request now.

These commit gives recording and playing of voice keyer messages and contest soundlogs a thorough overhaul. The code relies on the tools from the SoX package with some sane default settings but allow user configuration of all play and record commands (See also the discussion in PR #276) The default directory for recorded soundlogs was changed to ./soundlogs and are therefor contest specific now. These setting can also be overwritten by a configuration keyword (see man page for details). Co-authored-by: zcsahok <[email protected]>

dl1jbe requested review from N0NB and zcsahok September 1, 2021 08:07

Fix stopping voice keyer messages

33068fc

dl1jbe force-pushed the stop_tx branch from bbe0eae to 33068fc Compare September 2, 2021 16:01

dl1jbe mentioned this pull request Sep 3, 2021

Rework sound recorder and playback. #272

Closed

Fix ordering of parameters (Tnx N0NB)

a7df989

N0NB reviewed Sep 3, 2021

View reviewed changes

Use separate thread instead child process for playing audio

256b259

- avoids leaving zombies after playing the sound - respects VK playtime for auto CQ (start again AFTER end of sound file)

zcsahok reviewed Sep 9, 2021

View reviewed changes

scripts/play_vk Outdated Show resolved Hide resolved

src/main.c Outdated Show resolved Hide resolved

N0NB approved these changes Sep 10, 2021

View reviewed changes

dl1jbe added 2 commits September 19, 2021 09:37

Merge branch 'master' into stop_tx

bd99dae

Move voice keying related code to audio.c

3d32d3d

dl1jbe added 2 commits September 20, 2021 17:57

Block playing another voice message as long as the old one is running

eba541c

Speed up stop of voice keyer message

351937f

zcsahok approved these changes Sep 20, 2021

View reviewed changes

dl1jbe added 13 commits September 29, 2021 19:45

Fix playing back soundlog records by using SoX command

accff91

cleanup unused prototypes and defines

37922b2

cleanup unused keyword SC_DEVICE

d3f6ca7

Use consistent naming for voice keyer functions

dcbdcec

Prepare for user configuration of VK play and record commands

e7e1a22

Add keywords and tests for recording/playing the soundlog

ed55e0a

Run VK record commands in background automatically

eac356c

Rework start and stop of soundlog recording

12bfa54

Add SOUNDLOG_DIRECTORY keyword

8b14120

Refactor stop of running comamnd

f47cbc2

Merge branch 'master' into stop_tx

94e9adf

Refactor display of sound file names

767dd36

Refactor playing back soundlog files

56b8418

The playback command will be executed in the soundlog directory, so we can directly reference the file by name

dl1jbe requested review from N0NB and zcsahok November 7, 2021 17:03

dl1jbe added 2 commits November 7, 2021 18:16

Fix use after free error

698843e

Fix default SOUNDLOG_RECORD_COMMAND

f9d48e3

dl1jbe mentioned this pull request Feb 26, 2022

Man edits and more #274

Closed

dl1jbe mentioned this pull request Mar 2, 2022

Stop voice keyer messages #317

Merged

zcsahok approved these changes Mar 2, 2022

View reviewed changes

dl1jbe mentioned this pull request Mar 4, 2022

Rework audio handling #318

Merged

dl1jbe closed this Mar 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix stopping voice keyer messages #276

Fix stopping voice keyer messages #276

dl1jbe commented Sep 1, 2021

dl1jbe commented Sep 3, 2021

N0NB left a comment

N0NB commented Sep 3, 2021

N0NB Sep 3, 2021

dl1jbe Sep 3, 2021

dl1jbe Sep 3, 2021

N0NB Sep 3, 2021

N0NB Sep 3, 2021

dl1jbe Sep 3, 2021

N0NB Sep 3, 2021

dl1jbe commented Sep 7, 2021

zcsahok commented Sep 9, 2021

N0NB commented Sep 9, 2021

zcsahok commented Sep 9, 2021

dl1jbe commented Sep 9, 2021 via email

N0NB commented Sep 9, 2021

dl1jbe commented Sep 9, 2021 via email

N0NB commented Sep 9, 2021

N0NB commented Sep 9, 2021

zcsahok commented Sep 9, 2021

dl1jbe commented Sep 9, 2021 via email

zcsahok left a comment

N0NB commented Sep 10, 2021

N0NB left a comment

dl1jbe commented Sep 10, 2021 via email

zcsahok commented Sep 10, 2021

N0NB commented Sep 11, 2021 •

edited

Loading

zcsahok left a comment

zcsahok Sep 20, 2021

dl1jbe commented Sep 20, 2021

dl1jbe commented Nov 7, 2021

dl1jbe commented Mar 2, 2022

dl1jbe commented Mar 4, 2022

Fix stopping voice keyer messages #276

Fix stopping voice keyer messages #276

Conversation

dl1jbe commented Sep 1, 2021

dl1jbe commented Sep 3, 2021

N0NB left a comment

Choose a reason for hiding this comment

N0NB commented Sep 3, 2021

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

dl1jbe commented Sep 7, 2021

zcsahok commented Sep 9, 2021

N0NB commented Sep 9, 2021

zcsahok commented Sep 9, 2021

dl1jbe commented Sep 9, 2021 via email

N0NB commented Sep 9, 2021

dl1jbe commented Sep 9, 2021 via email

N0NB commented Sep 9, 2021

N0NB commented Sep 9, 2021

zcsahok commented Sep 9, 2021

dl1jbe commented Sep 9, 2021 via email

zcsahok left a comment

Choose a reason for hiding this comment

N0NB commented Sep 10, 2021

N0NB left a comment

Choose a reason for hiding this comment

dl1jbe commented Sep 10, 2021 via email

zcsahok commented Sep 10, 2021

N0NB commented Sep 11, 2021 • edited Loading

zcsahok left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dl1jbe commented Sep 20, 2021

dl1jbe commented Nov 7, 2021

dl1jbe commented Mar 2, 2022

dl1jbe commented Mar 4, 2022

N0NB commented Sep 11, 2021 •

edited

Loading