search

miroslavpejic85 / mirotalkc2c

✨WebRTC - C2C - Real-time cam-2-cam video calls & screen sharing, end-to-end encrypted, to embed in any website with a simple iframe.
https://c2c.mirotalk.com
GNU Affero General Public License v3.0
232 stars 57 forks source link

Display all local and remote video #8

Closed ZZYSonny closed 1 year ago

ZZYSonny commented 1 year ago

Reason for PR: When I am testing c2c with my laptop and my phone, both my Phone and my PC show a pixelated face from me. And the face takes up the whole screen. However, in PiP mode (all video displayed and original aspect ratio), the video isn't too bad. (btw I am also working on a PR on codec and bitrate to improve video quality)

In this PR: The wrapper of myLocalVideo will automatically fit the aspect ratio of local video. RemoteVideo will fit the screen in the original aspect ratio. (grey background will be shown if they don't match perfectly.) "my-video-wrap-width" and "my-video-wrap-height" are tested on my laptop and phone. These two numbers seems to work good enough for me. But I am open to any changes.

miroslavpejic85 commented 1 year ago

Hey @ZZYSonny!

I want to express my gratitude for your pull request (PR). It's greatly appreciated!

You've brought up an excellent point regarding the aspect ratio feature, which is present in other MiroTalk versions but seems to be missing here. What about to make it optional (commit). This way, users can choose whether they want to activate this feature or not, which adds a nice touch of customization to our product.

Thank you once again for your contribution, and I look forward to seeing this enhancement in action. Keep up the great work! 😊🚀

c2c-aspect-ratio

ZZYSonny commented 1 year ago

Hi @miroslavpejic85 Thank you for making this PR even better. Allowing users to toggle aspect ratio is great.

I notice you are explicitly setting the width and height on a mobile device. And if aspect ratio is toggled on, grey background will appear in that banner. What about removing line 660-663 and also making the local video banner fit to the aspect ratio of local video (just like on pc)?

Before: 1695557137320 After: 1695557001354

miroslavpejic85 commented 1 year ago

Maybe you are testing it on an Android mobile, but on an iOS mobile, I need to keep it to make it work properly. Try with last commit if show correctly? Thank you!

ZZYSonny commented 1 year ago

With the latest commit, the local banner is showing correctly. Thanks for fixing!