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

No convenient way of rebasing AudioData timestamp #505

Open
tguilbert-google opened this issue Jun 29, 2022 · 4 comments
Open

No convenient way of rebasing AudioData timestamp #505

tguilbert-google opened this issue Jun 29, 2022 · 4 comments
Labels
extension Interface changes that extend without breaking.

Comments

@tguilbert-google
Copy link
Member

audioData.timestamp is immutable, by design. There is however no convenient way of creating a new AudioData object with a different timestamp. Currently, users have to copy out data (plane by plane) and pass in the same sampleRate, numberofFrames, numberOfChannels parameters.

This issue tracks a proposal to add a new AudioData constructor, which mirrors VideoFrame's "copy constructor".

interface AudioData {
  constructor(AudioData audioData, AudioDataCopyInit init)
  [...]
}

AudioDataCopyInit {
  required long long timestamp;  // microseconds
}

The new constructor would copy all the data and metadata from the passed in audioData, and replace the original timestamp with the one passed in the AudioDataCopyInit.

@onthegit
Copy link

If the copy is synchronous it would be even more convenient and useful.

@padenot
Copy link
Collaborator

padenot commented Jul 4, 2022

A nice way of doing this is to allow stealing the data from a buffer, and constructing a new buffer. Since audio buffers are more or less always in regular memory, this allows a zero copy path.

This is discussed here: #287

@tguilbert-google
Copy link
Member Author

tguilbert-google commented Jul 7, 2022

Thanks for the link!

Assuming there is an audio_data.CloseAndTransfer() or similar function as a result of #287, I think we would still need a dedicated "copy" or "transfer" constructor. Otherwise, to update a timestamp, we'd still end up with something like:

let old_data = ...

let new_data_init = {
    timestamp: new_timestamp,
    numberOfChannels: old_data.channels,
    numberOfFrames: old_data.frames,
    sampleRate: old_data.sampleRate,
    format: old_data.format,
}

new_data_init.data = old_data.CloseAndTransfer();

let new_data = new AudioData(new_data_init);

Which seems clunky from an ergonomics point of view.

Is there precedent for an actual transfer constructor on the web? Such as

let new_data = new AudioData( *[* old_data *]* ,  { timestamp: new_timestamp });

If we had audio_data.CloseAndTransfer(), the copy constructor proposed in #506 could instead be a transfer constructor, which automatically calls CloseAndTransfer() on the AudioData passed in.

// This will close() old_data.
let new_data = new AudioData(old_data, { timestamp: new_timestamp });

Users could still pass in old_data.clone() instead, and force a copy of old_data.

Until #287 resolves, the constructor would make a copy internally before closing. Once #287 is resolved, it would transfer internally (assuming RefPtr::HasOneRef() etc).

@dalecurtis dalecurtis added the extension Interface changes that extend without breaking. label Mar 16, 2023
@kedicesur
Copy link

kedicesur commented Mar 15, 2024

I had to do this by utilizing the AudioData.copyTo(destination, options) tool where the destination is an ArrayBuffer with size calculated from source AudioData as AudioData.allocationSize({planeIndex:0, frameOffset:0}) * AudioData.numberOfChannels() and the options parameter is again {planeIndex:0, frameOffset:0}. However you need to take a reference to this destination ArrayBuffer in advance as .copyTo() returns just undefined an we will need it. Then you instantiate a new AudioData with these at hand like new AudioData(init). The init object parameter has a data property that is assigned with the destinationArrayBuffer, and the remaining properties of init can be copied from the original AudioData except for the timestamp which you want to assign to whatever your new timestamp value is. Terrible..!

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

No branches or pull requests

5 participants