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

Audio recording #594

Merged
merged 29 commits into from
Jan 15, 2018
Merged

Audio recording #594

merged 29 commits into from
Jan 15, 2018

Conversation

cjcliffe
Copy link
Owner

@cjcliffe cjcliffe commented Dec 31, 2017

Audio Recording features

  • Recording Path setting in File menu
  • WAV File output support
  • Internal AudioSink demod support
  • Record/Stop buttons added in bookmark view actives list
  • Activate Hover + R to record demod, Shift-R to record/stop all

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 1, 2018

Hello @cjcliffe! First, I wish you a happy new Year !

I tried the audio_recording branch, and it works well here on Windows 10 x64.

I have a couple of UI suggestions:

  1. Make the recording path appear in the menu entry itself, just like I did for Sample Rate /Manual entry.
  2. I think the pulsating of the frequency label during the recording is too faint. Maybe the whole spectrum band should pulsate, a bit like the squelch break make the whole waterfall band pulsate in yellow ?
  3. with the optional [V], [M], [S] and now [R] flags displayed in spectrum, the frequency display is quit crowed and so overlaps very often, especially in non-default text sizes. Maybe we should move the flags elsewhere, for instance above the demodulator label area ?

std::wstring userLabel = getDemodulatorUserLabel();

// TODO: Can we support wstring filenames for user labels?
std::string userLabelStr(userLabel.begin(), userLabel.end());
Copy link
Collaborator

@vsonnier vsonnier Jan 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. A better and still quick way to convert a std::wstring to a std::string in a consistent way is to use wxString services:
wxString userLabelForFileName(userLabel);
std::string userLabelStr = userLabelForFileName.ToStdString();

It works well to create "best-effort" file names from std::wstring while the previous solution may create invalid ones.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 2, 2018

Some thoughts about my previous suggestions:
2. After practicing, I'm getting used to the frequency-only green pulsing label. Since I'm always using 1.5x text size I find it big enough finally. So it is your call to improve that.
3. Instead of moving the flags elsewhere, I think compacting their display a bit would do. For example, instead of having [V] 100.40 [R] etc. with spaces between we could change it be like [VRMS]100.4 i.e always start with the flags, stick the one-letter flags together, and use no space between the flags and the frequency.
In addition it seems easier to read the whole thing with all the flags at the beginning.

If you agree with this I can contribute the 1. 3. 4. modifications if you don't have time to do it yourself.

@cjcliffe
Copy link
Owner Author

cjcliffe commented Jan 2, 2018

@vsonnier Happy New Year! -- all good suggestions thanks; I'll tinker with the placement / recording indication ideas when I'm back later this evening. At some point I'd like to use some symbols/graphics to indicate things and will likely add a simple 'sprite sheet' type rendering which would be basically the same as the text renderer without the string building logic and font loader.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 3, 2018

Last changes Look Cool To Me.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 6, 2018

@cjcliffe I think I've found a bug : nothing is registered in the Wav file if the demod level is below the squelch.

If you test by changing the squelch during the recording, and back again, you can hear that no "silence" is registered between the 2 sequences.

Given the existing implementation, I think we need to fill the gaps between squelch breaks by dumping Zero-level audio into the WAV file so that the WAV timecode is consistent with what a user would hear in realtime.

If a demod never break the squelch during a recording, the file is even never created at all.

@starvald
Copy link

starvald commented Jan 6, 2018

@vsonnier is recording hours of silence the intended result of these patches or are you still working towards something?

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 6, 2018

Recording "silence" as you say is perfectly intended.
Imagine you start a recording at Start_time (date and time are in the WAV filename itself), then stops some time later.
If you later find something of interest at some date D in the recording you will be able to know it actually happened roughly at Start_time + D.

If you don't record the below-squelch level as "silence", the time in the record will be all fucked up and you won't be able to know when some particular event occurred.

I checked that a 48KHz - Stereo - S16 bit WAV file (recording FMS) is 0.28MB/ s which leads to about 1GB per hour.
All the other modulations (NFM, AM and so on) are twice smaller because they are inherently Mono.
I think that manageable nowadays.
The WAV format is apparently limited to 4Go which leads to a max record time of 4 hours for a FM stereo recording as above.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 6, 2018

@starvald To answer your question, I don't think there is more we can do here with a WAV file format so I'm not planing any further improvement on this.

@abesnowman
Copy link

abesnowman commented Jan 6, 2018 via email

@starvald
Copy link

starvald commented Jan 6, 2018

couldn't you make that a toggle to record silence or not?
i'm recording 9 NFM channels 24/7 on laptop with an ssd and cubic is capable of being run on a Pi3 off a microsd card.
flash media will take a big hit with this.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 6, 2018

Thanks @mmgarland3 for the suggestions ! I merely fixed a inconsistency to be able to track the time correctly.
The user can define a squelch per-modem (or not), so the present way of recording excludes what the user didn't wanted to hear the same way.

On the other hand, a muted demod is recorded anyway with the actual code, which make sense:
You want all the active modems recorded (by Shift + R) but while doing so you don't want to actually listen to them, so either you keep them muted, or you use Solo Mode ([S]).

It is up to @cjcliffe now to decide of further improvements among all those suggestions.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 6, 2018

@starvald The WAV is a simple "stream" format with the samples stored in a single array without other additional information. As a consequence, we are forced to store "zero samples" to mark the correct elapsing of time.

- Limit WAV size to 2GB for maximum compatibility,
- Continue recording on another file when size gets too big (XXX_001.wav, then XXX_002.wav and so on)
- The sequence assure up to 2000GB worth of recording which should be enough
- Changed file pattern to international Year.Month.Day so its recognizable whether you are English of French or whatever :)
@vsonnier
Copy link
Collaborator

