Skip to content

Conversation

@jatinchowdhury18
Copy link
Contributor

No description provided.

@jatinchowdhury18 jatinchowdhury18 requested a review from Copilot June 10, 2025 01:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refines multichannel IR loading and adds a new helper to clear unused state segments.

  • Refactors load_ir to zero-pad each FFT segment after copying input data.
  • Updates load_multichannel_ir to track and assign segment counts across channels.
  • Introduces reset_process_state_segments to clear leftover segments in the process state.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
chowdsp_convolution.h Declares new reset_process_state_segments function.
chowdsp_convolution.cpp Refactors load_ir, adjusts multichannel IR logic, and implements reset_process_state_segments.
Comments suppressed due to low confidence (3)

chowdsp_convolution.h:145

  • The doc comment is identical to reset_process_state. Update it to explain that this function zeros only the unused segments in the state based on the IR's segment count.
/** Zeros the process state. */

chowdsp_convolution.cpp:131

  • [nitpick] The name new_num_segments is ambiguous—consider renaming to something like last_channel_segments or computed_segments to reflect its purpose.
int new_num_segments = 0;

chowdsp_convolution.cpp:219

  • There are no tests covering reset_process_state_segments. Consider adding unit tests to confirm that unused state segments are zeroed as expected.
void reset_process_state_segments (const Convolution_Config* config, Process_Uniform_State* state, const IR_Uniform* ir)

.max_num_segments = ir->max_num_segments,
.num_channels = 1,
};
load_ir (config, &this_channel_ir, ir_data[ch], ir_num_samples, fft_scratch);
Copy link

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

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

This assignment overwrites the segment count on each channel without checking consistency. If different channels have different segment counts, the final value may be incorrect. Add an assert or verify that this_channel_ir.num_segments matches previous channels before updating.

Suggested change
load_ir (config, &this_channel_ir, ir_data[ch], ir_num_samples, fft_scratch);
load_ir (config, &this_channel_ir, ir_data[ch], ir_num_samples, fft_scratch);
if (ch == 0)
{
expected_num_segments = this_channel_ir.num_segments; // Set expected value
}
else
{
assert(this_channel_ir.num_segments == expected_num_segments && "Inconsistent num_segments across channels");
}

Copilot uses AI. Check for mistakes.
@jatinchowdhury18 jatinchowdhury18 merged commit 2d92fb9 into main Jun 10, 2025
6 checks passed
@jatinchowdhury18 jatinchowdhury18 deleted the multichannel-ir-fixes branch June 10, 2025 01:09
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.

2 participants