Add Data Channel support #10
Conversation
There was a problem hiding this comment.
Thank your for your contribution. As far as I have skimmed your code, your code correctly uses the binding infrastructure this library provides and probably this should properly function as you intend.
However, it should be amended to follow the convention of this library strictly to be merged to this repository because codes which lives here are expected to be reusable even in situations we do not have any idea.
| RTC_EXPORT RtcString* webrtcDataChannelLabel( | ||
| const WebrtcDataChannelInterface* channel); | ||
|
|
||
| RTC_EXPORT int webrtcDataChannelStatus( |
There was a problem hiding this comment.
| RTC_EXPORT int webrtcDataChannelStatus( | |
| RTC_EXPORT int webrtcDataChannelState( |
Follow the naming of the underlying function. It is crucial to make it easy to understand what feature it represents, especially because we do not provide additional documentations for the binding and users have to rely on the counterparts of the C++ APIs.
If you think such renaming is strongly preferred, it should be submitted to the upstream and reviewed by its maintainers.
| const char* text | ||
| ) { | ||
| auto chan = rtc::ToCplusplus(channel); | ||
| const auto db = webrtc::DataBuffer(text); |
There was a problem hiding this comment.
Do not short-circuit webrtc::DataBuffer and webrtc::DataChannelInterface::Send. The strict one-to-one rule of these bindings allow to reuse them for different purposes. For example, the bindings would not serve for your purpose if they were designed to have convenient interfaces for the example Unity project we made. We need you to follow the rule even if it is so troublesome.
| const struct WebrtcDataChannelObserverFunctions* functions) { | ||
|
|
||
| auto chan = rtc::ToCplusplus(channel); | ||
| auto obs = new webrtc::DelegatingDataChannelObserver(context, functions); |
There was a problem hiding this comment.
Same here. It should have a dedicated function, something like webrtcCreateDataChannelObserver.
| @@ -15,12 +15,43 @@ | |||
|
|
|||
| extern "C" { | |||
There was a problem hiding this comment.
| extern "C" { | |
| extern "C" { | |
| #else | |
| #include <stdbool.h> | |
| #include <stdint.h> |
| @@ -91,16 +91,25 @@ extern "C" void webrtcDeleteSessionDescriptionInterface( | |||
| delete rtc::ToCplusplus(desc); | |||
| } | |||
|
|
|||
| extern "C" bool webrtcIceCandidateInterfaceToString( | |||
There was a problem hiding this comment.
Leave webrtcIceCandidateInterfaceToString as is.
| @@ -9,7 +9,7 @@ | |||
| <Project> | |||
| <PropertyGroup> | |||
| <BaseOutputPath>bin/</BaseOutputPath> | |||
| <Configuration>Debug</Configuration> | |||
There was a problem hiding this comment.
Do not do that. Instead, you should pass /p:Configuration=Release when you invoke MSBuild.
| @@ -19,7 +19,7 @@ | |||
| <FullOutputPath>$([System.IO.Path]::GetFullPath('$(OutputPath)'))</FullOutputPath> | |||
| <IsDebug Condition="'$(Configuration)' == Debug">true</IsDebug> | |||
| <IsDebug Condition="'$(Configuration)' != Debug">false</IsDebug> | |||
| <Targets>Android;Ios;LinuxX64;MacX64;WinX64</Targets> | |||
There was a problem hiding this comment.
This also should not happen.
sdk/dotnet/unity/unity.msbuildproj
Outdated
| Condition="!Exists('$(IntermediateOutputPath)win_x64/build.ninja')" /> | ||
| <Exec Command="ninja -C $(EscapedIntermediateOutputPath)win_x64 :jingle_peerconnection_so" /> | ||
| <Copy DestinationFolder="$(OutputPath)Runtime" SourceFiles="$(IntermediateOutputPath)win_x64/jingle_peerconnection_so.dll;$(MSBuildThisFileDirectory)Runtime/jingle_peerconnection_so.dll.meta" /> | ||
| </Target> | ||
| <Target Condition="$(ParsibleTargets.Contains(';WinX86;'))" Name="BuildWinX86" DependsOnTargets="BuildExternal"> |
There was a problem hiding this comment.
Do not add this. Unity does not support x86.
| @@ -1,47 +1,116 @@ | |||
| /* | |||
There was a problem hiding this comment.
I wonder what happened to this file. Maybe you changed line endings by accident?
sdk/dotnet/webrtc/webrtc.csproj
Outdated
| <ItemGroup> | ||
| <Compile Remove="api\rtc_databuffer.cs" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
This is unnecessary because it uses Microsoft.NET.Sdk.
This reverts commit b534d6f.
Unfortunate typo and week tests made it so if only a middle spatial layer is active, vp9 encoder would be configured to send two top layers. This is a cherry-pick of: https://webrtc.googlesource.com/src/+/a945cdadffa50028f96449823ce6f900cd6e8b95 Original reviewed on: https://webrtc-review.googlesource.com/c/src/+/185043 Bug: webrtc:11319, chromium:1131839 Change-Id: Ia850099d8023d9ec63e27d536a546cf529bf31a4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185184 Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/branch-heads/4240@{#10} Cr-Branched-From: 93a9d19-refs/heads/master@{#31969}
The DxgiOutputDuplicator uses a vector<byte> to hold the rects that have changed on the screen. It then iterates over the vector to grab each rect and apply it to the updated_region. There is vector resizing logic which checks the 'capacity' of the vector and determines whether there is enough space for the changed rect data. Often the 'capacity' and 'size' of the vector are equal but that isn't always true. When the capacity is greater than size, and the number of changed rects is high enough, rect data will be written past the element pointed to by (data() + size()) and this is the error that ASAN is warning of. The fix is to use size() instead of capacity() when determining whether to resize the vector and as the buffer size we provide to the Windows API. There are no other usages of this vector so there aren't any problems caused by the size/capacity discrepancy in the existing builds. The ASAN issue is worth fixing in case someone comes along and decides to use the vector differently (e.g rely on the size instead of capacity so some of the rects are not counted). (cherry picked from commit ea3e321) Bug: chromium:1138446 Change-Id: I3041091423de889e0f8aabc56ece9466a3000b4f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188900 Reviewed-by: Jamie Walch <jamiewalch@chromium.org> Commit-Queue: Joe Downing <joedow@google.com> Cr-Original-Commit-Position: refs/heads/master@{#32425} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190165 Commit-Queue: Jamie Walch <jamiewalch@chromium.org> Cr-Commit-Position: refs/branch-heads/4280@{#10} Cr-Branched-From: 75b9ab6-refs/heads/master@{#32272}
update object recognition
This adds data channel support to the library. Only tested so far with data channels created on the client.