Skip to content

optimize unnecessary copy #573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
theomonnom wants to merge 3 commits into
base: main
Choose a base branch
from
+22 −9
Open

optimize unnecessary copy #573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4b89ac
wip
theomonnom Feb 10, 2026
5ddfccd
fix types
theomonnom Feb 10, 2026
5e7e3bf
Update audio_frame.py
theomonnom Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions livekit-rtc/livekit/rtc/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,18 @@ def task_done_logger(task: asyncio.Task) -> None:
return


def get_address(mv: memoryview) -> int:
return ctypes.addressof(ctypes.c_char.from_buffer(mv))
def get_address(data) -> int:
if isinstance(data, memoryview):
if not data.readonly:
return ctypes.addressof(ctypes.c_char.from_buffer(data))
data = data.obj
Copy link

@devin-ai-integration devin-ai-integration bot Feb 10, 2026

Choose a reason for hiding this comment

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

🔴 get_address loses slice offset for readonly sliced memoryviews via .obj unwrapping

When get_address receives a readonly sliced memoryview, data = data.obj on line 47 returns the full underlying buffer, discarding the slice offset. The returned address points to the start of the full buffer, not the start of the slice.

Root Cause and Impact

The get_address function handles readonly memoryviews by falling through to data = data.obj, which returns the original underlying object (e.g., a bytes object). For a sliced readonly memoryview like memoryview(b'hello world')[2:6], .obj returns the full b'hello world', and the address returned is of the start of b'hello world', not of index 2.

This affects VideoFrame._proto_info() at livekit-rtc/livekit/rtc/video_frame.py:113 and VideoFrame.get_plane() at line 147, where get_address(self.data) is called with memoryview(self._data). If a user constructs a VideoFrame with a sliced readonly memoryview (e.g., VideoFrame(..., data=memoryview(some_bytes)[offset:])), the FFI layer receives the wrong data pointer, leading to incorrect video frame processing or memory access issues.

The writable (non-readonly) path correctly handles slices via ctypes.c_char.from_buffer(data), which respects the memoryview's buffer protocol including offset.

Prompt for agents 
In livekit-rtc/livekit/rtc/_utils.py line 47, `data = data.obj` discards the slice offset for readonly memoryviews. To fix this, you need to handle readonly sliced memoryviews properly. One approach: convert the readonly memoryview to bytes first via `data = bytes(data)` which preserves the slice content. Alternatively, use `ctypes.c_char.from_buffer_copy(data)` to get the address of a copy, though this adds a copy. The cleanest fix may be to convert to bytes: replace line 47 with `data = bytes(data)`, which correctly captures just the sliced region.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

if isinstance(data, bytearray):
return ctypes.addressof(ctypes.c_char.from_buffer(data))
if isinstance(data, bytes):
addr = ctypes.cast(ctypes.c_char_p(data), ctypes.c_void_p).value
assert addr is not None
return addr
raise TypeError(f"expected bytes, bytearray, or memoryview, got {type(data)}")
Comment on lines +43 to +54
Copy link

@devin-ai-integration devin-ai-integration bot Feb 10, 2026

Choose a reason for hiding this comment

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

🔴 get_address falls through to TypeError for read-only memoryview backed by non-bytes/bytearray object

When get_address receives a read-only memoryview whose .obj is not bytes or bytearray (e.g., a memoryview of another memoryview, or a read-only buffer from a third-party library), it raises TypeError instead of returning the correct address.

Root Cause

At _utils.py:44-47, when a read-only memoryview is passed, the code does data = data.obj and then checks only for bytearray and bytes. If .obj is any other type (e.g., another memoryview, a numpy array, or a ctypes array), the function falls through to the raise TypeError at line 54.

For example, memoryview(memoryview(some_bytes)) creates a memoryview whose .obj is the inner memoryview, not the bytes object. This would cause:

get_address(memoryview(memoryview(b"hello")))
# data.readonly = True → data = data.obj (inner memoryview)
# not bytearray, not bytes → TypeError

This can be triggered indirectly through apm.py:55 where get_address(memoryview(bdata)) wraps an already-memoryview bdata in another memoryview, or through audio_resampler.py:102.

