-
-
Notifications
You must be signed in to change notification settings - Fork 52
feat: get stream based on bitrate and listeners #50
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: get stream based on bitrate and listeners #50
Conversation
6adb3a3 to
a20305d
Compare
|
@huyenltnguyen any progress on this solution? |
|
@ahmadabdolsaheb sorry I haven't made any progress on the error handling. Reading the Nchan doc (https://nchan.io/) requires more time (and brains) than I thought, and I haven't been able to invest enough time on this. Do you think it's okay to have this PR merged first, to enable the auto stream switching, and we will tackle the error handling in a future PR? |
|
No worries. |
|
@huyenltnguyen it seems like we have an outage with remote streams, it might be an ideal time to test error handling. |
a20305d to
80847d5
Compare
|
@ahmadabdolsaheb Sooo sorry for dragging this too long. I have found a way to switch audio source when an error occurs, finally, by using the I believe all of the relay urls are having issue at the moment, so I was able to test this by selecting a relay url, then observe the app automatically switches to another audio source. Check this out when you have a moment (I know things are crazy over the main repo), and let me know if there is any changes I should make :) |
| data-meta="stream-select" | ||
| defaultValue={this.props.url} | ||
| onChange={this.handleChange.bind(this)} | ||
| value={this.props.url} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is to make the dropdown renders the correct selected url.
|
Thanks for the update. |
|
I think the ssl certificate needs updating. |
|
The issue with the certificate and the relays has been resolved. |
|
The logic is great. |
|
Yeah to be honest, I personally don't like the solution, since I fell into infinite loop a lot when I tested this, but I decided to pushed it up anyway to it is still a progress 😅 I'll try to find a way to improve this 😄 |
|
we could have a state that keeps the streams with the errors, so each time something errors out we only select one of the streams that has not been already in use. make sure to change the state first and then change url. (I think you can pass a function that you would like to be called after state change as a parameter to setState). |
883e253 to
eedf951
Compare
|
@ahmadabdolsaheb This PR is ready for another look (I pushed the changes yesterday but forgot to notify you 😅) |
|
Thank you for your hard work. |
Closes #38