Skip to content

Commit

Permalink
bugfix: Rework allocation policy & fix crash
Browse files Browse the repository at this point in the history
There was a crash during ISource/ISink deallocation caused by
virtual inheritance in IDevice (we were passing incorrect
address to IArena).

This commit reworks and simplifies allocation policies to avoid
such problems:

  - to use object with ScopedPtr, object should be inherited from
    ArenaAllocation, PoolAllocation, or NoopAllocation

  - to use object with SharedPtr, object should be inherited from
    RefCounted<ArenaAllocation>, RefCounted<PoolAllocation>, or
    RefCounted<NoopAllocation>

  - FuncAllocation was removed; new class is introduced instead:
    ScopedRelease

  - object may override dispose() method of allocation policy;
    default implementation returns object to arena/pool

  - if virtual inheritance is used, dispose() should be overridden
    to pass correct object address to arena/pool, otherwise default
    implementation is fine

  - IDevice forces derived classes to override dispose()
  • Loading branch information
gavv committed Aug 13, 2024
1 parent 3e57e33 commit 39e29db
Show file tree
Hide file tree
Showing 143 changed files with 1,229 additions and 996 deletions.
2 changes: 1 addition & 1 deletion 3rdparty/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ thirdparty_versions = {
'sox': '14.4.2',
'speexdsp': '1.2.0',

# CLI tools
# cli tools
'gengetopt': '2.22.6',
'ragel': '6.10',

Expand Down
20 changes: 18 additions & 2 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,7 @@ if meta.compiler == 'gcc':
])

if meta.compiler_ver[:2] >= (10, 0):
# enable
for var in ['CXXFLAGS', 'CFLAGS']:
env.Append(**{var: [
'-Wdouble-promotion',
Expand All @@ -1065,6 +1066,11 @@ if meta.compiler == 'gcc':
'-Woverlength-strings',
'-Wsign-conversion',
]})
else:
# disable
env.Append(CXXFLAGS=[
'-Wno-reorder',
])

if meta.compiler == 'clang':
for var in ['CXXFLAGS', 'CFLAGS']:
Expand Down Expand Up @@ -1099,15 +1105,25 @@ if meta.compiler == 'clang':
]})

env.Append(CXXFLAGS=[
'-Wno-invalid-offsetof',
# enable
'-Wnon-virtual-dtor',

# disable
'-Wno-invalid-offsetof',
])

if meta.platform in ['darwin', 'android'] or meta.compiler_ver[:2] < (11, 0):
if meta.platform not in ['darwin', 'android'] and meta.compiler_ver[:2] >= (11, 0):
# enable
pass
else:
# disable
for var in ['CXXFLAGS', 'CFLAGS']:
env.Append(**{var: [
'-Wno-unknown-warning-option',
]})
env.Append(CXXFLAGS=[
'-Wno-reorder',
])

