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.

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

AudioFrame/VideoFrame should be marked as serializable/transferable #210

Closed
tguilbert-google opened this issue Apr 30, 2021 · 7 comments · Fixed by #286
Closed

AudioFrame/VideoFrame should be marked as serializable/transferable #210

tguilbert-google opened this issue Apr 30, 2021 · 7 comments · Fixed by #286
Labels
extension Interface changes that extend without breaking.

Comments

@tguilbert-google
Copy link
Member

This might be a duplicate issue, or already in a PR, but I'm opening this to make sure it isn't missed.

The Chrome implementation already supports this, but it is not called out in the spec.

@chcunningham
Copy link
Collaborator

Thanks for filing this. Agree, spec should change (it's straitforward once the ref-counting PRs land).

@chcunningham chcunningham added the extension Interface changes that extend without breaking. label May 12, 2021
@chcunningham
Copy link
Collaborator

Triage note: marking extension, as these are add-on attributes/behaviors

@chcunningham
Copy link
Collaborator

Having said that, they are already implemented in Chrome, so we should firm up consensus on their semantics ASAP.

Fortunately, I think these are not controversial.

For those reading along, some background:

The chrome impl behaves in the following way:

  • serializing is effectively a clone(). Serializing a frame creates a record of its "media resource" without detaching that frame's "resource reference". The deserialize operation will create a new VideoFrame/AudioData that points to this same "media resource".
  • transferring the close()s the source frame, producing a new frame on the other end that refers to the same media resource while the original frame is now detached.

@padenot @aboba WDYT?

@padenot
Copy link
Collaborator

padenot commented May 17, 2021

Yes, I think that's uncontroversial, but what API should be used? An old-school transfer list, or the new way of doing things? Also, this needs to be designed at the same time as #212, so that the ergonomics can be the best possible for memory management.

@dalecurtis
Copy link
Contributor

EncodedAudioChunk and EncodedVideoChunk will also need to be serializable/transferable. Especially if we end up with worker only exposure.

@dalecurtis
Copy link
Contributor

(The associated metadata pieces too)

@chcunningham
Copy link
Collaborator

EncodedAudioChunk and EncodedVideoChunk will also need to be serializable/transferable. Especially if we end up with worker only exposure.

Tracked in #289

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Interface changes that extend without breaking.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants