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.

Sign up for GitHub

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

Sample Track Recording with Jack backend #7567

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Sample Track Recording with Jack backend #7567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/AudioEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ class LMMS_EXPORT AudioEngine : public QObject
return m_audioDev;
}

bool captureDeviceAvailable() const;


// audio-port-stuff
inline void addAudioPort(AudioPort * port)
Expand Down Expand Up @@ -341,6 +343,11 @@ class LMMS_EXPORT AudioEngine : public QObject
AudioDevice * tryAudioDevices();
MidiClient * tryMidiClients();

const AudioDevice* audioDev() const
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggesting to put the whole function in one line to save space (at least the coding conventions allow it).

{
return m_audioDev;
}

void renderStageNoteSetup();
void renderStageInstruments();
void renderStageEffects();
Expand Down
4 changes: 4 additions & 0 deletions include/AudioJack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ private slots:

int processCallback(jack_nframes_t nframes);

static int setBufferSizeCallback(jack_nframes_t nframes, void* udata);
static int staticProcessCallback(jack_nframes_t nframes, void* udata);
static void shutdownCallback(void* _udata);

Expand All @@ -110,8 +111,11 @@ private slots:

std::atomic<MidiJack*> m_midiClient;
std::vector<jack_port_t*> m_outputPorts;
std::vector<jack_port_t*> m_inputPorts;
jack_default_audio_sample_t** m_tempOutBufs;
jack_default_audio_sample_t* m_inputFrameBuffer;
SampleFrame* m_outBuf;
SampleFrame* m_inBuf;

f_cnt_t m_framesDoneInCurBuf;
f_cnt_t m_framesToDoInCurBuf;
Expand Down
1 change: 1 addition & 0 deletions include/SampleClipView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public slots:
void mouseDoubleClickEvent( QMouseEvent * ) override;
void paintEvent( QPaintEvent * ) override;

bool recordingCapabilitiesAvailable() const;

private:
SampleClip * m_clip;
Expand Down
5 changes: 4 additions & 1 deletion include/SampleRecordHandle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Track;
class SampleRecordHandle : public PlayHandle
{
public:
SampleRecordHandle( SampleClip* clip );
SampleRecordHandle(SampleClip* clip, TimePos startRecordTimeOffset);
~SampleRecordHandle() override;

void play( SampleFrame* _working_buffer ) override;
Expand All @@ -70,6 +70,9 @@ class SampleRecordHandle : public PlayHandle
PatternTrack* m_patternTrack;
SampleClip * m_clip;

// The offset from the start of m_track that the record has
// started from.
TimePos m_startRecordTimeOffset;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TimePos m_startRecordTimeOffset;
const TimePos m_startRecordTimeOffset;

} ;


Expand Down
14 changes: 9 additions & 5 deletions src/core/AudioEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ void AudioEngine::renderStageNoteSetup()

if( it != m_playHandles.end() )
{
( *it )->audioPort()->removePlayHandle( ( *it ) );
if ((*it)->audioPort()) { (*it)->audioPort()->removePlayHandle(*it); }
if( ( *it )->type() == PlayHandle::Type::NotePlayHandle )
{
NotePlayHandleManager::release( (NotePlayHandle*) *it );
Expand Down Expand Up @@ -389,7 +389,7 @@ void AudioEngine::renderStageEffects()
}
if( ( *it )->isFinished() )
{
( *it )->audioPort()->removePlayHandle( ( *it ) );
if ((*it)->audioPort()) { (*it)->audioPort()->removePlayHandle(*it); }
if( ( *it )->type() == PlayHandle::Type::NotePlayHandle )
{
NotePlayHandleManager::release( (NotePlayHandle*) *it );
Expand Down Expand Up @@ -581,6 +581,10 @@ void AudioEngine::restoreAudioDevice()
}


bool AudioEngine::captureDeviceAvailable() const
{
return audioDev()->supportsCapture();
}


void AudioEngine::removeAudioPort(AudioPort * port)
Expand All @@ -604,7 +608,7 @@ bool AudioEngine::addPlayHandle( PlayHandle* handle )
if (handle->type() == PlayHandle::Type::InstrumentPlayHandle || !criticalXRuns())
{
m_newPlayHandles.push( handle );
handle->audioPort()->addPlayHandle( handle );
if (handle->audioPort()) { handle->audioPort()->addPlayHandle(handle); }
return true;
}

Expand All @@ -625,7 +629,7 @@ void AudioEngine::removePlayHandle(PlayHandle * ph)
// which were created in a thread different than the audio engine thread
if (ph->affinityMatters() && ph->affinity() == QThread::currentThread())
{
ph->audioPort()->removePlayHandle(ph);
if (ph->audioPort()) { ph->audioPort()->removePlayHandle(ph); }
bool removedFromList = false;
// Check m_newPlayHandles first because doing it the other way around
// creates a race condition
Expand Down Expand Up @@ -684,7 +688,7 @@ void AudioEngine::removePlayHandlesOfTypes(Track * track, PlayHandle::Types type
{
if ((*it)->isFromTrack(track) && ((*it)->type() & types))
{
( *it )->audioPort()->removePlayHandle( ( *it ) );
if ((*it)->audioPort()) { (*it)->audioPort()->removePlayHandle(*it); }
if( ( *it )->type() == PlayHandle::Type::NotePlayHandle )
{
NotePlayHandleManager::release( (NotePlayHandle*) *it );
Expand Down
12 changes: 11 additions & 1 deletion src/core/DataFile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,17 @@ bool DataFile::copyResources(const QString& resourcesDir)
{
// Get absolute path to resource
bool error;
QString resPath = PathUtil::toAbsolute(el.attribute(*res), &error);
auto attribute = el.attribute(*res);

// Skip empty resources. In some cases, there might be an
// alternative way of actually representing the data (e.g. data element for SampleTCO)
if (attribute.isEmpty())
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please explain exactly why this was necessary to be in this PR?

Copy link
Contributor

@JohannesLorenz JohannesLorenz Dec 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, this is mentioned in #5990 : #5990 (comment) . I still wonder if it could break anything else?

{
++res;
continue;
}

QString resPath = PathUtil::toAbsolute(attribute, &error);
// If we are running without the project loaded (from CLI), "local:" base
// prefixes aren't converted, so we need to convert it ourselves
if (error)
Expand Down
3 changes: 2 additions & 1 deletion src/core/PlayHandle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ PlayHandle::PlayHandle(const Type type, f_cnt_t offset) :
m_affinity(QThread::currentThread()),
m_playHandleBuffer(BufferManager::acquire()),
m_bufferReleased(true),
m_usesBuffer(true)
m_usesBuffer(true),
m_audioPort{nullptr}
{
}

Expand Down
9 changes: 6 additions & 3 deletions src/core/SampleRecordHandle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@ namespace lmms
{


SampleRecordHandle::SampleRecordHandle( SampleClip* clip ) :
PlayHandle( Type::SamplePlayHandle ),
SampleRecordHandle::SampleRecordHandle(SampleClip* clip, TimePos startRecordTimeOffset) :
PlayHandle(Type::SamplePlayHandle),
m_framesRecorded( 0 ),
m_minLength( clip->length() ),
m_track( clip->getTrack() ),
m_patternTrack( nullptr ),
m_clip( clip )
m_clip(clip),
m_startRecordTimeOffset{startRecordTimeOffset}
{
}

Expand All @@ -53,6 +54,8 @@ SampleRecordHandle::~SampleRecordHandle()
{
if (!m_buffers.empty()) { m_clip->setSampleBuffer(createSampleBuffer()); }

m_clip->setStartTimeOffset(m_startRecordTimeOffset);

while( !m_buffers.empty() )
{
delete[] m_buffers.front().first;
Expand Down
30 changes: 29 additions & 1 deletion src/core/audio/AudioJack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ AudioJack::AudioJack(bool& successful, AudioEngine* audioEngineParam)
, m_midiClient(nullptr)
, m_tempOutBufs(new jack_default_audio_sample_t*[channels()])
, m_outBuf(new SampleFrame[audioEngine()->framesPerPeriod()])
, m_inBuf(new SampleFrame[audioEngine()->framesPerPeriod()])
, m_framesDoneInCurBuf(0)
, m_framesToDoInCurBuf(0)
{
Expand All @@ -66,6 +67,8 @@ AudioJack::AudioJack(bool& successful, AudioEngine* audioEngineParam)
if (successful) {
connect(this, SIGNAL(zombified()), this, SLOT(restartAfterZombified()), Qt::QueuedConnection);
}

m_supportsCapture = true;
}


Expand All @@ -90,6 +93,7 @@ AudioJack::~AudioJack()
delete[] m_tempOutBufs;

delete[] m_outBuf;
delete[] m_inBuf;
}


Expand Down Expand Up @@ -154,6 +158,10 @@ bool AudioJack::initJackClient()
clientName.toLatin1().constData(), jack_get_client_name(m_client));
}

m_inputFrameBuffer = new jack_default_audio_sample_t[channels() * jack_get_buffer_size(m_client)];

jack_set_buffer_size_callback(m_client, setBufferSizeCallback, this);

// set process-callback
jack_set_process_callback(m_client, staticProcessCallback, this);

Expand All @@ -167,6 +175,10 @@ bool AudioJack::initJackClient()
QString name = QString("master out ") + ((ch % 2) ? "R" : "L") + QString::number(ch / 2 + 1);
m_outputPorts.push_back(
jack_port_register(m_client, name.toLatin1().constData(), JACK_DEFAULT_AUDIO_TYPE, JackPortIsOutput, 0));

QString input_name = QString("master in ") + ((ch % 2) ? "R" : "L") + QString::number(ch / 2 + 1);
m_inputPorts.push_back(jack_port_register(m_client, input_name.toLatin1().constData(), JACK_DEFAULT_AUDIO_TYPE, JackPortIsInput, 0));

if (m_outputPorts.back() == nullptr)
{
printf("no more JACK-ports available!\n");
Expand Down Expand Up @@ -286,11 +298,17 @@ void AudioJack::renamePort(AudioPort* port)
}


int AudioJack::setBufferSizeCallback(jack_nframes_t nframes, void* udata)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if this is allowed to do allocations? I just ask because some callbacks must happen in realtime-context and thus are not allowed to do slow things like new[].

My guess is that realtime-safety does not matter when a buffer size changes, but I could not tell from the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

According to Perplexity AI it is called in a non-realtime thread. However, I wasn't able to find that information in the pointers it gave either. 🤔 So it should be taken with a big grain of salt.

On the other hand, it would be strange if that information was communicated on the realtime audio thread because it is very likely that clients need to allocate memory as well if that event happens.

Perplexitiy blurb Yes, the callback that informs about changed buffer sizes in the JACK audio API is called outside of the audio thread. This callback, known as the buffer size callback, is executed in a separate non-real-time thread[1][2].

The buffer size callback is registered using the jack_set_buffer_size_callback function. It is called whenever the size of the buffer that will be passed to the process callback is about to change[1]. This callback is designed to allow clients that depend on knowing the buffer size to prepare for the change before it occurs.

It's important to note that:

  1. The buffer size callback is not called from within the real-time audio processing thread.
  2. The normal process cycle is suspended during the execution of this callback, causing a gap in the audio flow[2].
  3. Because it's not in the real-time thread, the callback can perform operations that are not real-time safe, such as allocating memory or touching previously unreferenced memory[2].

This design allows for more flexibility in handling buffer size changes without interfering with the real-time audio processing thread, which has strict timing requirements.

Citations:
[1] https://jackaudio.org/api/group__ClientCallbacks.html
[2] https://jackclient-python.readthedocs.io/en/0.3.0/
[3] spatialaudio/jackclient-python#81
[4] https://docs.rs/jack/latest/jack/struct.Client.html
[5] https://stackoverflow.com/questions/78429446/which-thread-handles-a-realtime-callback-in-jack-audio/78882670
[6] https://linuxmusicians.com/viewtopic.php?t=22620
[7] https://devforum.play.date/t/c-api-sound-callbacks-happen-on-another-thread/4414
[8] https://www.rncbc.org/drupal/node/2162

{
auto thisClass = static_cast<AudioJack*>(udata);
delete[] thisClass->m_inputFrameBuffer;
thisClass->m_inputFrameBuffer = new jack_default_audio_sample_t[thisClass->channels() * nframes];
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider to use an std::vector<jack_default_audio_sample_t> that's simply resized in this block. This would also spare you the manual deletion in the destructor and would thus make the code a bit more memory safe.

return 0;
}


int AudioJack::processCallback(jack_nframes_t nframes)
{

// do midi processing first so that midi input can
// add to the following sound processing
if (m_midiClient && nframes > 0)
Expand Down Expand Up @@ -356,6 +374,16 @@ int AudioJack::processCallback(jack_nframes_t nframes)
}
}

for (int c = 0; c < channels(); ++c)
{
jack_default_audio_sample_t* jack_input_buffer = (jack_default_audio_sample_t*) jack_port_get_buffer(m_inputPorts[c], nframes);

for (jack_nframes_t frame = 0; frame < nframes; frame++)
{
m_inputFrameBuffer[frame * channels() + c] = jack_input_buffer[frame];
}
}
audioEngine()->pushInputFrames ((SampleFrame*) m_inputFrameBuffer, nframes);
return 0;
}

Expand Down
49 changes: 37 additions & 12 deletions src/gui/clips/SampleClipView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
#include <QMenu>
#include <QPainter>

#include "AudioEngine.h"
#include "GuiApplication.h"
#include "AutomationEditor.h"
#include "embed.h"
#include "FontHelper.h"
#include "PathUtil.h"
#include "SampleClip.h"
#include "SampleLoader.h"
Expand Down Expand Up @@ -77,9 +79,12 @@ void SampleClipView::constructContextMenu(QMenu* cm)
{
cm->addSeparator();

/*contextMenu.addAction( embed::getIconPixmap( "record" ),
tr( "Set/clear record" ),
m_clip, SLOT(toggleRecord()));*/

QAction* recordToggleAction = cm->addAction(embed::getIconPixmap("record"),
tr("Set/clear record"),
m_clip, &SampleClip::toggleRecord);

recordToggleAction->setEnabled(recordingCapabilitiesAvailable());

cm->addAction(
embed::getIconPixmap("flip_x"),
Expand Down Expand Up @@ -306,18 +311,34 @@ void SampleClipView::paintEvent( QPaintEvent * pe )
}
// recording sample tracks is not possible at the moment

/* if( m_clip->isRecord() )
if (m_clip->isRecord())
{
p.setFont( pointSize<7>( p.font() ) );
p.setFont(adjustedToPixelSize(p.font(), 10));

const auto fontHeight = p.fontMetrics().height();

const auto baseLine = height() - 3;

constexpr int recordSymbolRadius = 3;
constexpr int recordSymbolCenterX = recordSymbolRadius + 4;
const int recordSymbolCenterY = baseLine - fontHeight / 2 + 1;

constexpr int textStartX = recordSymbolCenterX + recordSymbolRadius + 4;

auto textPos = QPoint(textStartX, baseLine);

p.setPen( textShadowColor() );
p.drawText( 10, p.fontMetrics().height()+1, "Rec" );
p.setPen( textColor() );
p.drawText( 9, p.fontMetrics().height(), "Rec" );
const auto rec = tr("Rec");

p.setBrush( QBrush( textColor() ) );
p.drawEllipse( 4, 5, 4, 4 );
}*/
p.setPen(textShadowColor());
p.drawText(textPos + QPoint(1, 1), rec);

p.setPen(textColor());
p.drawText(textPos, rec);

p.setBrush(QBrush(textColor()));

p.drawEllipse(QPoint(recordSymbolCenterX, recordSymbolCenterY), recordSymbolRadius, recordSymbolRadius);
}

p.end();

Expand Down Expand Up @@ -376,5 +397,9 @@ bool SampleClipView::splitClip( const TimePos pos )
else { return false; }
}

bool SampleClipView::recordingCapabilitiesAvailable() const
{
return Engine::audioEngine()->captureDeviceAvailable();
}

} // namespace lmms::gui
14 changes: 12 additions & 2 deletions src/gui/editors/SongEditor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include <QTimeLine>

#include "ActionGroup.h"
#include "AudioDevice.h"
#include "AudioEngine.h"
#include "AutomatableSlider.h"
#include "ClipView.h"
Expand Down Expand Up @@ -912,7 +911,7 @@ ComboBoxModel *SongEditor::snappingModel() const


SongEditorWindow::SongEditorWindow(Song* song) :
Editor(Engine::audioEngine()->audioDev()->supportsCapture(), false),
Editor(true, false),
m_editor(new SongEditor(song)),
m_crtlAction( nullptr ),
m_snapSizeLabel( new QLabel( m_toolBar ) )
Expand Down Expand Up @@ -1029,6 +1028,17 @@ SongEditorWindow::SongEditorWindow(Song* song) :

connect(song, SIGNAL(projectLoaded()), this, SLOT(adjustUiAfterProjectLoad()));
connect(this, SIGNAL(resized()), m_editor, SLOT(updatePositionLine()));

// In case our current audio device does not support capture,
// disable the record buttons.
if (!Engine::audioEngine()->captureDeviceAvailable())
{
for (auto &recordAction : {m_recordAccompanyAction, m_recordAction})
Copy link
Contributor

Choose a reason for hiding this comment

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

I was able to test with the "m_recordAccompanyAction" (I guess that's the record button with the play sign), but when I pressed m_recordAction (I guess that's the record button without play sign), just nothing happened - the cursor did not move and nothing was recorded.

Do you know why? What steps are required to make any use of m_recordAction?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this is also mentioned in #5990 .

{
recordAction->setEnabled(false);
recordAction->setToolTip(tr("Recording is unavailable: try connecting an input device or switching backend"));
}
}
}

QSize SongEditorWindow::sizeHint() const
Expand Down
Loading
Loading