Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework audio handling #318

Merged
merged 23 commits into from
Mar 24, 2022
Merged

Rework audio handling #318

merged 23 commits into from
Mar 24, 2022

Conversation

dl1jbe
Copy link
Member

@dl1jbe dl1jbe commented Mar 4, 2022

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).

Please update the script soundlog in /usr/bin before testing.

As I will be away the next days please merge the PR in if there are no problems.

This was referenced Mar 4, 2022
@zcsahok
Copy link
Member

zcsahok commented Mar 7, 2022

I see at least one issue: contest recording can be started multiple times. The user has no feedback whether it is already active. Ideally options 1 and 2 would be mutually exclusive. Similarly on exit the recording continues, that could be viewed both as a feature or a bug. Here it would make sense to ask for confirmation to continue with the recording, e.g. in case user wants to make a quick parameter change and get back to TLF.

zcsahok added 2 commits March 7, 2022 19:27
 - clear whole screen on start
 - drop period from menu items
 - add comment for 3->4 transition
@zcsahok
Copy link
Member

zcsahok commented Mar 7, 2022

Added some minor cosmetic fixes.

dl1jbe and others added 5 commits March 12, 2022 19:38
- Start/stop is done by the same command, so recording can not be
  started two times in a row.
- Code honors recording started in a former run of TLF. So you can start
  recording, leave TLF and start it anew. TLF recognizes already started
  sound recording and allows now stop of recording,
- Drop unused case for separate playing the sound file.
@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 17, 2022

I see at least one issue: contest recording can be started multiple times. The user has no feedback whether it is already active. Ideally options 1 and 2 would be mutually exclusive. Similarly on exit the recording continues, that could be viewed both as a feature or a bug. Here it would make sense to ask for confirmation to continue with the recording, e.g. in case user wants to make a quick parameter change and get back to TLF.

Thanks. That sounds like a valid enhancement. The following commits fixes both problems.

dl1jbe and others added 2 commits March 17, 2022 19:06
play_thread assumes now that the 'ptr' parameter is dynamically
allocated and has to be freed by the thread after use.
@zcsahok
Copy link
Member

zcsahok commented Mar 17, 2022

Thanks for the updates! Just added minor cosmetic fixes.

On playback using the default command I get this warning:
image
It's the same from command line:

$ time play -q 172036.au
play WARN alsa: can't encode 0-bit Unknown or not applicable

real	0m51.251s

I does play it back, just the warning is annoying. In my case a trailing -t alsa option fixes it, but maybe this is not a universal solution. Info: https://groups.google.com/g/linux.debian.bugs.dist/c/jCqdwFWPUKk

Another point: it would be nice to use the same terms (Enable/Disable vs. Start/Stop) on the :sound screen and at exit.

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 17, 2022

Hmm, that does not look nice. Here it works without problems. The provided discussion is quite old, just wondering that the problem is not fixed yet in SoX.
We could redirect all output to /dev/null , but it seems not right to me. Maybe we find another solution.

Wrt start/stop I agree. Will fix it.

@zcsahok
Copy link
Member

zcsahok commented Mar 17, 2022

I have this SoX version (Debian 11)

$ play --version
play:      SoX v14.4.2

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 18, 2022

Same version here but with patchset from May 2021. v14.4.2 is otherwise quite old - 2015.

Anyway the code should work with the tools provided by the actual standard distributions.

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 18, 2022

Can you please test if the warning from play goes only to stderr (play -q xyz 2> /dev/null)?

@zcsahok
Copy link
Member

zcsahok commented Mar 18, 2022

There is no warning with stderr redirect. (2>/dev/null)

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 23, 2022

Thanks for the quick test and sorry for the late reply. Last days I did not feel well.

I think best is to redirect that error message by adding the '2>/dev/null' but only for the default soundlog play command. If the user decides t use another program for play back there will be nost likely no error. See coming patch.

@zcsahok
Copy link
Member

zcsahok commented Mar 23, 2022

OK with me.

@zcsahok
Copy link
Member

zcsahok commented Mar 23, 2022

(stopping with Ctrl-C is somewhat slow, but that's the same from command line. most likely it buffers a large amount of sound data that needs to be drained first.)

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 23, 2022

Yes draining the buffers takes some time. Other than for voice key messages the delay seems acceptable here. So no need to hard kill the program and let it finish graceful.

Again, Thanks for the feedback. Will merge it in tomorrow.

@dl1jbe dl1jbe merged commit bd0dc38 into Tlf:master Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants