refactor: Use webrtc interface. #777
Conversation
| @@ -48,7 +49,7 @@ class _RTCVideoViewState extends State<RTCVideoView> { | |||
|
|
|||
| @override | |||
| void dispose() { | |||
| widget._renderer.delegate.removeListener(_onRendererListener); | |||
| videoRenderer.removeListener(_onRendererListener); | |||
| super.dispose(); | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
@cloudwebrtc: I know you didn't change the following (in the void didUpdateWidget(RTCVideoView oldWidget))
Timer(
Duration(milliseconds: 10), () => videoRenderer.mirror = widget.mirror);
But I have a hard time figure it out why we would poll each 10 milliseconds if we need to mirror the value. Why we are not able to listen to a change of the value!
There was a problem hiding this comment.
I also think this is not necessary and should be removed.
| import 'interface/enums.dart'; | ||
| import 'interface/media_recorder.dart' as _interface; | ||
| import 'interface/media_stream.dart'; | ||
| import 'interface/media_stream_track.dart'; | ||
|
|
||
| class MediaRecorder extends _interface.MediaRecorder { |
There was a problem hiding this comment.
Maybe we can rename the class as MediaRecorderDelegate to make it obvious what it is. So you will not need to use the as _interface in this case :-)
There was a problem hiding this comment.
I want to remove MediaRecorder from the interface because this interface is completely dependent on the platform implementation and is not a standard in the WebRTC API
| import '../interface/mediadevices.dart'; | ||
| import '../interface/navigator.dart'; | ||
| import 'package:webrtc_interface/webrtc_interface.dart'; | ||
|
|
||
| import 'mediadevices_impl.dart'; | ||
|
|
||
| class NavigatorNative extends Navigator { |
There was a problem hiding this comment.
Maybe we can let go this class since it does the same thing as MediaDevices. Or at least mark it as deprecated
There was a problem hiding this comment.
I think it is currently marked as deprecated, try not to break the API, remain unchanged in version 1.0, and deprecated in 2.x
| @@ -34,6 +36,9 @@ class RTCVideoRendererNative extends VideoRenderer { | |||
| @override | |||
| MediaStream? get srcObject => _srcObject; | |||
|
|
|||
| @override | |||
| Function? onResize; | |||
There was a problem hiding this comment.
I think we can be more clear about the signature of onResize at the interface level.
void Function()? onResize;
There was a problem hiding this comment.
|
I add comments on the PR. But you merge it before sawing it :-)
Can you look at it.
…On Thu., Nov. 25, 2021, 07:00 CloudWebRTC, ***@***.***> wrote:
Merged #777 <#777>
into master.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#777 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJL6EZ4GXLJHUUU27GH5WYDUNYQODANCNFSM5IQ64IGQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
I‘m so sorry，I needed an urgent transitional version, so I merged it in advance. A separate version of dart web and flutter, I merged it without breaking any API. |
If we split flutter_webrtc into three packages, we will be able to support flutter native, flutter web, and dart web at the same time.
and we can use different packages to maintain multiple platforms in the future. #692 (comment)
close #692
TODO: