-
Notifications
You must be signed in to change notification settings - Fork 5
serializer: dict-based handler dispatch + transport hot-path log cleanup (+ tests) #4
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.
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
Changes from 2 commits
e24776f
b0fb96b
c34ac4f
137e7c9
a495530
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,8 +4,7 @@ | |
| # SPDX-License-Identifier: BSD-2-Clause | ||
| # | ||
|
|
||
| import inspect | ||
| from typing import Awaitable, Callable, Optional, cast | ||
| from typing import Awaitable, Callable, Optional | ||
| from loguru import logger | ||
| from pipecat.audio.dtmf.types import KeypadEntry | ||
| from pipecat.audio.utils import create_stream_resampler | ||
|
|
@@ -77,28 +76,59 @@ def __init__(self, sample_rate: int = 0): | |
| sample_rate | ||
| ) # What sample rate is used in Asterisk websocket channel. If 0, will be populated during setup or from MEDIA_START event | ||
|
|
||
| # Pre-built dispatch tables for `serialize`. Keyed by frame class so | ||
| # `serialize` does a single dict lookup per frame instead of | ||
| # `getattr(self, f"_frame_{type(frame).__name__.lower()}")` + an | ||
| # `inspect.isawaitable` check on the hot path. Async handlers are | ||
| # kept in a separate table so we know at lookup time whether to | ||
| # `await` the result. | ||
| self._sync_frame_handlers: dict[type, Callable[[Frame], str | bytes | None]] = { | ||
| AsteriskCommandFrame: self._frame_asteriskcommandframe, | ||
| EndFrame: self._frame_endframe, | ||
| CancelFrame: self._frame_cancelframe, | ||
| InterruptionFrame: self._frame_interruptionframe, | ||
| } | ||
| self._async_frame_handlers: dict[ | ||
| type, Callable[[Frame], Awaitable[str | bytes | None]] | ||
| ] = { | ||
| OutputAudioRawFrame: self._frame_outputaudiorawframe, | ||
| } | ||
|
|
||
| # Event handlers are keyed by Asterisk event-name string, matching | ||
| # the values produced by the protocol parser. All handlers are sync | ||
| # so one table is enough; same goal as the frame tables - skip the | ||
| # per-event `getattr` + `f"_ev_{name.lower()}"` lookup. | ||
| self._event_handlers: dict[str, Callable[[dict], Frame | None]] = { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea behind But I agree, it's inefficient. Actually, in the first iteration I used a dict as handler registry. However, I would create this dictionary in a loop iterating through all the methods of the class, in init method. This would solve the perf issue and doesn't affect the mechanism of adding |
||
| "MEDIA_START": self._ev_media_start, | ||
| "MEDIA_XOFF": self._ev_media_xoff, | ||
| "MEDIA_XON": self._ev_media_xon, | ||
| "DTMF_END": self._ev_dtmf_end, | ||
| "QUEUE_DRAINED": self._ev_queue_drained, | ||
| } | ||
|
|
||
| def _handle_event(self, message: dict) -> Frame | None: | ||
| """Call the event handler if the handler is defined in the class, otherwise return None. | ||
| """Dispatch an Asterisk event through the precomputed handler table. | ||
|
|
||
| The handler methods should be named as "_ev_{event_name.lower()}" and should take the event message as a dictionary and return a Frame or None. | ||
| The event-handler table is built once in ``__init__`` and keyed by | ||
| the exact event-name string Asterisk sends. Unknown events fall | ||
| through to the same warning path as the original reflective form. | ||
|
|
||
| Args: | ||
| message: The event message as a dictionary. | ||
| """ | ||
|
|
||
| message_type = message.get("event", None) | ||
| message_type = message.get("event") | ||
| if message_type is None: | ||
| logger.warning( | ||
| f"Received Asterisk WebSocket message without 'event' field: {message}" | ||
| "Received Asterisk WebSocket message without 'event' field: {}", | ||
| message, | ||
| ) | ||
| return None | ||
| handler = getattr(self, f"_ev_{message_type.lower()}", None) | ||
| if callable(handler): | ||
| typed_handler = cast(Callable[[dict], Frame | None], handler) | ||
| return typed_handler(message) | ||
| else: | ||
| logger.info(f"Received unhandled Asterisk WebSocket event: {message}") | ||
| return None | ||
| handler = self._event_handlers.get(message_type) | ||
| if handler is not None: | ||
| return handler(message) | ||
| logger.info("Received unhandled Asterisk WebSocket event: {}", message) | ||
| return None | ||
|
|
||
| ### Asterisk Event handlers ### | ||
|
|
||
|
|
@@ -395,24 +425,31 @@ async def setup(self, frame: StartFrame): | |
| async def serialize(self, frame: Frame) -> str | bytes | None: | ||
| """Convert a frame to its serialized representation suitable for Asterisk WebSocket channel. | ||
|
|
||
| Looks up the frame's exact type in the precomputed sync/async | ||
| dispatch tables built in ``__init__``. This avoids the per-frame | ||
| ``getattr`` + f-string + ``inspect.isawaitable`` overhead the old | ||
| reflective form paid, which matters because audio frames go through | ||
| this method dozens of times per second. | ||
|
|
||
| Args: | ||
| frame: The frame to serialize. | ||
|
|
||
| Returns: | ||
| Serialized frame data as string, bytes, or None if serialization fails. | ||
| """ | ||
| handler = getattr(self, f"_frame_{type(frame).__name__.lower()}", None) | ||
| if callable(handler): | ||
| result = handler(frame) | ||
| if inspect.isawaitable(result): | ||
| return cast(str | bytes | None, await result) | ||
| else: | ||
| return cast(str | bytes | None, result) | ||
| else: | ||
| logger.trace( | ||
| f"Received unhandled frame type in Asterisk WebSocket serializer: {type(frame)}. Frame: {frame}" | ||
| ) | ||
| return None | ||
| frame_type = type(frame) | ||
| sync_handler = self._sync_frame_handlers.get(frame_type) | ||
| if sync_handler is not None: | ||
| return sync_handler(frame) | ||
| async_handler = self._async_frame_handlers.get(frame_type) | ||
| if async_handler is not None: | ||
| return await async_handler(frame) | ||
| logger.trace( | ||
| "Received unhandled frame type in Asterisk WebSocket serializer: {}. Frame: {}", | ||
| frame_type, | ||
| frame, | ||
| ) | ||
| return None | ||
|
|
||
| async def deserialize(self, data: str | bytes) -> Frame | None: | ||
| """Convert serialized data from Asterisk's websocket channel to a frame object. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same idea as here
We can parse the handlers once in init to avoid overhead on every frame, save them into sync/async dicts. And splitting the handlers into sync and async is a good point; I didn't think about it