if meta.compiler in ['gcc', 'clang']:
for var in ['CXXFLAGS', 'CFLAGS']:
Expand Down
9 changes: 5 additions & 4 deletions src/internal_modules/roc_audio/beep_plc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
namespace roc {
namespace audio {

BeepPlc::BeepPlc(core::IArena& arena,
BeepPlc::BeepPlc(const PlcConfig& config,
const SampleSpec& sample_spec,
FrameFactory& frame_factory,
const PlcConfig& config,
const SampleSpec& sample_spec)
: sample_spec_(sample_spec)
core::IArena& arena)
: IPlc(arena)
, sample_spec_(sample_spec)
, signal_pos_(0) {
if (!sample_spec_.is_valid() || !sample_spec_.is_raw()) {
roc_panic("beep plc: required valid sample specs with raw format: spec=%s",
Expand Down
6 changes: 3 additions & 3 deletions src/internal_modules/roc_audio/beep_plc.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ namespace audio {
class BeepPlc : public IPlc {
public:
//! Initialize.
BeepPlc(core::IArena& arena,
BeepPlc(const PlcConfig& config,
const SampleSpec& sample_spec,
FrameFactory& frame_factory,
const PlcConfig& config,
const SampleSpec& sample_spec);
core::IArena& arena);

virtual ~BeepPlc();

Expand Down
8 changes: 4 additions & 4 deletions src/internal_modules/roc_audio/builtin_resampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ inline size_t get_frame_size(size_t window_size,

} // namespace

BuiltinResampler::BuiltinResampler(core::IArena& arena,
FrameFactory& frame_factory,
const ResamplerConfig& config,
BuiltinResampler::BuiltinResampler(const ResamplerConfig& config,
const SampleSpec& in_spec,
const SampleSpec& out_spec)
const SampleSpec& out_spec,
FrameFactory& frame_factory,
core::IArena& arena)
: IResampler(arena)
, in_spec_(in_spec)
, out_spec_(out_spec)
Expand Down
8 changes: 4 additions & 4 deletions src/internal_modules/roc_audio/builtin_resampler.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ namespace audio {
class BuiltinResampler : public IResampler, public core::NonCopyable<> {
public:
//! Initialize.
BuiltinResampler(core::IArena& arena,
FrameFactory& frame_factory,
const ResamplerConfig& config,
BuiltinResampler(const ResamplerConfig& config,
const SampleSpec& in_spec,
const SampleSpec& out_spec);
const SampleSpec& out_spec,
FrameFactory& frame_factory,
core::IArena& arena);

~BuiltinResampler();

Expand Down
6 changes: 3 additions & 3 deletions src/internal_modules/roc_audio/decimation_resampler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const size_t InputFrameSize = 16;

DecimationResampler::DecimationResampler(
const core::SharedPtr<IResampler>& inner_resampler,
core::IArena& arena,
FrameFactory& frame_factory,
const SampleSpec& in_spec,
const SampleSpec& out_spec)
const SampleSpec& out_spec,
FrameFactory& frame_factory,
core::IArena& arena)
: IResampler(arena)
, inner_resampler_(inner_resampler)
, use_inner_resampler_(in_spec.sample_rate() != out_spec.sample_rate())
Expand Down
6 changes: 3 additions & 3 deletions src/internal_modules/roc_audio/decimation_resampler.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ class DecimationResampler : public IResampler, public core::NonCopyable<> {
public:
//! Initialize.
DecimationResampler(const core::SharedPtr<IResampler>& inner_resampler,
core::IArena& arena,
FrameFactory& frame_factory,
const SampleSpec& in_spec,
const SampleSpec& out_spec);
const SampleSpec& out_spec,
FrameFactory& frame_factory,
core::IArena& arena);

~DecimationResampler();

Expand Down
6 changes: 3 additions & 3 deletions src/internal_modules/roc_audio/fanout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
namespace roc {
namespace audio {

Fanout::Fanout(FrameFactory& frame_factory,
core::IArena& arena,
const SampleSpec& sample_spec)
Fanout::Fanout(const SampleSpec& sample_spec,
FrameFactory& frame_factory,
core::IArena& arena)
: outputs_(arena)
, sample_spec_(sample_spec)
, init_status_(status::NoStatus) {
Expand Down
6 changes: 3 additions & 3 deletions src/internal_modules/roc_audio/fanout.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ namespace audio {
class Fanout : public IFrameWriter, public core::NonCopyable<> {
public:
//! Initialize.
Fanout(FrameFactory& frame_factory,
core::IArena& arena,
const SampleSpec& sample_spec);
Fanout(const SampleSpec& sample_spec,
FrameFactory& frame_factory,
core::IArena& arena);

//! Check if the object was successfully constructed.
status::StatusCode init_status() const;
Expand Down
4 changes: 4 additions & 0 deletions src/internal_modules/roc_audio/iframe_decoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
namespace roc {
namespace audio {

IFrameDecoder::IFrameDecoder(core::IArena& arena)
: core::ArenaAllocation(arena) {
}

IFrameDecoder::~IFrameDecoder() {
}

Expand Down
6 changes: 5 additions & 1 deletion src/internal_modules/roc_audio/iframe_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ namespace roc {
namespace audio {

//! Audio frame decoder interface.
class IFrameDecoder {
class IFrameDecoder : public core::ArenaAllocation {
public:
//! Initialize.
explicit IFrameDecoder(core::IArena& arena);

//! Deinitialize.
virtual ~IFrameDecoder();

//! Check if the object was successfully constructed.
Expand Down
4 changes: 4 additions & 0 deletions src/internal_modules/roc_audio/iframe_encoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
namespace roc {
namespace audio {

IFrameEncoder::IFrameEncoder(core::IArena& arena)
: core::ArenaAllocation(arena) {
}

IFrameEncoder::~IFrameEncoder() {
}

Expand Down
6 changes: 5 additions & 1 deletion src/internal_modules/roc_audio/iframe_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@ namespace roc {
namespace audio {

//! Audio frame encoder interface.
class IFrameEncoder {
class IFrameEncoder : public core::ArenaAllocation {
public:
//! Initialize.
explicit IFrameEncoder(core::IArena& arena);

//! Deinitialize.
virtual ~IFrameEncoder();

//! Check if the object was successfully constructed.
Expand Down
4 changes: 4 additions & 0 deletions src/internal_modules/roc_audio/iplc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
namespace roc {
namespace audio {

IPlc::IPlc(core::IArena& arena)
: core::ArenaAllocation(arena) {
}

IPlc::~IPlc() {
}

Expand Down
7 changes: 5 additions & 2 deletions src/internal_modules/roc_audio/iplc.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,12 @@ namespace audio {
//!
//! PLC implementation is allowed to use arbitrary PCM format, specified
//! by its sample_spec() method.
class IPlc {
class IPlc : public core::ArenaAllocation {
public:
//! Deinitialization.
//! Initialize.
explicit IPlc(core::IArena& arena);

//! Deinitialize.
virtual ~IPlc();

//! Check if the object was successfully constructed.
Expand Down
8 changes: 4 additions & 4 deletions src/internal_modules/roc_audio/mixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
namespace roc {
namespace audio {

Mixer::Mixer(FrameFactory& frame_factory,
core::IArena& arena,
const SampleSpec& sample_spec,
bool enable_timestamps)
Mixer::Mixer(const SampleSpec& sample_spec,
bool enable_timestamps,
FrameFactory& frame_factory,
core::IArena& arena)
: frame_factory_(frame_factory)
, inputs_(arena)
, sample_spec_(sample_spec)
Expand Down
8 changes: 4 additions & 4 deletions src/internal_modules/roc_audio/mixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ class Mixer : public IFrameReader, public core::NonCopyable<> {
public:
//! Initialize.
//! @p enable_timestamps defines whether to enable calculation of capture timestamps.
Mixer(FrameFactory& frame_factory,
core::IArena& arena,
const SampleSpec& sample_spec,
bool enable_timestamps);
Mixer(const SampleSpec& sample_spec,
bool enable_timestamps,
FrameFactory& frame_factory,
core::IArena& arena);

//! Check if the object was successfully constructed.
status::StatusCode init_status() const;
Expand Down
9 changes: 5 additions & 4 deletions src/internal_modules/roc_audio/pcm_decoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@
namespace roc {
namespace audio {

IFrameDecoder* PcmDecoder::construct(core::IArena& arena, const SampleSpec& sample_spec) {
return new (arena) PcmDecoder(sample_spec);
IFrameDecoder* PcmDecoder::construct(const SampleSpec& sample_spec, core::IArena& arena) {
return new (arena) PcmDecoder(sample_spec, arena);
}

PcmDecoder::PcmDecoder(const SampleSpec& sample_spec)
: pcm_mapper_(sample_spec.pcm_format(), Sample_RawFormat)
PcmDecoder::PcmDecoder(const SampleSpec& sample_spec, core::IArena& arena)
: IFrameDecoder(arena)
, pcm_mapper_(sample_spec.pcm_format(), Sample_RawFormat)
, n_chans_(sample_spec.num_channels())
, stream_pos_(0)
, stream_avail_(0)
Expand Down
4 changes: 2 additions & 2 deletions src/internal_modules/roc_audio/pcm_decoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ namespace audio {
class PcmDecoder : public IFrameDecoder, public core::NonCopyable<> {
public:
//! Construction function.
static IFrameDecoder* construct(core::IArena& arena, const SampleSpec& sample_spec);
static IFrameDecoder* construct(const SampleSpec& sample_spec, core::IArena& arena);

//! Initialize.
PcmDecoder(const SampleSpec& sample_spec);
PcmDecoder(const SampleSpec& sample_spec, core::IArena& arena);

//! Check if the object was successfully constructed.
virtual status::StatusCode init_status() const;
Expand Down
9 changes: 5 additions & 4 deletions src/internal_modules/roc_audio/pcm_encoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
namespace roc {
namespace audio {

IFrameEncoder* PcmEncoder::construct(core::IArena& arena, const SampleSpec& sample_spec) {
return new (arena) PcmEncoder(sample_spec);
IFrameEncoder* PcmEncoder::construct(const SampleSpec& sample_spec, core::IArena& arena) {
return new (arena) PcmEncoder(sample_spec, arena);
}

PcmEncoder::PcmEncoder(const SampleSpec& sample_spec)
: pcm_mapper_(Sample_RawFormat, sample_spec.pcm_format())
PcmEncoder::PcmEncoder(const SampleSpec& sample_spec, core::IArena& arena)
: IFrameEncoder(arena)
, pcm_mapper_(Sample_RawFormat, sample_spec.pcm_format())
, n_chans_(sample_spec.num_channels())
, frame_data_(NULL)
, frame_byte_size_(0)
Expand Down
4 changes: 2 additions & 2 deletions src/internal_modules/roc_audio/pcm_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ namespace audio {
class PcmEncoder : public IFrameEncoder, public core::NonCopyable<> {
public:
//! Construction function.
static IFrameEncoder* construct(core::IArena& arena, const SampleSpec& sample_spec);
static IFrameEncoder* construct(const SampleSpec& sample_spec, core::IArena& arena);

//! Initialize.
PcmEncoder(const SampleSpec& sample_spec);
PcmEncoder(const SampleSpec& sample_spec, core::IArena& arena);

//! Check if the object was successfully constructed.
virtual status::StatusCode init_status() const;
Expand Down
Loading

0 comments on commit 39e29db

Please sign in to comment.