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 out CameraEncoder, MicrophoneEncoder, and ScreenEncoder #90

Merged
merged 3 commits into from Jul 10, 2023

Conversation

ronen
Copy link
Collaborator

@ronen ronen commented Jul 3, 2023

[Another step towards #74]

Factored out CameraEncoder, MicrophoneEncoder, and ScreenEncoder

The only structural change is that previously there was a single bit destroy: Arc<AtomicBool> that would stop all encoding when it was set, but now each Encoder has its own. Previously the host.destroy() method would set that bit, but now it calls a .stop() method for each encoder.

Also:

  • Moved the transforms from model/mod.rs into model/encode/transform.rs since they're only used in model/encode.

  • Moved the wrappers from model/mod.rs into model/wrappers.rs just so that mod.rs isn't a mixed bag of submodule declarations and code definition.

The only structural change is that previously there was a single bit
`destroy: Arc<AtomicBool>` that would stop all encoding when
it was set, but now each Encoder has its own. Previously the
`host.destroy()` method would set that bit, but now it calls a `.stop()`
method for each encoder.

Also: 
 * Moved the transforms from model/mod.rs into model/encode/transform.rs
since they're only used in model/encode.

 * Moved the wrappers from model/mod.rs into model/wrappers.rs just so
that mod.rs isn't a mixed bag of submodule declarations and code definition.
@griffobeid
Copy link
Member

Thanks for the contribution @ronen

I tested it out locally and it looks good to me! @darioalessandro can you take a look?

@darioalessandro
Copy link
Member

Wow !!! this is great!! I love how you moved all the model out of the host!!

looking now

@darioalessandro
Copy link
Member

I am sorry for taking this long! will review again by eod tomorrow July 9

@ronen
Copy link
Collaborator Author

ronen commented Jul 10, 2023

I am sorry for taking this long! will review again by eod tomorrow July 9

July 9, but luckily you didn't specify a year :)

Just joking around, no rush.

@darioalessandro
Copy link
Member

I am sorry for taking this long! will review again by eod tomorrow July 9

July 9, but luckily you didn't specify a year :)

Just joking around, no rush.

XD XD sorry mate, I am on it right now

@darioalessandro darioalessandro merged commit 48d6691 into security-union:main Jul 10, 2023
2 checks passed
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.

None yet

3 participants