Skip to content
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.

Sign up for GitHub

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

Constructor frame #223

Merged
merged 13 commits into from
Apr 19, 2024
Merged

Constructor frame #223

Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
03df57b
Add constructor for encoded frames
palak8669 Jan 16, 2024
4f2c438
Update
palak8669 Jan 16, 2024
303d24f
deep clone of data
palak8669 Jan 17, 2024
dccb6f6
update wording of metadata
palak8669 Jan 18, 2024
9739ea1
Changes to address the comments
palak8669 Apr 2, 2024
3c35ee7
rm internal slot
palak8669 Apr 2, 2024
417f896
Use CLoneArrayBuffer
palak8669 Apr 3, 2024
ae86de4
rename the Options
palak8669 Apr 4, 2024
8ae6738
address comments
palak8669 Apr 19, 2024
ae0c342
remove conditions on [[value]]
palak8669 Apr 19, 2024
f229c33
add introduction
palak8669 Apr 19, 2024
18c1b3e
nits
palak8669 Apr 19, 2024
429171d
add deep copy in the steps
palak8669 Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,34 @@ dictionary RTCEncodedVideoFrameMetadata {
// re-use or extend the equivalent defined in WebCodecs.
[Exposed=(Window,DedicatedWorker), Serializable]
interface RTCEncodedVideoFrame {
constructor(RTCEncodedVideoFrame originalFrame, optional RTCEncodedVideoFrameMetadata newMetadata);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer the second argument to be an options object, e.g.

new RTCEncodedVideoFrame(frame, {metadata: frame.getMetadata()});

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

readonly attribute RTCEncodedVideoFrameType type;
attribute ArrayBuffer data;
RTCEncodedVideoFrameMetadata getMetadata();
};
</pre>

### Constructor ### {#RTCEncodedVideoFrame-members}
<dl dfn-for="RTCEncodedVideoFrame" class="dictionary-members">
<dt>
<dfn for="RTCEncodedVideoFrame" method>constructor()</dfn>
</dt>
<dd>
<p>
Creates a new {{RTCEncodedVideoFrame}} from the given |originalFrame| and sets the internal metadata to |newMetadata|.

When called, run the following steps:

1. Set this.`[[type]]` to |originalFrame|.`[[type]]`.
1. Let this.`[[data]]` be a new ArrayBuffer object whose `[[ArrayBufferData]]` internal slot value is |originalFrame|.`[[data]]`.`[[ArrayBufferData]]`, and whose `[[ArrayBufferByteLength]]` internal slot value is |originalFrame|.`[[data]]`.`[[ArrayBufferByteLength]]`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to clone the data and I am not sure it does that there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is meant to do deep clone the data. Does that work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed it now to use CloneArrayBuffer definition so that it's more clear.

1. Let `[[metadata]]` represent the metadata associated with this newly constructed frame.
1. For each {`[[key]]`,`[[value]]`} pair of |originalFrame|.`[[getMetadata()]]`, set `[[metadata]]`.`[[key]]` to `[[value]]`.
1. For each {`[[key]]`,`[[value]]`} pair of |newMetadata|, if the `[[value]]` exists, set `[[metadata]]`.`[[key]]` to `[[value]]`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to make sure that changing value does not change what metadata would be exposed in the newly constructed object. This seems fine as is now.
It might be good to add a [[metadata]] internal slot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I add the [[metadata]] internal slot in a follow-up change because that would need using this internal slot in the whole encoded-transforms spec, which would be unrelated to this change?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine, but let's add a comment that we want to do a deep copy here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


</p>
</dd>
</dl>

### Members ### {#RTCEncodedVideoFrame-members}
<dl dfn-for="RTCEncodedVideoFrame" class="dictionary-members">
<dt>
Expand Down Expand Up @@ -626,11 +648,32 @@ dictionary RTCEncodedAudioFrameMetadata {
<pre class="idl">
[Exposed=(Window,DedicatedWorker), Serializable]
interface RTCEncodedAudioFrame {
constructor(RTCEncodedAudioFrame originalFrame, optional RTCEncodedAudioFrameMetadata newMetadata);
attribute ArrayBuffer data;
RTCEncodedAudioFrameMetadata getMetadata();
};
</pre>

### Constructor ### {#RTCEncodedAudioFrame-members}
<dl dfn-for="RTCEncodedAudioFrame" class="dictionary-members">
<dt>
<dfn for="RTCEncodedAudioFrame" method>constructor()</dfn>
</dt>
<dd>
<p>
Creates a new {{RTCEncodedAudioFrame}} from the given |originalFrame| and sets the internal metadata to |newMetadata|.

When called, run the following steps:

1. Let this.`[[data]]` be a new ArrayBuffer object whose `[[ArrayBufferData]]` internal slot value is |originalFrame|.`[[data]]`.`[[ArrayBufferData]]`, and whose `[[ArrayBufferByteLength]]` internal slot value is |originalFrame|.`[[data]]`.`[[ArrayBufferByteLength]]`.
1. Let `[[metadata]]` represent the metadata associated with this newly constructed frame.
1. For each {`[[key]]`,`[[value]]`} pair of |originalFrame|.`[[getMetadata()]]`, set `[[metadata]]`.`[[key]]` to `[[value]]`.
1. For each {`[[key]]`,`[[value]]`} pair of |newMetadata|, if the `[[value]]` exists, set `[[metadata]]`.`[[key]]` to `[[value]]`.

</p>
</dd>
</dl>

### Members ### {#RTCEncodedAudioFrame-members}
<dl dfn-for="RTCEncodedAudioFrame" class="dictionary-members">
<dt>
Expand Down