EncodedChunks confusingly provide access to constructed ".data" -- despite the decoder making an internal copy.

[dalecurtis]

As folks are starting to play with the API, at least one person has hit a footgun where they modified the encoded chunk after sending it to the decoder. We should just copy before control returns to JS to avoid this.

Obviously this prevents a zero copy on encoded data path, but generally copying the input data isn't expensive.

[koush]

I am that footgun user.
In my head, EncodedVideoChunk seemed like a simple dictionary object (which is why I modified post creation), but I guess more is happening under the hood there.

This was before I had thought the data had been passed to the decoder (via the decode) call: simply constructing the EncodedVideoChunk object with values has side effects.

[sandersdan]

There is potential confusion either way. One solution is to be more verbose; instead of an EncodedChunk constructor we could have a static EncodedChunk::CopyFrom(). We can also remove the .data member and provide a copyInto() method (like Plane has), and that would remove a copy in the Chrome implementation.

It does mean that JS codec implementations would have to copy their chunks at least once, but that is likely worth the cost.

[tidoust]

Regarding memory copies, I note that ongoing discussions in the workshop on Machine Learning on the Web suggest that, in the case of ML inference applied to media frames, memory copies throughout the processing pipeline (fetch, decoding, processing, rendering) may take as much time as the ML inference itself. See the Memory copies issue for details.

I'm proposing a dedicated virtual breakout session during TPAC breakout week (26-30 October) on Memory copies & zero-copy operations on the Web, so that we can better understand when copies are needed, expensive/inexpensive, etc.

[dalecurtis]

Thanks. The one API that would help us avoid this is an immutable ArrayBuffer.

Note: We're only discussing copies of encoded data here, not the raw frames. So in real-time cases I wouldn't expect these copies to go over ~15MBit/s. The copies may slow down faster-than-realtime encoding, but I would still expect them to be smaller than the raw frames used by ML.

[koush]

This was part of the reason I managed to run into this bug: I assumed the underlying pipeline would be zero copy (as that's something I strive towards for performance reasons). Ie, the ArrayBuffer I pass in belongs to the decoder until the call/promise returns.

Similar to Android's MediaCodec's management of input/output buffers that are queued/dequeued.

[chcunningham]

See related issue #104.

One solution is to be more verbose; instead of an EncodedChunk constructor we could have a static EncodedChunk::CopyFrom()

Right now we have 2 copies.

1. at creation of EncodedChunk
2. sometime after decode() is called (when the decode is actually processed)

#104 is about eliminating copy number 2. But what if we eliminate both. Rather than EncodedChunk::CopyFrom(), we could define EncodedChunk::TransferTo() (naming needs work), which would transfer the underlying bytes from input buffer, leaving neutering the callers reference.

[sandersdan]

General note: all of our APIs that take ArrayBuffers should probably have a transfer flag or transferList.

Either way we do it, one of the two operations will be eager:

• Encoder implementation outputs EncodedChunk: ideally we would not yet create an ArrayBuffer for the data.
• Demuxer creates EncodedChunk: we should transfer an ArrayBuffer to avoid copying. (Note that there may be cases where copying is actually better, EncodedChunks are often small.)

If we can avoid exposing any implementation details of EncodedChunk then it could be generic over different backings. (That would be ArrayBuffer and DecoderBuffer in Chrome.)

[padenot]

General note: all of our APIs that take ArrayBuffers should probably have a transfer flag or transferList.

#104, but also #212.

[chcunningham]

Triage note: 'breaking', as we are removing an attribute (replaced by a method).

[erights]

From #80 (comment) above:

Thanks. The one API that would help us avoid this is an immutable ArrayBuffer.

Please see the Immutable ArrayBuffer proposal, which is currently at Stage 2 of the tc39 process, has enthusiastic support from some implementations, and not (yet?) any serious objections. Would this still help with the concerns expressed above?

[erights]

#80 (comment)

From #80 (comment) above:

Thanks. The one API that would help us avoid this is an immutable ArrayBuffer.

Please see the Immutable ArrayBuffer proposal, which is currently at Stage 2 of the tc39 process, has enthusiastic support from some implementations, and not (yet?) any serious objections. Would this still help with the concerns expressed above?

Ping. I am about to ask for advancement to stage 2.7 at tc39, so any timely answer or opinions would be most useful. Thanks!

[dalecurtis]

I don't think we have anything new to add over the initial comment. Having immutable access would indeed be helpful with avoiding copies.