-
Notifications
You must be signed in to change notification settings - Fork 156
Define AudioDataCopyToOptions with frameOffset and frameCount #228
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
Conversation
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.
It will be easier to decide how this needs to work when the output format is specced precisely.
+1 to adding the sample format to the copyTo options or also having a convertTo(format, options). These are all easy to convert, but as an implementation quality issue a NotSupportedError could be thrown if an implementation doesn't support it for now. |
index.src.html
Outdated
|options|.{{AudioDataCopyToOptions/planeIndex}}. | ||
|
||
NOTE: For interleaved formats with a single plane, |frameCount| will | ||
be the total number of frames accross all channels. |
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.
It's the sample count then, if it's across all channels. A frame count is specifically channel-count independent.
This steps should be about finding a number of elements to copy, based on the various parameters passed as parameter. Then the caller simply copies.
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.
Thanks, hopefully fixed! Algo now returns number of elements, including multiplication by channel count for interleaved formats.
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.
Thanks Paul
index.src.html
Outdated
|options|.{{AudioDataCopyToOptions/planeIndex}}. | ||
|
||
NOTE: For interleaved formats with a single plane, |frameCount| will | ||
be the total number of frames accross all channels. |
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.
Thanks, hopefully fixed! Algo now returns number of elements, including multiplication by channel count for interleaved formats.
Looks good, thanks! Can you rebase it so I can rebase #256 and merge it? |
Editors call: @aboba says LGTM |
Thanks! Rebased and proceeding to merge. |
SHA: 0db2aae Reason: push, by @chcunningham Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: 0db2aae Reason: push, by @chcunningham Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: 0db2aae Reason: push, by @chcunningham Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: 0db2aae Reason: push, by @tguilbert-google Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: 0db2aae Reason: push, by @tguilbert-google Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
SHA: 0db2aae Reason: push, by @tguilbert-google Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Following up on this TODO
https://github.com/w3c/webcodecs/pull/162/files#r625845109
Fixes #179
Fixes #223
Preview | Diff