[iOS] Integrate Jitsi SDK with Transcription support and JitsiSDK Update #23323

iccub · 2024-04-29T14:37:53Z

Resolves brave/brave-browser#38368
Security ticket https://github.com/brave/reviews/issues/1613

Submitter Checklist:

I confirm that no security/privacy review is needed and no other type of reviews are needed, or that I have requested them
There is a ticket for my issue
Used Github auto-closing keywords in the PR description above
Wrote a good PR/commit description
Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
Checked the PR locally:
- npm run test -- brave_browser_tests, npm run test -- brave_unit_tests wiki
- npm run presubmit wiki, npm run gn_check, npm run tslint
Ran git rebase master (if needed)

Reviewer Checklist:

A security review is not needed, or a link to one is included in the PR description
New files have MPL-2.0 license header
Adequate test coverage exists to prevent regressions
Major classes, functions and non-trivial code blocks are well-commented
Changes in component dependencies are properly reflected in gn
Code follows the style guide
Test plan is specified in PR before merging

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on
All relevant documentation has been updated, for instance:

Test Plan:

The call has to be started from a desktop or android device, iOS does not offer recording options for both transcripts and video.
Join the call from desktop/android
on iOS the transcripts are gathered during the call only. It does not store transcripts prior to joining the call.
Tap on the arrow in top left to enter picture in picture
Tap on the menu to open Leo
Tap "Summarize this page"
Leo returns the transcript summary

Few notes

iOS cannot start recording transcripts or video calls, it can only join existing calls/
Unlike desktop, iOS transcripts start when joined only, it does not transcripts right from the beginning.
It has to be done in picture in picture mode, if you leave the call the transcript is gone
We have no direct control over the Brave Talk UI, it comes from our vendor in a form of native SDK.

kylehickinson · 2024-05-16T14:11:48Z

ios/brave-ios/Sources/TestHelpers/ConcurrencyHelpers.swift

@@ -35,3 +35,13 @@ public func XCTAssertAsyncNoThrow<T>(
    XCTFail(message(), file: file, line: line)
  }
 }
