Skip to content

AudioSensor::AddSource uses out-of-bounds Vector3 index for source position #2632

Description

@DongDoroDongDong

Description

I found what appears to be an out-of-bounds indexing bug in AudioSensor.cpp when passing the audio source position to RLRAudioPropagation::AddSource.

The source position is stored as a Magnum::Vector3, but it is later passed to RLR using indices [1], [2], [3]:

audioSimulator_->AddSource(RLRAudioPropagation::Vector3f{
    lastSourcePos_[1], lastSourcePos_[2], lastSourcePos_[3]});

Since Magnum::Vector3 has three components, the valid indices should be [0], [1], [2]. Accessing lastSourcePos_[3] appears to be out-of-bounds and may result in undefined behavior.

Relevant Code

File:

src/esp/sensor/AudioSensor.cpp

The source position is stored here:

void AudioSensor::setAudioSourceTransform(const Magnum::Vector3& sourcePos) {
  ESP_DEBUG() << logHeader_ << "Setting the audio source position : " << sourcePos << "]";
  lastSourcePos_ = sourcePos;
  newSource_ = true;
}

Then, in AudioSensor::runSimulation(...), it is passed to RLR here:

if (newSource_) {
  newSource_ = false;
  ESP_DEBUG() << logHeader_ << "Adding source at position : " << lastSourcePos_;
  audioSimulator_->AddSource(RLRAudioPropagation::Vector3f{
      lastSourcePos_[1], lastSourcePos_[2], lastSourcePos_[3]});
}

For comparison, the listener position in the same file is passed using [0], [1], [2]:

audioSimulator_->AddListener(
    RLRAudioPropagation::Vector3f{agentPos[0], agentPos[1], agentPos[2]},
    RLRAudioPropagation::Quaternion{agentRotQuat[0], agentRotQuat[1],
                                    agentRotQuat[2], agentRotQuat[3]},
    audioSensorSpec_->channelLayout_);

Expected Behavior

The source position should likely be passed as:

audioSimulator_->AddSource(RLRAudioPropagation::Vector3f{
    lastSourcePos_[0], lastSourcePos_[1], lastSourcePos_[2]});

Actual Behavior

The current code passes:

{lastSourcePos_[1], lastSourcePos_[2], lastSourcePos_[3]}

This effectively shifts the source coordinates and reads one invalid element:

expected: [x, y, z]
actual:   [y, z, out-of-bounds]

Potential Impact

This may affect spatial audio simulation results, especially for:

  • RIR generation
  • Ambisonics / FOA rendering
  • Sound source localization
  • Direction-of-arrival estimation
  • Audio-visual localization datasets generated from Habitat-Sim

In my use case, this may explain large directional errors when using Habitat-Sim-generated FOA/RIR data.

Suggested Fix

Change:

audioSimulator_->AddSource(RLRAudioPropagation::Vector3f{
    lastSourcePos_[1], lastSourcePos_[2], lastSourcePos_[3]});

to:

audioSimulator_->AddSource(RLRAudioPropagation::Vector3f{
    lastSourcePos_[0], lastSourcePos_[1], lastSourcePos_[2]});

Question

Is there any intentional coordinate convention behind using indices [1], [2], [3]` here?

If not, this appears to be an out-of-bounds indexing bug.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions