dolbyio-samples / rts-app-react-publisher-viewer

A Real Time Streaming App example for dolby.io
MIT License
15 stars 9 forks source link

Issue 360 handle empty source name in dolby.io broadcast app #361

Closed jackshen closed 1 year ago

jackshen commented 1 year ago

360

Current broadcast app in dolby.io dashboard publishes a single stream with no name/label. Because stream labels are used for identification in the Viewer app, there was an edge case where unnamed streams were causing the Viewer to crash.

Jack Shen 2 hours ago I've made changes in Viewer to handle missing source IDs, so that dolby.io's Millicast Publisher should work. The problem is if we have more than one stream without a source ID, it's entirely impossible to tell which event belongs to which stream. In other words, we can support viewing ONLY one stream with a missing label/source ID

Jack Shen 1 hour ago oh it'll also break if there's one unnamed stream and one stream named "null"

Jack Shen 1 hour ago In the publisher app, we explicitly disallow unnamed streams. I can commit my changes, but it's very specific to this one issue in dolby.io dashboard, and would otherwise be a flawed implementation

Jack Shen 1 hour ago [@Vincent Song]() lmk your thoughts on this issue

Wenfeng Song 1 hour ago we only take one source without name, when another empty source name comes, we ignore it. Or could we give it a name when it’s empty name?

Jack Shen 1 hour ago Right now, in Publisher app, sources must have a name. When every source has a name, there's no problem in Viewer app

Jack Shen 1 hour ago The problem is dolby.io dashboard app uses a source without a name

Jack Shen 1 hour ago My current changes are able to address this edge case, but the dolby.io dashboard app should not be publishing streams that are missing names

Wenfeng Song 1 hour ago Sure, I got it. The product owner told me they only allow one empty name source. So let’s ignore the coming empty name sources if that happen.

Jack Shen 1 hour ago Unfortunately, we can not ignore the coming empty name sources because won't know which one it belongs to (the first one, or the second one)

Jack Shen 1 hour ago The good thing is it looks like the dolby.io dashboard app can't publish more than one stream at a time

Jack Shen 1 hour ago But the bottom line is that the fix is fundamentally flawed

Wenfeng Song 1 hour ago I mean if we received the first empty id in active event, then we will not accept any source with an empty id until we received inactive event with an empty source id.

Jack Shen 1 hour ago The problem extends further than that, because we'll still receive other events such as layers and we won't know which it belongs to (edited)

Jack Shen 1 hour ago Again, this isn't an immediate problem as it's not possible to have streams with no name in Publisher app or more than one stream with no name in dolby.io broadcast app (edited)

Wenfeng Song 1 hour ago yes, let’s assume we only handle one stream with no name.

Wenfeng Song 1 hour ago [@shivank.dubey]() do you think we need mention this in README.md?

Jack Shen 1 hour ago Ideally, we should roll back the changes I've made once the fundamental problem has been solved in dolby.io broadcaster app

Wenfeng Song 1 hour ago It should be fine, we only handle one empty name source, it should work even after the broadcast app solved this issue, isn’t it?

Jack Shen 43 minutes ago Yes, but the implementation necessarily has other issues:

  • Can't have an empty name source and a source named "null"
  • Edge case with more than one empty name source (we don't anticipate this scenario, but it's still an unclosed edge case)
  • TypeScript isn't happy (edited)