Port windows implementation from C# to C++ #731
Conversation
| @@ -16,7 +16,9 @@ | |||
| "author": "Zhen Wang <[email protected]> (http://blog.zmxv.com)", | |||
| "license": "MIT", | |||
| "peerDependencies": { | |||
| "react-native": ">=0.8.0" | |||
| "react": ">=16.8.6", | |||
| "react-native": ">=0.60.0-rc.2", | |||
There was a problem hiding this comment.
It might work for previous versions.
There's also the question of other platforms as well, which might support lower versions.
This causes warnings at most.
|
@RomualdPercereau Could you take a look at this PR when you have a moment? |
windows/RNSound/RNSound/RNSound.cpp
Outdated
|
|
||
| winrt::Windows::Foundation::IAsyncAction RNSound::loadFile(MediaPlayer player, | ||
| const std::string &fileName) { | ||
| StorageFile file{nullptr}; |
There was a problem hiding this comment.
this change seems to break loading a file from url. All thats needed is the following before looking for storage folders/storage files:
if (fileName.rfind("http", 0) == 0)
{
winrt::Windows::Foundation::Uri uri{ winrt::to_hstring(fileName) };
auto mediaSource = MediaSource::CreateFromUri(uri);
player.Source(mediaSource);
}
There was a problem hiding this comment.
I have some changes that would support doing this, but the janeasystems repo doesn't seem to support pull requests from additional contributors.
There was a problem hiding this comment.
Ah thanks for catching that! Let me check and get back to you to see what the best option we have for adding changes to that repo is.
There was a problem hiding this comment.
Hi @chrisfromwork , what error or message are you getting when trying to create a pull request? I think it should be possible to open a PR.
There was a problem hiding this comment.
Changes from @chrisfromwork are now in the PR.
There was a problem hiding this comment.
@jaimecbernardo I do not see the JameaSystems repo as a repo I can target with a cross fork pr. I am not sure why
There was a problem hiding this comment.
thanks @chiaramooney and @jaimecbernardo for picking up this uri support change
|
@RomualdPercereau Could you take a look at this PR when you have a moment? |
This PR removes the Windows C# implementation and adds a Windows C++ implementation, for improving performance and supporting more recent versions of react-native-windows.