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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions