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

Remove audio engine from exporting code #7452

Draft
wants to merge 70 commits into
base: master
Choose a base branch
from

Conversation

szeli1
Copy link
Contributor

@szeli1 szeli1 commented Aug 14, 2024

potentially closes #7449

Project folder needs to export to its own audio and AudioEngine does not allow this.

This pull request removes AudioEngine and AudioDevice from AudioFileDevice so the exporting code does not depend on it. This way in the future a Project folder can provide its own audio buffers simplifying the exporting process.
A new class is added to handle only exporting code. It is called "LmmsExporter" because in the future it might need to support exporting other datatypes.

AudioEngine::doSetAudioDevice is now unused.

Needs testing.

TODO:

  • remove qDebug
  • remove TODO
  • check includes

@szeli1 szeli1 marked this pull request as draft August 14, 2024 20:51
Copy link
Contributor

@Rossmaxx Rossmaxx left a comment

Choose a reason for hiding this comment

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

This is what I got for now.

const fpp_t _frames,
int_sample_t * _output_buffer,
const bool _convert_endian = false );

// clear given signed-int-16-buffer
void clearS16Buffer( int_sample_t * _outbuf,
const fpp_t _frames );
static void clearS16Buffer(int_sample_t * _outbuf,
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
static void clearS16Buffer(int_sample_t * _outbuf,
static void clearS16Buffer(int_sample_t* _outbuf,

static AudioFileDevice* getInst(
OutputSettings const &outputSettings,
bool &successful,
const QString & outputFilename,
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
const QString & outputFilename,
const QString& outputFilename,

static AudioFileDevice* getInst(
OutputSettings const &outputSettings,
bool &successful,
const QString & outputFilename,
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
const QString & outputFilename,
const QString& outputFilename,

@@ -240,6 +240,15 @@ void AudioEngine::stopProcessing()
}


void AudioEngine::startExporting(const struct qualitySettings& qs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't there be a finishExporting too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Else there's chances of lmms becoming unresponsive after export.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finish exporting would run AudioEngine::stopProcessing(). I could add in that function if you want.

Copy link
Contributor

Choose a reason for hiding this comment

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

If on finish, processing is stopped, then why do you need to stop processing on starting export too?

@@ -242,5 +97,50 @@ void ProjectRenderer::updateConsoleProgress()
fflush( stderr );
}

// gets a buffer and some data as input
Copy link
Contributor

Choose a reason for hiding this comment

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

I think saker said on discord about this. The comment should go in the header file as

/*
* First line
* Second line
* ... 
*/

format

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't it look like this?:

/**
* First line
* Second line
* ... 
*/

And does this mean that I need to rewrite every comment I wrote in my PRs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this is what i meant, and regarding re writing, don't rewrite everything, just the ones you use to describe the function.

if (newBuffer != nullptr && Engine::getSong()->isExportDone() == false)
{
// copy new buffer to bufferOut
for (size_t i = 0; i < bufferOut->size(); i++)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe there's a std::copy for this.

{
const unsigned int channels = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

If this doesn't change at all, better make it constexpr

{
auto temp = static_cast<int_sample_t>(AudioEngine::clip(_ab[frame][chnl]) * OUTPUT_SAMPLE_MULTIPLIER);

( _output_buffer + frame * channels() )[chnl] =
( _output_buffer + frame * channels )[chnl] =
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
( _output_buffer + frame * channels )[chnl] =
(_output_buffer + frame * channels)[chnl] =

Copy link
Contributor

Choose a reason for hiding this comment

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

Btw what's this used for?

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 don't know.

But audio exporting classes use it.

}




void AudioDevice::clearS16Buffer( int_sample_t * _outbuf, const fpp_t _frames )
void AudioDevice::clearS16Buffer(int_sample_t * _outbuf, const fpp_t _frames)
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
void AudioDevice::clearS16Buffer(int_sample_t * _outbuf, const fpp_t _frames)
void AudioDevice::clearS16Buffer(int_sample_t* _outbuf, const fpp_t _frames)

OutputSettings::BitDepth bitDepth = getOutputSettings().getBitDepth();

if( bitDepth == OutputSettings::BitDepth::Depth32Bit || bitDepth == OutputSettings::BitDepth::Depth24Bit )
{
auto buf = new float[_frames * channels()];
auto buf = new float[_frames * channels];
Copy link
Contributor

Choose a reason for hiding this comment

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

Std::array?

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.

4 participants