Skip to content
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

Factor underlying client-side engine out of yew app, for use in other clients #74

Open
ronen opened this issue Jun 18, 2023 · 6 comments

Comments

@ronen
Copy link
Collaborator

ronen commented Jun 18, 2023

I'd like to embed a zoom-rs client in an existing web / electron app, written in javascript using a non-yew UI framework. That would mean decoupling the encoding/decoding code and control mechanisms from the yew UI code, factoring them out to create an "engine" that could be used in other contexts. (Something like this would be needed also for #49 (comment) which suggests using electron on MacOS.)

I'm imagining another top-level directory client-engine/ that would hold that code. It would be set it up so that it would both:

  • define a local crate that could be imported into the yew-ui app
  • compile into a wasm npm module that could be imported into javascript apps.

The engine code wouldn't require specific HTML and wouldn't create any HTML. It would just provide a control API for establishing connections etc, it would take ids of the canvas elements to render into, and it would use callbacks to communicate interesting events back to the app.

My approach would be to first factor out the client engine code but keep it within yew-ui/src/-- and once that looks good, snip it out and move it into that other directory and set up the cargo & npm mechanisms. The factoring could be done incrementally with PRs along the way, allowing reviews/suggestions as it goes and avoiding a massive shock to the code.

What do you think? I'm happy to take this on.

@darioalessandro
Copy link
Member

darioalessandro commented Jun 26, 2023

Hey @ronen !!

I do like this because I am almost sure that by mixing render code and model we are missing on:

  1. performance.
  2. cleaner architecture (MVVM?)

My only request is to do this change incrementally so that PRs are small and decrease the chances of conflicts

@ronen
Copy link
Collaborator Author

ronen commented Jun 26, 2023

Cool! I've already started working on this :)

I'll see what I can do about making small PRs -- but I want to be sure it works before taking the code down a path that may not prove fruitful. Hopefully something coming up soon.

@ronen
Copy link
Collaborator Author

ronen commented Jun 27, 2023

I'll see what I can do about making small PRs -- but I want to be sure it works before taking the code down a path that may not prove fruitful. Hopefully something coming up soon.

Well, even though I don't have the whole thing done yet, seeing #83 go by made me realize you're right, I better PR what I can while I can :) So #85 is an initial step.

@ronen
Copy link
Collaborator Author

ronen commented Jul 3, 2023

#90 was waiting in the wings for #85 to be ready :)

ronen added a commit to ronen/videocall-rs that referenced this issue Jul 10, 2023
Ultimate goal for security-union#74 is to move away from yew dependency, so originally
used custom callbacks instead of yew's.  But in working on 
websocket/webtransport connection (in another branch) it's currently
deeply tied to yew Callback.

So for now, for consistency it makes sense to
use yew Callbacks everywhere.

Later on in working on security-union#74 will try to come up with a consistent approach
to extracting the yew Callback dependency.
@ronen
Copy link
Collaborator Author

ronen commented Jul 12, 2023

Hi guys -- here's another commit waiting in the wings:

022476c "Factor out web media connection and use into model::connection::Connection"

I haven't made a PR yet since that commit is based on the commits of #96 and a PR would include all those commits too. Once #96 is merged I'll rebase against main and create a PR with just the new commit. Just showing it to you now in case you want a sneak peak / give feedback ahead of the PR.

darioalessandro pushed a commit that referenced this issue Jul 17, 2023
* Factor out PeerDecodeManager

* Add on_first_frame callback, use it to update for screen sharing

* clippy & naming cleanups

* Use yew::Callbacks instead of custom (for now)

Ultimate goal for #74 is to move away from yew dependency, so originally
used custom callbacks instead of yew's.  But in working on 
websocket/webtransport connection (in another branch) it's currently
deeply tied to yew Callback.

So for now, for consistency it makes sense to
use yew Callbacks everywhere.

Later on in working on #74 will try to come up with a consistent approach
to extracting the yew Callback dependency.

* aside: move webaudio comment from attendants.rs to peer_decoder.rs

(it got left behind when doing the initial refactor)

* Added DecodeStatus, changed MultiDecoder::decode() to return Error

Also, tidied: made a PeerDecode trait for the decode method

* cargo fmt
@ronen
Copy link
Collaborator Author

ronen commented Aug 19, 2023

H guys, FYI once #137 is merged, here's what I'm thinking my next steps are:

  1. Pull out the things in yew-ui/src/model into a top-level videocall-client directory. That directory would define a crate with a library that provides:

    • VideoCallClient
    • MicrophoneEncoder, CameraEncoder, ScreenEncoder
    • MediaDeviceAccess

    And change yew-ui to use that crate.

  2. Get that new videocall-client crate to compile into a wasm npm module that could be imported into javascript apps.

  3. Make another top-level directory, with an electron app that uses that npm module (and runs on MacOS :) It'd do roughly the same thing as the yew-UI app.

And once all that is working, I'd say it'd be a good time to re-visit the whole API.

What do you think?

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

No branches or pull requests

2 participants