+
+// swift-format-ignore
+public func XCTunwrapAsync<T>(


minor nit: XCTUnwrapAsync

kylehickinson · 2024-05-16T14:12:44Z

ios/brave-ios/Tests/BraveTalkTests/BraveTalkJitsiTranscriptProcessorTests.swift

+
+import Collections
+import Foundation
+@_spi(AppLaunch) import Shared


Don't need @_spi(AppLaunch) since you're not updating the channel in this test

kylehickinson · 2024-05-16T14:13:54Z

ios/brave-ios/Sources/BraveTalk/BraveTalkJitsiTranscriptProcessor.swift

+  @MainActor
+  func processTranscript(dictionary: [AnyHashable: Any]) async {


Why does simple JSON decoding have to be done on the main actor?

kylehickinson · 2024-05-16T14:14:48Z

ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController.swift

@@ -3693,3 +3694,13 @@ extension BrowserViewController {
    present(chatController, animated: true)
  }
 }
+
+extension BraveTalkJitsiCoordinator: AIChatBraveTalkJavascript {
+  @MainActor


main actor is not needed

kylehickinson · 2024-05-16T14:16:16Z

ios/brave-ios/Sources/BraveTalk/BraveTalkJitsiTranscriptProcessor.swift

+import Foundation
+import os.log
+
+public class BraveTalkJitsiTranscriptProcessor {


Seems like you want an actor here based on number of MainActor additions

It has been an actor but @Brandon-T reverted it to a class when integrating with Leo :P I think he ran into some issues

kylehickinson · 2024-05-16T14:17:01Z

ios/brave-ios/Sources/AIChat/ModelView/AIChatViewModel.swift

@@ -14,6 +14,7 @@ public class AIChatViewModel: NSObject, ObservableObject {
  private var api: AIChat!
  private weak var webView: WKWebView?
  private let script: any AIChatJavascript.Type
+  private let braveTalkScript: AIChatBraveTalkJavascript?


(any AIChatBraveTalkJavascript)?

kylehickinson · 2024-05-16T14:17:20Z

ios/brave-ios/Sources/AIChat/ModelView/AIChatJavaScript.swift

@@ -19,3 +19,8 @@ public protocol AIChatJavascript {
  @MainActor
  static func getPrintViewPDF(webView: WKWebView) async -> Data
 }
+
+public protocol AIChatBraveTalkJavascript {
+  @MainActor


why main actor isolated?

github-actions · 2024-05-27T15:45:32Z

[puLL-Merge] - brave/brave-core@23323

Description

This PR adds the ability to use Brave Talk transcripts as context for AI chat. It introduces a new BraveTalkJitsiTranscriptProcessor class to handle processing and storing transcripts received from Jitsi, and exposes this functionality to the AIChatViewModel through a new AIChatBraveTalkJavascript protocol.

The main motivation for this change seems to be enhancing the AI chat experience by leveraging the conversation context from Brave Talk calls.

Changes

Changes

ios/brave-ios/App/Client.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved: Updates the revision of the Jitsi WebRTC dependency.
ios/brave-ios/Package.swift: Adds test target dependencies for BraveTalkTests.
ios/brave-ios/Sources/AIChat/ModelView/AIChatJavaScript.swift: Defines a new AIChatBraveTalkJavascript protocol for retrieving Brave Talk transcripts.
ios/brave-ios/Sources/AIChat/ModelView/AIChatViewModel.swift:
- Adds braveTalkScript property and initializer parameter to support AIChatBraveTalkJavascript.
- In getContext(), attempts to retrieve a Brave Talk transcript before falling back to other context sources.
ios/brave-ios/Sources/Brave/Frontend/Browser/BrowserViewController/BrowserViewController.swift:
- Passes self.braveTalkJitsiCoordinator to AIChatViewModel initializer.
- Extends BraveTalkJitsiCoordinator to conform to AIChatBraveTalkJavascript.
ios/brave-ios/Sources/Brave/Frontend/Settings/SettingsViewController.swift: Passes nil for braveTalkScript when initializing AIChatViewModel from settings.
ios/brave-ios/Sources/BraveTalk/BraveTalkJitsiCoordinator.swift:
- Adds jitsiTranscriptProcessor property to store a BraveTalkJitsiTranscriptProcessor instance.
- Updates JitsiDelegate to handle transcript chunk events.
ios/brave-ios/Sources/BraveTalk/BraveTalkJitsiTranscriptProcessor.swift: Implements the new BraveTalkJitsiTranscriptProcessor class for handling Jitsi transcripts.
ios/brave-ios/Sources/TestHelpers/ConcurrencyHelpers.swift: Adds a new XCTunwrapAsync helper function.
ios/brave-ios/Tests/BraveTalkTests/BraveTalkJitsiTranscriptProcessorTests.swift: Adds tests for BraveTalkJitsiTranscriptProcessor.
third_party/ios_deps/JitsiMeet/: Updates the bundled Jitsi Meet SDK.

Security Hotspots

The main potential security concern I see is around handling the transcript data received from Jitsi. It's important to ensure this external data is properly validated and sanitized before using it in any way, to prevent issues like injection attacks or unintended data leakage. The current implementation does deserialize the received data, but it would be good to have an extra validation step.

The tests added for BraveTalkJitsiTranscriptProcessor help provide some confidence that the transcript handling behaves as expected, but additional testing around edge cases and malformed data would further improve coverage.

github-actions bot assigned iccub Apr 29, 2024

iccub changed the title ~~wip~~ [iOS] JitsiSDK Update Apr 29, 2024

iccub added CI/skip-android Do not run CI builds for Android CI/skip-macos-x64 Do not run CI builds for macOS x64 CI/skip-windows-x64 Do not run CI builds for Windows x64 labels May 6, 2024

Brandon-T force-pushed the bugfix/jitsi_9.2.2 branch from 1eaf065 to 64dd6a7 Compare May 9, 2024 19:24

iccub force-pushed the bugfix/jitsi_9.2.2 branch from 64dd6a7 to 1ebeb5c Compare May 13, 2024 14:57

iccub marked this pull request as ready for review May 13, 2024 15:41

iccub requested a review from a team as a code owner May 13, 2024 15:41

iccub changed the title ~~[iOS] JitsiSDK Update~~ [iOS] Brave Talk x Leo transcript integration and JitsiSDK Update May 14, 2024

iccub changed the title ~~[iOS] Brave Talk x Leo transcript integration and JitsiSDK Update~~ [iOS] Integrate Jitsi SDK with Transcription support and JitsiSDK Update May 14, 2024

Brandon-T approved these changes May 16, 2024

View reviewed changes

kylehickinson reviewed May 16, 2024

View reviewed changes

github-actions bot added the puLL-Merge label May 17, 2024

iccub requested a review from kylehickinson May 24, 2024 13:43

kylehickinson approved these changes May 24, 2024

View reviewed changes

iccub added the needs-security-review label May 24, 2024

iccub and others added 6 commits May 27, 2024 17:43

wip

2c40edc

Transcript processing

60a514b

handoff

9ada9fe

clean logic

09596b7

Inject transcripts into AI-Chat

6134915

fix tests

41355d6

iccub force-pushed the bugfix/jitsi_9.2.2 branch from 02f1d91 to 41355d6 Compare May 27, 2024 15:44

iccub merged commit ddf2eb7 into master May 31, 2024
19 checks passed

iccub deleted the bugfix/jitsi_9.2.2 branch May 31, 2024 13:49

github-actions bot added this to the 1.68.x - Nightly milestone May 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS] Integrate Jitsi SDK with Transcription support and JitsiSDK Update #23323

[iOS] Integrate Jitsi SDK with Transcription support and JitsiSDK Update #23323

iccub commented Apr 29, 2024 •

edited

kylehickinson May 16, 2024 •

edited

kylehickinson May 16, 2024

kylehickinson May 16, 2024

kylehickinson May 16, 2024

kylehickinson May 16, 2024

iccub May 16, 2024

kylehickinson May 16, 2024

kylehickinson May 16, 2024

github-actions bot commented May 27, 2024

Changes

		@MainActor
		func processTranscript(dictionary: [AnyHashable: Any]) async {

[iOS] Integrate Jitsi SDK with Transcription support and JitsiSDK Update #23323

[iOS] Integrate Jitsi SDK with Transcription support and JitsiSDK Update #23323

Conversation

iccub commented Apr 29, 2024 • edited

Submitter Checklist:

Reviewer Checklist:

After-merge Checklist:

Test Plan:

kylehickinson May 16, 2024 • edited

Choose a reason for hiding this comment

kylehickinson May 16, 2024

Choose a reason for hiding this comment

kylehickinson May 16, 2024

Choose a reason for hiding this comment

kylehickinson May 16, 2024

Choose a reason for hiding this comment

kylehickinson May 16, 2024

Choose a reason for hiding this comment

iccub May 16, 2024

Choose a reason for hiding this comment

kylehickinson May 16, 2024

Choose a reason for hiding this comment

kylehickinson May 16, 2024

Choose a reason for hiding this comment

github-actions bot commented May 27, 2024

Description

Changes

Security Hotspots

iccub commented Apr 29, 2024 •

edited

kylehickinson May 16, 2024 •

edited