[runtime/onnxruntime] AudioFrame copy constructor missing data/global_start/global_end (rule-of-three violation)

[wangxiuwen]

Summary

AudioFrame::AudioFrame(const AudioFrame &other) in runtime/onnxruntime/src/audio.cpp is incomplete — it only copies 4 of the 7 logical fields, missing data, global_start, and global_end. Combined with the destructor's free(data) this creates a latent bug.

Location

runtime/onnxruntime/src/audio.cpp (current main), lines 153-159:

AudioFrame::AudioFrame(const AudioFrame &other)
{
    start = other.start;
    end = other.end;
    len = other.len;
    is_final = other.is_final;
    // ← data, global_start, global_end not copied
}

The class declares float* data = nullptr; int global_start = 0; int global_end = 0; (header), and the destructor frees data:

AudioFrame::~AudioFrame(){
    if(data != nullptr){
        free(data);
        data = nullptr;
    }
}

Why it's currently latent

• Because of the in-class default data = nullptr, the new copy has data == nullptr, so the destructor's if(data != nullptr) skips the free. No double-free, no immediate crash.
• All current uses pass AudioFrame* (the runtime stores them in queues by pointer; see audio.h frame_queue / asr_online_queue / asr_offline_queue), so the copy ctor never fires in practice.

But the class fails the Rule of Three: there's a non-trivial dtor (frees data), so the copy ctor and copy assignment must also be defined consistently. As written, anyone who refactors a queue to hold AudioFrame by value, returns one by value, or stores them in a std::vector<AudioFrame> will silently get frames with mismatched fields and missing audio data, with no crash to alert them.

global_start / global_end are also semantically required for downstream timestamp computation (used in FunTpassInferBuffer at frame->global_start).

Proposed fix

Deep-copy data and copy the missing timestamp fields:

 AudioFrame::AudioFrame(const AudioFrame &other)
 {
     start = other.start;
     end = other.end;
     len = other.len;
     is_final = other.is_final;
+    global_start = other.global_start;
+    global_end = other.global_end;
+    if (other.data != nullptr && other.len > 0) {
+        data = (float*)malloc(sizeof(float) * other.len);
+        memcpy(data, other.data, sizeof(float) * other.len);
+    }
 }

(The rest of the rule-of-three — copy assignment — is also currently absent. Adding it explicitly, or =delete if value-copy is meant to be unsupported, would make the class' ownership semantics explicit.)

Why fix it now if it doesn't crash today

Latent semantic bugs in fundamental types like AudioFrame are exactly the ones that bite during refactors years later, when the original author is gone and the next person assumes the class behaves like every other ordinary value type.