-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(*): add support for new architecture #67
Conversation
Status: Waiting for testWhat has been done?
Not tested but it should fixed What is missing?
TODO
How can I help?
How to test?
Please write you have been testing 🙏 |
a474326
to
c9390cf
Compare
ee24a7c
to
e38608b
Compare
e38608b
to
06f6fc9
Compare
0f2cd62
to
340f3b5
Compare
5455f91
to
3b6e344
Compare
Your issue happens here: https://github.com/ThibaultBee/StreamPack/blob/994b995705e1566ccb2a18d848ad907e4afaebdb/core/src/main/java/io/github/thibaultbee/streampack/internal/sources/AudioSource.kt#L63 I am currently releasing the Android dependency to integrate the fixes on the preview in this branch. Are you going to perform other tests? @Veryinheart @matthewfleming How long do you need to test this PR? |
Yes, i havent started actually streaming yet.
The very first one was with the old arch i believe. Is there a way to check which arch it is running on from the running code? |
This package is supposed to support both arch on iOS and Android.
Not sure... There might be a log somewhere. |
Hi @ThibaultBee , we have tested on some devices and below is the result: IOS: Android: Question: Do we support live stream in background mode? live stream will be stopped if we move app to background mode when it is live streaming. |
Yes, this is on purpose. I thought using cameras in background was not legit for Android but I am not sure anymore. But we will have to use a backgroud service and it is not something we want to do. |
I've tested streaming on Iphone 8 and Samsung A22, couldn't find any problems. |
When i looked into this, the easiest way was to use PiP. But yes. Older people (and even some younger) will not know why their stream stopped just because they tapped away from the application. What i have done is to display an in-app notification telling the user their stream has stopped, even if they stop streaming themselves.. |
That's going to be a tricky one. If you remove |
I dont believe it is present. With default expo the only app config available is https://docs.expo.dev/versions/latest/config/app/#orientation Just to add on the orientation. Listening to orientation change with an event listener and keep a state with the current orientation. This has worked great so far. But if the outgoing stream is not oriented according to the preview the user would not know until looking at the viewer perspective. |
Your orientation setting is not the same as orientation in api.video-reactnative-live-stream/example/android/app/src/main/AndroidManifest.xml Line 17 in aa6138b
I need to dig a bit deeper in this issue. |
I don't think this is an odd ask, but why not release this with the bug and continue working on the fix? I don't think this bug has anything to do with this feature? |
Mainly because this regression is due to the new architecture, so it is related to this branch. And... the fix is going to be postponed because I am focusing on api.video project with a higher priority. |
bb95ba9
to
efd904d
Compare
@BlueBazze |
@ThibaultBee |
Sorry, I kept forgetting about it. |
@nikitapilgrim |
Done testing on android. The previous issue has not occurred. |
Notice that,
Log from the example app
This is because of my streaming streaming server not being able to handle a resolution change during the reconnection. The end user could accidentally start the stream in portrait, and then after seeing the preview change it to landscape. The easiest solution would be to implement a timer when this scenario occurs. I know this cant be fixed from the package. But down the line, it would be nice to see a reason argument for onDisconnect, that would describe how the stream was disconnected. Was the disconnect userInitiated, connectionClosed etc.. |
I use react-native-vision-camera and this module in my project. When switching from react-native-vision-camera to this module, this bug occurs. |
hmm looks like the camera has not been released when this package tries to access it. If this is the case, we can't really do something on our side. |
@BlueBazze @nikitapilgrim I don't want to hold the release for these but we can work on it in a second time. |
Update to Fabric architecture and clean the project