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

Various fixes and additions related to recording the mix to a file #1035

Merged
merged 12 commits into from Nov 6, 2016
Merged
54 changes: 51 additions & 3 deletions src/engine/sidechain/enginerecord.cpp
Expand Up @@ -131,6 +131,11 @@ bool EngineRecord::metaDataHasChanged()
}
m_iMetaDataLife = 0;

//TODO: Needs improvement. Usually when we start recording, all tracks are stopped
// and we start the track just after that. Given that this is executed with start recording,
// it waits for kMetaDataLifeTimeout until it actually notices that the track
// has started playing. This translates to the cue file start time being too late.
// Maybe we need a signal for this too.
TrackPointer pTrack = PlayerInfo::instance().getCurrentPlayingTrack();
if (!pTrack)
return false;
Expand Down Expand Up @@ -160,6 +165,9 @@ void EngineRecord::process(const CSAMPLE* pBuffer, const int iBufferSize) {
if (fileOpen()) {
Event::end("EngineRecord recording");
closeFile(); // Close file and free encoder.
if (m_bCueIsEnabled) {
closeCueFile();
}
emit(isRecording(false, false));
}
} else if (recordingStatus == RECORD_READY) {
Expand Down Expand Up @@ -191,7 +199,42 @@ void EngineRecord::process(const CSAMPLE* pBuffer, const int iBufferSize) {
// An error occurred.
emit(isRecording(false, true));
}
} else if (recordingStatus == RECORD_ON) {
} else if (recordingStatus == RECORD_SPLIT_CONTINUE) {
if (fileOpen()) {
closeFile(); // Close file and free encoder.
if (m_bCueIsEnabled) {
closeCueFile();
}
}
updateFromPreferences(); // Update file location from preferences.
if (openFile()) {
qDebug() << "Splitting to a new file: "<< m_fileName;
m_pRecReady->set(RECORD_ON);
Copy link
Member

Choose a reason for hiding this comment

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

This can be an (unlikely) race condition if the user has stopped the recording in between.
It can be solved by using connectValueChangeRequest() and write the value finally by setAndConfirm()

This is done for broadcasting as well:

m_pStatusCO->connectValueChangeRequest(

In addition, status and enable is separated into two COs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am unsure if I follow your reasoning fully here, but anyway, you seem to suggest a conceptual change.

If your scenario is:

mixxx is recording
process is called
split is required
new mode set to split
(time passes)
process is called
the split part is entered
user stops recording. This should be calling the encodingmanager to stop, and so set the mode to off
since we had already entered into the if, but not yet called the set on (we were opening the file), the stop request is ignored.

If you want to ensure thread synchronization, the only bet is synchronizing the whole method.

Copy link
Member

Choose a reason for hiding this comment

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

Since it was that way before anyway and the case is unlikely, it is not required to do this in this PR. But a bullet prove solution would be nice on the long run.

What do you mean with "synchronizing the whole method"?
For my feeling using two separate COs will works, but we have to verify this.

emit(isRecording(true, false)); // will notify the RecordingManager

// Since we just started recording, timeout and clear the metadata.
m_iMetaDataLife = kMetaDataLifeTimeout;
m_pCurrentTrack = TrackPointer();

// clean frames counting and get current sample rate.
m_frames = 0;
m_sampleRate = m_pSamplerate->get();

if (m_bCueIsEnabled) {
openCueFile();
m_cueTrack = 0;
}
} else { // Maybe the encoder could not be initialized
qDebug() << "Could not open" << m_fileName << "for writing.";
Event::end("EngineRecord recording");
qDebug("Setting record flag to: OFF");
m_pRecReady->slotSet(RECORD_OFF);
// An error occurred.
emit(isRecording(false, true));
}
}

if (recordingStatus == RECORD_ON || recordingStatus == RECORD_SPLIT_CONTINUE) {
// If recording is enabled process audio to compressed or uncompressed data.
if (m_encoding == ENCODING_WAVE || m_encoding == ENCODING_AIFF) {
if (m_pSndfile != NULL) {
Expand All @@ -214,7 +257,7 @@ void EngineRecord::process(const CSAMPLE* pBuffer, const int iBufferSize) {
// gets recorded duration and emit signal that will be used
// by RecordingManager to update the label besides start/stop button
if (lastDuration != m_recordedDuration) {
emit(durationRecorded(getRecordedDurationStr()));
emit(durationRecorded(m_recordedDuration));
}

if (m_bCueIsEnabled) {
Expand Down Expand Up @@ -308,6 +351,10 @@ bool EngineRecord::openFile() {
#endif
if (m_pSndfile) {
sf_command(m_pSndfile, SFC_SET_NORM_FLOAT, NULL, SF_TRUE);
// Warning! Depending on how libsndfile is compiled autoclip may not work.
// Ensure CPU_CLIPS_NEGATIVE and CPU_CLIPS_POSITIVE is setup properly in the build.
sf_command(m_pSndfile, SFC_SET_CLIPPING, NULL, SF_TRUE) ;

// Set meta data
int ret = sf_set_string(m_pSndfile, SF_STR_TITLE, m_baTitle.constData());
if (ret != 0) {
Expand Down Expand Up @@ -370,7 +417,8 @@ bool EngineRecord::openCueFile() {
}

m_cueFile.write(QString("FILE \"%1\" %2%3\n").arg(
QString(m_fileName).replace(QString("\""), QString("\\\"")),
QFileInfo(m_fileName).fileName() //strip path
.replace(QString("\""), QString("\\\"")), // escape doublequote
QString(m_encoding).toUpper(),
m_encoding == ENCODING_WAVE ? "E" : " ").toLatin1());
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/engine/sidechain/enginerecord.h
Expand Up @@ -66,7 +66,7 @@ class EngineRecord : public QObject, public EncoderCallback, public SideChainWor
// only one error can occur: the specified file was unable to be opened for
// writing.
void isRecording(bool recording, bool error);
void durationRecorded(QString duration);
void durationRecorded(quint64 durationInt);

private:
int getActiveTracks();
Expand Down
23 changes: 13 additions & 10 deletions src/preferences/dialog/dlgprefrecord.cpp
Expand Up @@ -37,8 +37,6 @@ DlgPrefRecord::DlgPrefRecord(QWidget* parent, UserSettingsPointer pConfig)
setupUi(this);

// See RECORD_* #defines in defs_recording.h
m_pRecordControl = new ControlProxy(
RECORDING_PREF_KEY, "status", this);

m_pRadioOgg = new QRadioButton("Ogg Vorbis");
m_pRadioMp3 = new QRadioButton(ENCODING_MP3);
Expand Down Expand Up @@ -116,22 +114,27 @@ DlgPrefRecord::DlgPrefRecord(QWidget* parent, UserSettingsPointer pConfig)
this, SLOT(slotChangeSplitSize()));

slotApply();
// Make sure a corrupt config file won't cause us to record constantly.
m_pRecordControl->set(RECORD_OFF);

comboBoxSplitting->addItem(SPLIT_650MB);
comboBoxSplitting->addItem(SPLIT_700MB);
comboBoxSplitting->addItem(SPLIT_1024MB);
comboBoxSplitting->addItem(SPLIT_2048MB);
comboBoxSplitting->addItem(SPLIT_4096MB);
comboBoxSplitting->addItem(SPLIT_60MIN);
comboBoxSplitting->addItem(SPLIT_74MIN);
comboBoxSplitting->addItem(SPLIT_80MIN);
comboBoxSplitting->addItem(SPLIT_120MIN);

QString fileSizeStr = m_pConfig->getValueString(ConfigKey(RECORDING_PREF_KEY, "FileSize"));
int index = comboBoxSplitting->findText(fileSizeStr);
if (index > 0) {
if (index >= 0) {
// Set file split size
comboBoxSplitting->setCurrentIndex(index);
}
// Otherwise 650 MB will be default file split size.
else {
//Use max RIFF size (4GB) as default index, since usually people don't want to split.
comboBoxSplitting->setCurrentIndex(4);
}

// Read CUEfile info
CheckBoxRecordCueFile->setChecked(
Expand Down Expand Up @@ -220,15 +223,15 @@ void DlgPrefRecord::slotRecordPathChange() {
void DlgPrefRecord::slotResetToDefaults() {
m_pRadioWav->setChecked(true);
CheckBoxRecordCueFile->setChecked(false);
// 650MB splitting is the default
comboBoxSplitting->setCurrentIndex(0);
// 4GB splitting is the default
comboBoxSplitting->setCurrentIndex(4);

LineEditTitle->setText("");
LineEditAlbum->setText("");
LineEditAuthor->setText("");

// 6 corresponds to 128kbps (only used by MP3 and Ogg though)
SliderQuality->setValue(6);
// 1 corresponds to 16 bits (WAVE/AIFF)
SliderQuality->setValue(1);
}

// This function updates/refreshes the contents of this dialog.
Expand Down
1 change: 0 additions & 1 deletion src/preferences/dialog/dlgprefrecord.h
Expand Up @@ -61,7 +61,6 @@ class DlgPrefRecord : public DlgPreferencePage, public Ui::DlgPrefRecordDlg {

// Pointer to config object
UserSettingsPointer m_pConfig;
ControlProxy* m_pRecordControl;
bool m_bConfirmOverwrite;
QString fileTypeExtension;
QRadioButton* m_pRadioOgg;
Expand Down
18 changes: 11 additions & 7 deletions src/recording/defs_recording.h
Expand Up @@ -11,21 +11,25 @@
#define RECORD_OFF 0.0
#define RECORD_READY 1.0
#define RECORD_ON 2.0
#define RECORD_SPLIT_CONTINUE 3.0

//File options for preferences Splitting
#define SPLIT_650MB "650 MB (CD)"
#define SPLIT_700MB "700 MB (CD)"
#define SPLIT_1024MB "1 GB"
#define SPLIT_2048MB "2 GB"
#define SPLIT_4096MB "4 GB"
#define SPLIT_60MIN "60 Minutes"
#define SPLIT_74MIN "74 Minutes (CD)"
#define SPLIT_80MIN "80 Minutes (CD)"
#define SPLIT_120MIN "120 Minutes"

// Byte conversions Instead of multiplying megabytes with 1024 to get kilobytes
// I use 1000 Once the recording size has reached there's enough room to add
// Byte conversions. Slightly rounded to leave enough room to add
// closing frames by the encoder. All sizes are in bytes.
#define SIZE_650MB Q_UINT64_C(650000000)
#define SIZE_700MB Q_UINT64_C(750000000)
#define SIZE_1GB Q_UINT64_C(1000000000)
#define SIZE_2GB Q_UINT64_C(2000000000)
#define SIZE_4GB Q_UINT64_C(4000000000)
#define SIZE_650MB Q_UINT64_C(680000000)
#define SIZE_700MB Q_UINT64_C(730000000)
#define SIZE_1GB Q_UINT64_C(1070000000)
#define SIZE_2GB Q_UINT64_C(2140000000)
#define SIZE_4GB Q_UINT64_C(4280000000)

#endif