-
Notifications
You must be signed in to change notification settings - Fork 27
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
Refactor spec to introduce media thread #107
Conversation
@@ -91,37 +91,44 @@ argument, ensure that the codec is disabled and produces no output. | |||
|
|||
### Stream creation ### {#stream-creation} | |||
|
|||
Each {{RTCRtpSender}} or {{RTCRtpReceiver}} has its own <dfn>media thread</dfn> on which media flows, | |||
from a source on which to read encoded data to a sink on which to write encoded data. | |||
The source is the packetizer for {{RTCRtpSender}} and the decoder for {{RTCRtpReceiver}}. |
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 seems backwards. For Sender, media comes from the encoder and goes to the packetizer; for the Receiver, media comes from the depacketizer and goes to the decoder.
Either I am misunderstanding line 95 or I'm misunderstanding lines 96+98.
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.
Right, it needs to be reversed.
@@ -91,37 +91,44 @@ argument, ensure that the codec is disabled and produces no output. | |||
|
|||
### Stream creation ### {#stream-creation} | |||
|
|||
Each {{RTCRtpSender}} or {{RTCRtpReceiver}} has its own <dfn>media thread</dfn> on which media flows, |
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.
I think this is not right; in the non-transform case, the media thread on which media flows is not the thread that is visible to JS.
I'll suggest alternate wording (after the meeting).
Alternative to this PR:
|
1. [=Queue a task=] to run the following step: | ||
1. [=Queue a task=] on [=this=]'s media thread to run the following steps: | ||
1. If [=this=].`[[pipeToController]]` is not null, abort these steps. | ||
1. Set [=this=].`[[pipeToController]]` to a new {{AbortController}}. |
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 creates a WebIDL interface on a background thread, which we shouldn't do.
Cleaning up old PRs and this has an |
The goal of this PR is to clarify the threading model, without changing much of the functionality.
For SFrameTransform, the transform happens as if in the media thread, though implementations may decide to do it however they want.
For RTCRtpScriptTransform, the streams are transferred from the media thread directly to the worker thread.
Preview | Diff