Impact: TypeError raised when processing audio through APM or resampler with certain data types.

Suggested change
def get_address(data) -> int:
if isinstance(data, memoryview):
if not data.readonly:
return ctypes.addressof(ctypes.c_char.from_buffer(data))
data = data.obj
if isinstance(data, bytearray):
return ctypes.addressof(ctypes.c_char.from_buffer(data))
if isinstance(data, bytes):
addr = ctypes.cast(ctypes.c_char_p(data), ctypes.c_void_p).value
assert addr is not None
return addr
raise TypeError(f"expected bytes, bytearray, or memoryview, got {type(data)}")
def get_address(data) -> int:
if isinstance(data, memoryview):
if not data.readonly:
return ctypes.addressof(ctypes.c_char.from_buffer(data))
obj = data.obj
if isinstance(obj, (bytes, bytearray)):
pass # fall through to handle obj below
else:
# For other backing types, convert to bytes to get address
addr = ctypes.cast(ctypes.c_char_p(bytes(data)), ctypes.c_void_p).value
assert addr is not None
return addr
data = obj
if isinstance(data, bytearray):
return ctypes.addressof(ctypes.c_char.from_buffer(data))
if isinstance(data, bytes):
addr = ctypes.cast(ctypes.c_char_p(data), ctypes.c_void_p).value
assert addr is not None
return addr
raise TypeError(f"expected bytes, bytearray, or memoryview, got {type(data)}")
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.



