Skip to content

Conversation

@pblazej
Copy link
Contributor

@pblazej pblazej commented Dec 9, 2025

Python reference livekit/python-sdks#533

This is a compromise between existing Audio/Video interfaces, mostly to get uniform "add" behavior from the Room/Session.

The ideal solution in pure Swift would be kinda:

public protocol FrameProcessor: Sendable {
    associatedtype Input
    associatedtype Output

    func process(frame: Input) -> Output
}

This encapsulates the idea of Audio/Video-agnostic interface.

However, the entry points in audio stack must remain ObjC compatible anyway (no generics/associated types), and you cannot get "one protocol conforming to the other" for free (without wrapping/adapters).

Alternative solution

We can just remove explicit FrameProcessor protocol and move RoomDelegate conformance to both Audio and Video processor protocols or typealiases. Everything is optional there anyway, so this is a non-braking change.

Then we're left with these 2 "dangling" methods that don't have any explicit representation anywhere, but also we cannot call them directly from the RoomDelegate, so the benefits are negligible:

    def _update_stream_info(
        self,
        *,
        room_name: str,
        participant_identity: str,
        publication_sid: str,
    ): ...

    def _update_credentials(self, *, token: str, url: str): ...

@github-actions
Copy link

github-actions bot commented Dec 9, 2025

⚠️ This PR does not contain any files in the .changes directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The minimal implementation is:

    private static let noiseFilter = AICAudioFilter()
    private let session = Session(
        tokenSource: ...,
        options: SessionOptions(audioProcessor: Self.noiseFilter)
    )

@objc
public protocol AudioCustomProcessingDelegate: Sendable {
@objc
var isEnabled: Bool { get set }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This IS a breaking change for Krisp, we can go for @objc optional but then this requirement does not make much sense to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

the alternative would be to not define it here and keep isEnabled on the processor implementations, right?

Maybe that's preferable if the alternative is a breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can also use @objc optional annotation, but then you get Bool? you need to default to ?? true for legacy things like Krisp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The motivation here is obviously to avoid casting in the audio/video thread.

Copy link
Contributor

@lukasIO lukasIO Dec 9, 2025

Choose a reason for hiding this comment

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

but implementations could still implement an isEnabled check in the process function and just return early?

Copy link
Contributor Author

@pblazej pblazej Dec 10, 2025

Choose a reason for hiding this comment

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

or should WE do it internally (not passing frames) (see below)


/// Called when the processor is no longer needed.
@objc optional
func close()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a little blind python copy, not sure how can we really utilize that outside deinit (destruction).

Copy link
Contributor

Choose a reason for hiding this comment

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

currently the python code will allow to close the aic processor and implicitly recreate it on a next .process call.
The reason why it works this way right now is that on the agents side we need the ability to switch tracks for a processor.


/// Credentials information.
@objc optional
func updateCredentials(token: String, url: String)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot magically call it, so this is basically just a hint that "you need to get the credentials from the RoomDelegate somehow".

Copy link
Contributor

Choose a reason for hiding this comment

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

should we remove it then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See Alternative solution above 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pblazej
Copy link
Contributor Author

pblazej commented Dec 10, 2025

Alternative solution 76c1ebe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants