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

Allow UA to clamp maxBufferSize to a value between 1 and the value provided by the web page #105

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

youennf
Copy link
Contributor

@youennf youennf commented Jan 29, 2024

Fixes #95


Preview | Diff

@@ -204,7 +212,7 @@ with |processor| as parameter.

The <dfn>handleNewFrame</dfn> algorithm is given a |processor| as input.
It is defined by running the following steps:
1. If |processor|.`[[maxBufferSize]]` has a value and |processor|.`[[queue]]` has |processor|.`[[maxBufferSize]]` elements, [=queue/dequeue=] an item from |processor|.`[[queue]]`.
1. If |processor|.`[[queue]]` has |processor|.`[[maxBufferSize]]` elements, [=queue/dequeue=] an item from |processor|.`[[queue]]`.
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably need to close the frame that you dequeued

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I'll fix it in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we close it here, we should also use "dequeue and close" on line 219.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll file an issue about this.
For camera tracks, what we enqueue here is not a WebCodecs VideoFrame object but the necessary information to create one.
For VideoTrackGenerator tracks given to MediaStreamTrackProcessor, we could indeed directly enqueue VideoFrames but we would not expose this VideoFrame object but a clone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I filed #106

@youennf
Copy link
Contributor Author

youennf commented Feb 2, 2024

Merging this PR given I filed #106 to handle the comment.

@youennf youennf merged commit 607f1d9 into w3c:main Feb 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UAs should be allowed to clamp maxBufferSize
4 participants