-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add FILE line to Cuefile to include file path for each track #13365
base: main
Are you sure you want to change the base?
Changes from 9 commits
84caae4
9b9daca
c0fadc1
d6a42bb
827c956
7ae6cb7
5a07163
b2327a0
c534739
77cc748
213bfdb
4d6590f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -18,7 +18,8 @@ EngineRecord::EngineRecord(UserSettingsPointer pConfig) | |||||
m_recordedDuration(0), | ||||||
m_iMetaDataLife(0), | ||||||
m_cueTrack(0), | ||||||
m_bCueIsEnabled(false) { | ||||||
m_bCueIsEnabled(false), | ||||||
m_bCueUsesFileAnnotation(false) { | ||||||
m_pRecReady = new ControlProxy(RECORDING_PREF_KEY, "status", this); | ||||||
m_sampleRate = mixxx::audio::SampleRate::fromDouble(m_sampleRateControl.get()); | ||||||
} | ||||||
|
@@ -36,6 +37,7 @@ int EngineRecord::updateFromPreferences() { | |||||
m_baAlbum = m_pConfig->getValueString(ConfigKey(RECORDING_PREF_KEY, "Album")); | ||||||
m_cueFileName = m_pConfig->getValueString(ConfigKey(RECORDING_PREF_KEY, "CuePath")); | ||||||
m_bCueIsEnabled = m_pConfig->getValueString(ConfigKey(RECORDING_PREF_KEY, "CueEnabled")).toInt(); | ||||||
m_bCueUsesFileAnnotation = m_pConfig->getValueString(ConfigKey(RECORDING_PREF_KEY, "CueFileAnnotationEnabled")).toInt(); | ||||||
m_sampleRate = mixxx::audio::SampleRate::fromDouble(m_sampleRateControl.get()); | ||||||
|
||||||
// Delete m_pEncoder if it has been initialized (with maybe) different bitrate. | ||||||
|
@@ -249,6 +251,12 @@ void EngineRecord::writeCueLine() { | |||||
.arg(m_pCurrentTrack->getArtist()) | ||||||
.toUtf8()); | ||||||
|
||||||
if (m_bCueUsesFileAnnotation) { | ||||||
m_cueFile.write(QString(" FILE \"%1\"\n") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Feel free to also fix other occurrence There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just pushed new commit that should take care of this. I updated the line you suggested as well as the ones above it. Thanks! |
||||||
.arg(m_pCurrentTrack->getLocation()) | ||||||
.toUtf8()); | ||||||
} | ||||||
|
||||||
// Woefully inaccurate (at the seconds level anyways). | ||||||
// We'd need a signal fired state tracker | ||||||
// for the track detection code. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,9 @@ DlgPrefRecord::DlgPrefRecord(QWidget* parent, UserSettingsPointer pConfig) | |
CheckBoxRecordCueFile->setChecked(m_pConfig->getValue<bool>( | ||
ConfigKey(RECORDING_PREF_KEY, "CueEnabled"), kDefaultCueEnabled)); | ||
|
||
CheckBoxUseCueFileAnnotation->setChecked(m_pConfig->getValue<bool>( | ||
ConfigKey(RECORDING_PREF_KEY, "cue_file_annotation_enabled"), false)); | ||
|
||
// Setting split | ||
comboBoxSplitting->addItem(SPLIT_650MB); | ||
comboBoxSplitting->addItem(SPLIT_700MB); | ||
|
@@ -119,6 +122,11 @@ DlgPrefRecord::DlgPrefRecord(QWidget* parent, UserSettingsPointer pConfig) | |
&QAbstractSlider::sliderReleased, | ||
this, | ||
&DlgPrefRecord::slotSliderCompression); | ||
|
||
connect(CheckBoxRecordCueFile, | ||
&QCheckBox::stateChanged, | ||
this, | ||
&DlgPrefRecord::slotToggleCueEnabled); | ||
} | ||
|
||
DlgPrefRecord::~DlgPrefRecord() { | ||
|
@@ -144,6 +152,7 @@ void DlgPrefRecord::slotApply() { | |
saveMetaData(); | ||
saveEncoding(); | ||
saveUseCueFile(); | ||
saveUseCueFileAnnotation(); | ||
saveSplitSize(); | ||
} | ||
|
||
|
@@ -175,10 +184,12 @@ void DlgPrefRecord::slotUpdate() { | |
|
||
loadMetaData(); | ||
|
||
// Setting miscellaneous | ||
// Setting miscellaneous | ||
CheckBoxRecordCueFile->setChecked(m_pConfig->getValue<bool>( | ||
ConfigKey(RECORDING_PREF_KEY, "CueEnabled"), kDefaultCueEnabled)); | ||
|
||
updateCueEnabled(); | ||
|
||
QString fileSizeStr = m_pConfig->getValueString(ConfigKey(RECORDING_PREF_KEY, "FileSize")); | ||
int index = comboBoxSplitting->findText(fileSizeStr); | ||
if (index >= 0) { | ||
|
@@ -201,7 +212,12 @@ void DlgPrefRecord::slotResetToDefaults() { | |
|
||
// 4GB splitting is the default | ||
comboBoxSplitting->setCurrentIndex(4); | ||
|
||
// Sets 'Create a CUE file' checkbox value | ||
CheckBoxRecordCueFile->setChecked(kDefaultCueEnabled); | ||
|
||
// Sets 'Enable File Annotation in CUE file' checkbox value | ||
CheckBoxUseCueFileAnnotation->setChecked(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you capture that default value in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I did this originally and then scrapped it when I realized I could keep it as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This helps with readability mainly - all defaults get listed at the top of the file, and this helps to understand why this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense, thanks! Just pushed new commit that should take care of this. |
||
} | ||
|
||
void DlgPrefRecord::slotBrowseRecordingsDir() { | ||
|
@@ -303,6 +319,11 @@ void DlgPrefRecord::slotSliderQuality() { | |
// Settings are only stored when doing an apply so that "cancel" can actually cancel. | ||
} | ||
|
||
// Set 'Enable File Annotation in CUE file' checkbox value depending on 'Create a CUE file' checkbox value | ||
void DlgPrefRecord::updateCueEnabled() { | ||
CheckBoxUseCueFileAnnotation->setEnabled(CheckBoxRecordCueFile->isChecked()); | ||
} | ||
|
||
void DlgPrefRecord::updateTextQuality() { | ||
EncoderRecordingSettingsPointer settings = | ||
EncoderFactory::getFactory().getEncoderRecordingSettings( | ||
|
@@ -429,11 +450,20 @@ void DlgPrefRecord::saveEncoding() { | |
} | ||
} | ||
|
||
void DlgPrefRecord::slotToggleCueEnabled() { | ||
updateCueEnabled(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these two methods are redundant. I would suggest keeping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, I've made this change but I'll wait til we resolve the others to push again. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just pushed new commit for this. |
||
|
||
void DlgPrefRecord::saveUseCueFile() { | ||
m_pConfig->set(ConfigKey(RECORDING_PREF_KEY, "CueEnabled"), | ||
ConfigValue(CheckBoxRecordCueFile->isChecked())); | ||
} | ||
|
||
void DlgPrefRecord::saveUseCueFileAnnotation() { | ||
m_pConfig->set(ConfigKey(RECORDING_PREF_KEY, "CueFileAnnotationEnabled"), | ||
ConfigValue(CheckBoxUseCueFileAnnotation->isChecked())); | ||
} | ||
|
||
void DlgPrefRecord::saveSplitSize() { | ||
m_pConfig->set(ConfigKey(RECORDING_PREF_KEY, "FileSize"), | ||
ConfigValue(comboBoxSplitting->currentText())); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you might be able to use a
QStringLiteral
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this one, fyi all the
m_cueFile.write
lines are existing code and use QString, I just added the if statement on line 254. I assumed whoever wrote that code used QString for a reason but idk.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QStringLiteral
is aQString
:) (doc)The value of a
QStringLiteral
is that it will create theQString
at compilte time, instead of instantiating at every function calls. This is a macro optimisation.The TL;DR is: if you use a static string in your
QString
(like here with" FILE \"%1\"\n"
), you can make it aQStringLiteral
.You are correct that the one above could also be using
QStringLiteral
tho! Feel free to update them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation, I will update mine and the ones above. Just pushed new commit that should take care of this.