vsonnier commented Jan 7, 2018

In the last commit, I've removed the recording duration limitation by generating multiple files automatically:

  • Limit WAV size to 2GB for maximum compatibility,
  • Continue recording on another file when size gets too big (XXX_001.wav, then XXX_002.wav and so on)
  • The sequence assure up to 2000GB worth of recording which should be enough for anybody.

That won't make WAV files any smaller, though :)

@cjcliffe
Copy link
Owner Author

@vsonnier thanks for the patches; I hadn't tested large files before and forgot about the size limit; and after some consideration I also agree that the squelch shouldn't affect recording times by default..

Though since @starvald 's use case matches my own initial thinking when deciding to discard the squelch gaps I think I'll go ahead and move the Recording path option into a "Recording" sub-menu and add a simple "Squelch" -> "Record Silence / Skip Silence / Record Always" option toggle there as well.

We do need to consider sometime adding a real "preferences" dialog to contain optional settings to soften some of the hard-coded details in general (and before we fill up the menu bar).

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 11, 2018

@cjcliffe My thoughts exactly, about a recording dialog, and about confuguring squelch as well so tthat it is ok for all tastes.
For now the squelch option is simple enough to add indeed.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 13, 2018

@starvald Hello ! I introduced the following changes in the last commit:

  • Moved recording options to a dedicated menu to reduce File menu clutter,
  • Added the Squelch options: Record Silence / Skip Silence / Record Always. Happy now ? :)
  • Added a file time limit, effectively closing Enhancement to audio recording. #587. Now surely happy, you are.

The commit message wrongly references #583, but I really meant #587.

@starvald
Copy link

Sounds great, thanks @vsonnier :)
although it's failing to compile atm.

[lee@Plasma build]$ make [ 1%] Building CXX object CMakeFiles/CubicSDR.dir/external/rtaudio/RtAudio.cpp.o [ 2%] Building CXX object CMakeFiles/CubicSDR.dir/src/CubicSDR.cpp.o In file included from /run/media/storageEXT4/arch-compiles/CubicSDR/src/modules/modem/Modem.h:8:0, from /run/media/storageEXT4/arch-compiles/CubicSDR/src/modules/modem/ModemDigital.h:5, from /run/media/storageEXT4/arch-compiles/CubicSDR/src/demod/DemodulatorInstance.h:11, from /run/media/storageEXT4/arch-compiles/CubicSDR/src/demod/DemodulatorMgr.h:10, from /run/media/storageEXT4/arch-compiles/CubicSDR/src/visual/PrimaryGLContext.h:14, from /run/media/storageEXT4/arch-compiles/CubicSDR/src/CubicSDR.h:13, from /run/media/storageEXT4/arch-compiles/CubicSDR/src/CubicSDR.cpp:17: /run/media/storageEXT4/arch-compiles/CubicSDR/src/audio/AudioThread.h:24:2: error: ‘boolean’ does not name a type; did you mean ‘GLboolean’? boolean is_squelch_active = false; ^~~~~~~ GLboolean make[2]: *** [CMakeFiles/CubicSDR.dir/build.make:87: CMakeFiles/CubicSDR.dir/src/CubicSDR.cpp.o] Error 1 make[1]: *** [CMakeFiles/Makefile2:100: CMakeFiles/CubicSDR.dir/all] Error 2 make: *** [Makefile:130: all] Error 2

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 13, 2018

LOL my Java habit strikes again ! here is the fix.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 13, 2018

@mmgarland3:

...though an option that only records when squelch is broken and writes
a new file to disk when squelched for more than, say 5 seconds, could be
useful for some applications.

Now that the record file time limit is configurable, we have this kind of feature.
The recording is implemented in such a way that no file is actually created, and named with the current timestamp until there is actually something to record.

So if you configure your File Time Limit to 10 s with the Skip Silence option on, for instance, and start a recording for the night.

The next day, you will only get files whose names denote the actual date (in their timestamp) at which something break the squelch, with a margin of error of 10 s.

From within a unique 10 s file, you won't know the exact time a given event occurred because of the reasons given above but on the bright side you won't record hours of silence either. ((c)@starvald).
That will indeed conserve a lot of disk space if the squelch breaks are rare.

@vsonnier
Copy link
Collaborator

vsonnier commented Jan 14, 2018

As you see I didn't use a dialog for Recording configuration because I think it is enough to have a separate menu considering we have very few options. Maybe in the future a Dialog may be needed if tons of other options are added.

And the real reason: I don't know how to use wxFormsBuilder so I did what I can do best :)

@cjcliffe
Copy link
Owner Author

@vsonnier thanks for the patches; taking a look now and will see about getting a build ready

@cjcliffe
Copy link
Owner Author

@vsonnier seems to have introduced a crash on exit in OSX after any recording operations, attempting to track it down now:
Terminating Visual Processor threads.. Application termination complete. Assertion failed: (e == 0), function ~recursive_mutex, file /BuildRoot/Library/Caches/com.apple.xbs/Sources/libcxx/libcxx-120.1/src/mutex.cpp, line 82.

@cjcliffe
Copy link
Owner Author

@vsonnier looks like there's some sort of recursive mutex issue with pushing the same object to the sink thread unless it's at the end of the demod thread function -- if it's pushed before the viz section uses it then it crashes with that error on application exit.

@cjcliffe cjcliffe merged commit 7fb66b6 into master Jan 15, 2018
@cjcliffe cjcliffe deleted the audio_recording branch January 15, 2018 03:30
@parkerlreed
Copy link

Just wanted to say, thanks a ton for the recording! Never realized how useful it would be and I love the implementation.

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.

5 participants