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

Add optional sample rate conversion to audio::TargetFile and audio::BufferRecorderNode #1869

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add optional sample rate conversion to audio::TargetFile and audio::BufferRecorderNode #1869

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7d8a439
Add sample rate conversion to TargetFile and BufferRecorderNode (Coco…
Jul 9, 2017
af4b0e6
Make default destructor TargetFile more explicit
Jul 18, 2017
89ad51f
Adjust to style guide
Aug 8, 2017
8202cda
Target: change getSampleRateTarget() to getSampleRateNative()
Aug 16, 2017
1d1bba6
Target: add supportsConversion()
Aug 16, 2017
a9ea7ed
Target: forgot to change sampleRateNative on MSW
Aug 16, 2017
8f0036e
Merge remote-tracking branch 'base/master' into target_sample_rate
Sep 3, 2017
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
4 changes: 2 additions & 2 deletions include/cinder/audio/SampleRecorderNode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ class CI_API BufferRecorderNode : public SampleRecorderNode {

//! \brief Writes the currently recorded samples to a file at \a filePath
//!
//! The encoding format is derived from \a filePath's extension and \a sampleType (default = SampleType::INT_16).
//! The encoding format is derived from \a filePath's extension, \a sampleType (default = SampleType::INT_16) and \a destSampleRate (default = getSampleRate()).
//! \note throws AudioFileExc if the write request cannot be completed.
void writeToFile( const ci::fs::path &filePath, SampleType sampleType = SampleType::INT_16 );
void writeToFile( const ci::fs::path &filePath, SampleType sampleType = SampleType::INT_16, size_t destSampleRate = 0 );

//! Returns the frame of the last buffer overrun or 0 if none since the last time this method was called. When this happens, it means the recorded buffer probably has skipped some frames.
uint64_t getLastOverrun();
Expand Down
26 changes: 19 additions & 7 deletions include/cinder/audio/Target.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,30 +32,42 @@ namespace cinder { namespace audio {

typedef std::shared_ptr<class TargetFile> TargetFileRef;

namespace dsp {
class Converter;
}

//! Base class that is used to create and write to an audio destination. Currently only supports .wav encoding.
class CI_API TargetFile {
public:
static std::unique_ptr<TargetFile> create( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType = SampleType::INT_16, const std::string &extension = "" );
static std::unique_ptr<TargetFile> create( const fs::path &path, size_t sampleRate, size_t numChannels, SampleType sampleType = SampleType::INT_16, const std::string &extension = "" );
virtual ~TargetFile() {}
static std::unique_ptr<TargetFile> create( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType = SampleType::INT_16, size_t targetSampleRate = 0, const std::string &extension = "" );
static std::unique_ptr<TargetFile> create( const fs::path &path, size_t sampleRate, size_t numChannels, SampleType sampleType = SampleType::INT_16, size_t targetSampleRate = 0, const std::string &extension = "" );
virtual ~TargetFile();

void write( const Buffer *buffer );
void write( const Buffer *buffer, size_t numFrames );
void write( const Buffer *buffer, size_t numFrames, size_t frameOffset );

//! Returns the user facing sample rate (input)
size_t getSampleRate() const { return mSampleRate; }
//! Returns the true sample rate of the target file. \note Actual input samplerate may differ. \see getSampleRate()
size_t getSampleRateTarget() const { return mSampleRateTarget; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think it's worth naming this getSampleRateNative() to be consistent with SourceFile? I can't say I like one name over the other, though it is slightly ambiguous that the class is called TargetFile and the method has the noun 'target' in in.

Copy link
Author

Choose a reason for hiding this comment

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

For me both seem reasonable. I personally didn't find getSampleRateTarget() ambiguous since I thought of it as "get the sample rate of the target [file]". But I can definitely see the consistency argument with SourceFile.


size_t getNumChannels() const { return mNumChannels; }

protected:
TargetFile( size_t sampleRate, size_t numChannels, SampleType sampleType )
: mSampleRate( sampleRate ), mNumChannels( numChannels ), mSampleType( sampleType )
{}
TargetFile( size_t sampleRate, size_t numChannels, SampleType sampleType, size_t destSampleRate = 0 );

// Implement to write \a numFrames frames of \a buffer to file. The writing begins at \a frameOffset.
virtual void performWrite( const Buffer *buffer, size_t numFrames, size_t frameOffset ) = 0;

size_t mSampleRate, mNumChannels;
// Sets up samplerate conversion if needed.
void setupSampleRateConversion();

size_t mSampleRate, mSampleRateTarget, mNumChannels, mMaxFramesPerConversion;
SampleType mSampleType;

std::unique_ptr<dsp::Converter> mConverter;
BufferDynamic mConverterSourceBuffer, mConverterDestBuffer;
};

} } // namespace cinder::audio
2 changes: 1 addition & 1 deletion include/cinder/audio/cocoa/FileCoreAudio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class SourceFileCoreAudio : public SourceFile {

class TargetFileCoreAudio : public TargetFile {
public:
TargetFileCoreAudio( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType, const std::string &extension );
TargetFileCoreAudio( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType, size_t targetSampleRate, const std::string &extension );
virtual ~TargetFileCoreAudio() {}

void performWrite( const Buffer *buffer, size_t numFrames, size_t frameOffset ) override;
Expand Down
4 changes: 2 additions & 2 deletions src/cinder/audio/SampleRecorderNode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,12 @@ BufferRef BufferRecorderNode::getRecordedCopy() const
return mCopiedBuffer;
}

void BufferRecorderNode::writeToFile( const fs::path &filePath, SampleType sampleType )
void BufferRecorderNode::writeToFile( const fs::path &filePath, SampleType sampleType, size_t destSampleRate )
{
size_t currentWritePos = mWritePos;
BufferRef copiedBuffer = getRecordedCopy();

audio::TargetFileRef target = audio::TargetFile::create( filePath, getSampleRate(), getNumChannels(), sampleType );
audio::TargetFileRef target = audio::TargetFile::create( filePath, getSampleRate(), getNumChannels(), sampleType, destSampleRate );
target->write( copiedBuffer.get(), currentWritePos );
}

Expand Down
57 changes: 50 additions & 7 deletions src/cinder/audio/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
*/

#include "cinder/audio/Target.h"
#include "cinder/audio/dsp/Converter.h"
#include "cinder/CinderAssert.h"

#include "cinder/Utilities.h"
Expand All @@ -38,7 +39,7 @@ namespace cinder { namespace audio {

// TODO: these should be replaced with a generic registrar derived from the ImageIo stuff.

std::unique_ptr<TargetFile> TargetFile::create( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType, const std::string &extension )
std::unique_ptr<TargetFile> TargetFile::create( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType, size_t sampleRateTarget, const std::string &extension )
{
#if ! defined( CINDER_UWP ) || ( _MSC_VER > 1800 )
std::string ext = dataTarget->getFilePathHint().extension().string();
Expand All @@ -48,20 +49,40 @@ std::unique_ptr<TargetFile> TargetFile::create( const DataTargetRef &dataTarget,
ext = ( ( ! ext.empty() ) && ( ext[0] == '.' ) ) ? ext.substr( 1, string::npos ) : ext;

#if defined( CINDER_COCOA )
return std::unique_ptr<TargetFile>( new cocoa::TargetFileCoreAudio( dataTarget, sampleRate, numChannels, sampleType, ext ) );
return std::unique_ptr<TargetFile>( new cocoa::TargetFileCoreAudio( dataTarget, sampleRate, numChannels, sampleType, sampleRateTarget, ext ) );
#elif defined( CINDER_MSW )
CI_ASSERT_MSG( sampleRateTarget == 0 || sampleRateTarget == sampleRate, "sample rate conversion not yet implemented on MSW" );
return std::unique_ptr<TargetFile>( new msw::TargetFileMediaFoundation( dataTarget, sampleRate, numChannels, sampleType, ext ) );
#endif
}

std::unique_ptr<TargetFile> TargetFile::create( const fs::path &path, size_t sampleRate, size_t numChannels, SampleType sampleType, const std::string &extension )
std::unique_ptr<TargetFile> TargetFile::create( const fs::path &path, size_t sampleRate, size_t numChannels, SampleType sampleType, size_t targetSampleRate, const std::string &extension )
{
return create( (DataTargetRef)writeFile( path ), sampleRate, numChannels, sampleType, extension );
return create( (DataTargetRef)writeFile( path ), sampleRate, numChannels, sampleType, targetSampleRate, extension );
}

TargetFile::TargetFile( size_t sampleRate, size_t numChannels, SampleType sampleType, size_t sampleRateTarget )
: mSampleRate( sampleRate ), mNumChannels( numChannels ), mSampleType( sampleType ), mSampleRateTarget( sampleRateTarget ), mMaxFramesPerConversion( 4092 )
{
setupSampleRateConversion();
}

TargetFile::~TargetFile() = default;

void TargetFile::setupSampleRateConversion()
{
if ( ! mSampleRateTarget ) {
mSampleRateTarget = mSampleRate;
} else if ( mSampleRateTarget != mSampleRate) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style nit: else statements should begin on a new line (apologies, this wasn't in the CONTRIBUTING.md, but I'm going to add it as it is something we've all agreed upon AFAIK).

Copy link
Author

Choose a reason for hiding this comment

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

I've just fixed this with 89ad51f.
I've also removed spaces between if/while and their braces. This isn't mentioned in the CONTRIBUTING.md either but is done very consistently in the code.

mConverter = audio::dsp::Converter::create( mSampleRate, mSampleRateTarget, getNumChannels(), getNumChannels(), mMaxFramesPerConversion );
mConverterSourceBuffer.setSize( mMaxFramesPerConversion, getNumChannels() );
mConverterDestBuffer.setSize( mMaxFramesPerConversion, getNumChannels() );
}
}

void TargetFile::write( const Buffer *buffer )
{
performWrite( buffer, buffer->getNumFrames(), 0 );
write( buffer, buffer->getNumFrames(), 0 );
}

void TargetFile::write( const Buffer *buffer, size_t numFrames )
Expand All @@ -71,7 +92,7 @@ void TargetFile::write( const Buffer *buffer, size_t numFrames )

CI_ASSERT_MSG( numFrames <= buffer->getNumFrames(), "numFrames out of bounds" );

performWrite( buffer, numFrames, 0 );
write( buffer, numFrames, 0 );
}

void TargetFile::write( const Buffer *buffer, size_t numFrames, size_t frameOffset )
Expand All @@ -81,7 +102,29 @@ void TargetFile::write( const Buffer *buffer, size_t numFrames, size_t frameOffs

CI_ASSERT_MSG( numFrames + frameOffset <= buffer->getNumFrames(), "numFrames + frameOffset out of bounds" );

performWrite( buffer, numFrames, frameOffset );
if( mConverter ) {
auto currFrame = frameOffset;
auto lastFrame = frameOffset + numFrames;

// process buffer in chunks of mMaxFramesPerConversion
while ( currFrame != lastFrame ) {
auto numSourceFrames = std::min( mMaxFramesPerConversion, lastFrame - currFrame );
auto numDestFrames = size_t( numSourceFrames * (float)getSampleRateTarget() / (float)getSampleRate() );

// copy buffer into temporary buffer to remove frame offset (needed for mConverter->convert)
mConverterSourceBuffer.copyOffset( *buffer, numSourceFrames, 0, currFrame );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Concerning this and your comment in the PR description about this being a bit hacky, I hope that in the future this problem can be alleviated once audio::BufferViews are implemented (see #1701). I think for now a TODO comment linking back to that would suffice. The copy is quite cheap compared to the actual encoding, but obviously eventually we want to do as little extra copying as possible.


mConverterSourceBuffer.setNumFrames( numSourceFrames );
mConverterDestBuffer.setNumFrames( numDestFrames );
tie( numSourceFrames, numDestFrames ) = mConverter->convert( &mConverterSourceBuffer, &mConverterDestBuffer );

performWrite( &mConverterDestBuffer, numDestFrames, 0 );

currFrame += numSourceFrames;
}
} else {
performWrite( buffer, numFrames, frameOffset );
}
}

} } // namespace cinder::audio
10 changes: 5 additions & 5 deletions src/cinder/audio/cocoa/FileCoreAudio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,19 +147,19 @@ vector<string> SourceFileCoreAudio::getSupportedExtensions()
// MARK: - TargetFileCoreAudio
// ----------------------------------------------------------------------------------------------------

TargetFileCoreAudio::TargetFileCoreAudio( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType, const std::string &extension )
: TargetFile( sampleRate, numChannels, sampleType )
TargetFileCoreAudio::TargetFileCoreAudio( const DataTargetRef &dataTarget, size_t sampleRate, size_t numChannels, SampleType sampleType, size_t targetSampleRate, const std::string &extension )
: TargetFile( sampleRate, numChannels, sampleType, targetSampleRate )
{
::CFURLRef targetUrl = ci::cocoa::createCfUrl( Url( dataTarget->getFilePath().string() ) );
::AudioFileTypeID fileType = getFileTypeIdFromExtension( extension );

::AudioStreamBasicDescription fileAsbd;
switch( mSampleType ) {
case SampleType::INT_16:
fileAsbd = createInt16Asbd( mSampleRate, mNumChannels, true );
fileAsbd = createInt16Asbd( mSampleRateTarget, mNumChannels, true );
break;
case SampleType::FLOAT_32:
fileAsbd = createFloatAsbd( mSampleRate, mNumChannels, true );
fileAsbd = createFloatAsbd( mSampleRateTarget, mNumChannels, true );
break;
default:
CI_ASSERT_NOT_REACHABLE();
Expand All @@ -175,7 +175,7 @@ TargetFileCoreAudio::TargetFileCoreAudio( const DataTargetRef &dataTarget, size_
::CFRelease( targetUrl );
mExtAudioFile = ExtAudioFilePtr( audioFile );

::AudioStreamBasicDescription clientAsbd = createFloatAsbd( mSampleRate, mNumChannels, false );
::AudioStreamBasicDescription clientAsbd = createFloatAsbd( mSampleRateTarget, mNumChannels, false );

status = ::ExtAudioFileSetProperty( mExtAudioFile.get(), kExtAudioFileProperty_ClientDataFormat, sizeof( clientAsbd ), &clientAsbd );
CI_VERIFY( status == noErr );
Expand Down