Add support for audio frame processor #4145
Conversation
| and self._room._token is not None | ||
| and self._room._server_url is not None | ||
| ): | ||
| self._processor._update_credentials(token=self._room._token, url=self._room._server_url) |
There was a problem hiding this comment.
Do you think the livekit-rtc package should be responsible for directly updating the credentials?
There was a problem hiding this comment.
agree, in theory that would be nice.
I wasn't sure what the best way would be for handling the possibility of the input stream changing here further down in the rtc SDK. More than the credential update this would be about the stream info update.
Any thoughts on this?
There was a problem hiding this comment.
Had a look at this from the rtc SDK side.
One challenge I see with shifting responsibility for this to the rtc SDK is that there's no control from within the rtc SDK over when/how users construct audio streams, do you have an idea how to achieve a tighter integration between stream and the room instance?
There was a problem hiding this comment.
I see, yes, it's probably fine for now.
Maybe in the future, rtc.Room should keep track of every rtc.AudioStream, I don't see an easy way to do it today tho
xianshijing-lk
left a comment
xianshijing-lk
left a comment
There was a problem hiding this comment.
lgtm in general, one small things to get your attention
|
|
||
| self._room.on("track_subscribed", self._on_track_available) | ||
| self._room.on("track_unpublished", self._on_track_unavailable) | ||
| self._room.on("token_refreshed", self._on_token_refreshed) |
There was a problem hiding this comment.
nit,
in line 127, function aclose(self) -> None:
add
self._room.off("token_refreshed", self._on_token_refreshed) ?
There was a problem hiding this comment.
btw, I think we should also cancel the ongoing task in aclose(self) function, like
for t in list(self._tasks):
await aio.cancel_and_wait(t)
self._tasks.clear()
which will syncrhonously wait and cancel the ongoing tasks
theomonnom
left a comment
theomonnom
left a comment
There was a problem hiding this comment.
lgtm! but how are users supposed to use this API?
RoomInput isn't exposed to public, only RoomIO
depends on livekit/python-sdks#533 and livekit/python-sdks#534