-
Notifications
You must be signed in to change notification settings - Fork 118
Fully configure frame processors when they are used directly on an audio stream #679
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
base: main
Are you sure you want to change the base?
Changes from all commits
27c841a
15fe1b4
56e4b95
4fdef63
5a55617
83dada3
2b96668
46f11dd
4fc15ea
8bc8953
dedf686
1699ad4
277b634
6bcdd5b
a8d3879
4c7a95e
6a2c5f2
d93f881
2c9d8de
ea1cde0
8e3a461
ea1de09
cb47aec
b9a9c15
92e334e
e8b12a4
4fe5018
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
1egoman marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -744,6 +744,8 @@ def _on_room_event(self, event: proto_room.RoomEvent) -> None: | |
| sid = event.local_track_published.track_sid | ||
| lpublication = self.local_participant.track_publications[sid] | ||
| ltrack = lpublication.track | ||
| if ltrack is not None: | ||
| ltrack._set_room(self) | ||
| self.emit("local_track_published", lpublication, ltrack) | ||
| elif which == "local_track_unpublished": | ||
| # During teardown the publication may already have been removed | ||
|
|
@@ -757,6 +759,14 @@ def _on_room_event(self, event: proto_room.RoomEvent) -> None: | |
| unpublished = self.local_participant._track_publications.get(sid) | ||
| if unpublished is not None: | ||
| del self.local_participant._track_publications[sid] | ||
| if unpublished.track is not None: | ||
| unpublished.track._set_room(None) | ||
|
1egoman marked this conversation as resolved.
|
||
| # Mirror track_unsubscribed: drop the publication's track | ||
| # reference. This also makes unpublish_track's own | ||
| # _set_room(None) a no-op when it loses the race (its | ||
| # `publication._track is not None` guard short-circuits), | ||
| # avoiding a redundant clear. | ||
| unpublished._track = None | ||
| self.emit("local_track_unpublished", unpublished) | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines
+762
to
770
Contributor
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. 🚩 Before this PR, the Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
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. This is sort of the same thing that was surfaced here just flipped around? I don't really have the depth of knowledge on this code to know for sure what to do here. Here's what a LLM had to say about this whole situation: I'm tempted to leave it alone since this very minor "breaking change" I think is actually now in practice more correct. But I'm open to alternate perspectives here.
Contributor
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. wouldn't the alternative be to simply null it after emitting the event? for sync event handlers that should keep the previous behaviour? |
||
| else: | ||
| logging.debug("local_track_unpublished for untracked publication sid %s", sid) | ||
|
|
@@ -774,6 +784,15 @@ def _on_room_event(self, event: proto_room.RoomEvent) -> None: | |
| del self.local_participant._track_publications[previous_sid] | ||
| republished._info = event.local_track_republished.info | ||
| self.local_participant._track_publications[republished.sid] = republished | ||
| if republished.track is not None: | ||
| # Keep the local-track invariant (track.sid == publication.sid, | ||
| # set at publish_track) intact across republish, then re-push | ||
| # metadata so any attached FrameProcessor learns the new | ||
| # publication SID / credentials. _set_room with the same room | ||
| # is a no-op for the token_refreshed listener but re-fans the | ||
| # metadata to every registered AudioStream. | ||
| republished.track._info.sid = republished.sid | ||
| republished.track._set_room(self) | ||
|
Comment on lines
+787
to
+795
Contributor
Author
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. Note to any reviewers: the Devin bot surfaced this case where when a full reconnect happens, tracks would not have their metadata kept up to date. I'm not sure if
Contributor
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. didn't dive too deep into this, just a thought: if it's about the metadata being outdated, shouldn't we override all of the track's |
||
| self.emit("local_track_republished", republished, previous_sid) | ||
| elif which == "local_track_subscribed": | ||
| sid = event.local_track_subscribed.track_sid | ||
|
|
@@ -799,17 +818,21 @@ def _on_room_event(self, event: proto_room.RoomEvent) -> None: | |
| rpublication._subscribed = True | ||
| if track_info.kind == TrackKind.KIND_VIDEO: | ||
| remote_video_track = RemoteVideoTrack(owned_track_info) | ||
| remote_video_track._set_room(self) | ||
| rpublication._track = remote_video_track | ||
| self.emit("track_subscribed", remote_video_track, rpublication, rparticipant) | ||
| elif track_info.kind == TrackKind.KIND_AUDIO: | ||
| remote_audio_track = RemoteAudioTrack(owned_track_info) | ||
| remote_audio_track._set_room(self) | ||
| rpublication._track = remote_audio_track | ||
| self.emit("track_subscribed", remote_audio_track, rpublication, rparticipant) | ||
| elif which == "track_unsubscribed": | ||
| identity = event.track_unsubscribed.participant_identity | ||
| rparticipant = self._remote_participants[identity] | ||
| rpublication = rparticipant.track_publications[event.track_unsubscribed.track_sid] | ||
| rtrack = rpublication.track | ||
| if rtrack is not None: | ||
| rtrack._set_room(None) | ||
| rpublication._track = None | ||
| rpublication._subscribed = False | ||
| self.emit("track_unsubscribed", rtrack, rpublication, rparticipant) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.