T = TypeVar("T")
Expand Down
15 changes: 9 additions & 6 deletions livekit-rtc/livekit/rtc/audio_frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,22 @@ def __init__(
Raises:
ValueError: If the length of `data` is smaller than the required size.
"""
data = memoryview(data).cast("B")
if isinstance(data, memoryview):
data = data.obj # type: ignore[assignment]
Comment on lines +52 to +53
Copy link

@devin-ai-integration devin-ai-integration bot Feb 10, 2026

Choose a reason for hiding this comment

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

🔴 AudioFrame.init using memoryview.obj loses slice offset and length information

When a sliced memoryview is passed to AudioFrame.__init__, data.obj returns the full underlying buffer object, discarding the slice's offset and length. This causes the frame to reference the wrong region of memory.

Root Cause and Impact

The old code did data = memoryview(data).cast("B") which preserved slice information — a sliced memoryview would remain sliced. The new code at line 53 does data = data.obj which always returns the original underlying buffer object, regardless of any slicing.

For example:

ba = bytearray(1000)
mv = memoryview(ba)[100:200]  # 100 bytes starting at offset 100
# Old: memoryview(mv).cast("B") → len=100, points to ba[100:200] ✓
# New: mv.obj → ba itself, len=1000, points to ba[0:1000] ✗

This causes two problems:

  1. Length validation is wrong: len(data) returns the full buffer length, so the size check at line 58 may pass when the actual slice is too small.
  2. Wrong memory address in FFI calls: _proto_info() at audio_frame.py:103 calls get_address(memoryview(self._data)) which returns the address of the start of the full buffer, not the slice offset. The FFI layer then reads/writes the wrong memory region.

Impact: Silent data corruption or out-of-bounds memory access when AudioFrame is constructed from a sliced memoryview.

Prompt for agents 
In audio_frame.py lines 52-53, instead of unconditionally unwrapping memoryview.obj, you need to preserve the data view correctly. The safest approach is to convert the memoryview to bytes or bytearray while preserving the slice boundaries. For example, replace:

    if isinstance(data, memoryview):
        data = data.obj

with something like:

    if isinstance(data, memoryview):
        if data.readonly:
            data = bytes(data)
        else:
            data = bytearray(data)

Alternatively, if the goal is zero-copy, you could check whether the memoryview is contiguous and non-sliced (data.contiguous and data.nbytes == len(data.obj)) before using data.obj, and fall back to copying otherwise. But this adds complexity. The simplest correct fix is to only use data.obj when the memoryview covers the entire underlying object.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


if len(data) < num_channels * samples_per_channel * ctypes.sizeof(ctypes.c_int16):
min_size = num_channels * samples_per_channel * ctypes.sizeof(ctypes.c_int16)
data_len = len(data)

if data_len < min_size:
raise ValueError(
"data length must be >= num_channels * samples_per_channel * sizeof(int16)"
)

if len(data) % ctypes.sizeof(ctypes.c_int16) != 0:
if data_len % ctypes.sizeof(ctypes.c_int16) != 0:
# can happen if data is bigger than needed
raise ValueError("data length must be a multiple of sizeof(int16)")

n = len(data) // ctypes.sizeof(ctypes.c_int16)
self._data = (ctypes.c_int16 * n).from_buffer_copy(data)
self._data = data
Copy link

@devin-ai-integration devin-ai-integration bot Feb 10, 2026

Choose a reason for hiding this comment

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

🔴 AudioFrame with immutable bytes data breaks in-place modification by APM

When AudioFrame is constructed with bytes data, the FFI layer in apm.py process_stream/process_reverse_stream writes to immutable memory, causing undefined behavior.

Root Cause and Impact

Previously, AudioFrame.__init__ always created a mutable copy via (ctypes.c_int16 * n).from_buffer_copy(data), so self._data was always a writable ctypes array. The new code stores the original data object directly (self._data = data at audio_frame.py:67), which can be immutable bytes.

The process_stream method in apm.py:51-55 gets a pointer to the frame's data and passes it to the FFI layer which modifies the audio in-place:

bdata = data.data.cast("b")  # read-only memoryview if _data is bytes
req.apm_process_stream.data_ptr = get_address(memoryview(bdata))

The get_address function at _utils.py:50-53 obtains the address of the bytes object via ctypes.c_char_p, and the FFI writes to that address. Writing to bytes memory is undefined behavior in CPython — it bypasses immutability guarantees and can corrupt string interning, hash caches, or cause segfaults.

This is confirmed by the existing test at tests/rtc/test_apm.py:61-76 which creates AudioFrames from bytes (via .tobytes()) and then calls apm.process_stream() on them, expecting in-place modification.

Impact: Silent data corruption (processed audio not actually written back) or crashes when using AudioProcessingModule with bytes-backed AudioFrames. The test test_audio_processing would produce incorrect output.

Prompt for agents 
The core issue is that AudioFrame now stores bytes data directly without copying, but some consumers (apm.py process_stream/process_reverse_stream) need to modify the data in-place via FFI. There are several possible fixes:

1. In AudioFrame.__init__, always ensure self._data is mutable: if isinstance(data, bytes), convert to bytearray(data). This preserves the zero-copy optimization for bytearray inputs while ensuring mutability.

2. Alternatively, in apm.py process_stream/process_reverse_stream, check if the frame data is writable and make a mutable copy if needed before passing to FFI, then write the result back.

3. Or document that AudioFrame data must be mutable (bytearray) for APM processing and raise an error in process_stream if the data is read-only.

Option 1 is the simplest and maintains backward compatibility. In audio_frame.py around line 67, change:
    self._data = data
to:
    self._data = data if isinstance(data, bytearray) else bytearray(data)

This still avoids the copy for bytearray inputs (the common internal path) while ensuring bytes inputs get converted to mutable storage.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


self._sample_rate = sample_rate
self._num_channels = num_channels
Expand Down Expand Up @@ -97,7 +100,7 @@ def _from_owned_info(owned_info: proto_audio.OwnedAudioFrameBuffer) -> "AudioFra

def _proto_info(self) -> proto_audio.AudioFrameBufferInfo:
audio_info = proto_audio.AudioFrameBufferInfo()
audio_info.data_ptr = get_address(memoryview(self._data))
audio_info.data_ptr = get_address(self._data)
audio_info.sample_rate = self.sample_rate
audio_info.num_channels = self.num_channels
audio_info.samples_per_channel = self.samples_per_channel
Expand Down
2 changes: 1 addition & 1 deletion livekit-rtc/livekit/rtc/video_frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def __init__(
self._width = width
self._height = height
self._type = type
self._data = bytearray(data)
self._data = data

@property
def width(self) -> int:
Expand Down
Loading