Open Bug 1826432 Opened 2 years ago Updated 10 months ago

Expose TextEncoder/TextDecoder interfaces everywhere

Categories

(Core :: DOM: Core & HTML, task)

task

Tracking

()

People

(Reporter: mgaudet, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

Assignee: nobody → mgaudet
Status: NEW → ASSIGNED
Attachment #9328452 - Attachment description: Bug 1826432 - Expose TextDecoder eveywhere r?smaug → Bug 1826432 - Expose TextDecoder and TextEncoder eveywhere r?smaug

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:mgaudet, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Flags: needinfo?(smaug)
Flags: needinfo?(mgaudet)
Flags: needinfo?(smaug)
Flags: needinfo?(mgaudet)

I've stumbled on https://github.com/WebAudio/web-audio-api/issues/2499, which discusses if indeed TextDe/Encoder should be exposed in audio worklets or not and how this should be reflected in the spec, because right now the spec says that it should be exposed.

I also noticed that Chrome doesn't actually support TextDe/Encoder in audio worklets anymore.

This should be ready to land.
As per discussion in the spec it is still unclear if this is going to change or not:

We do not have a comprehensive list of blocked APIs yet, but these are good questions to ask:

  1. Does it do any significant memory allocation within its operation?
  2. Does it have any unbound thread blocking operation?

If the answer to them is yes, then it is likely to be excluded from the AudioWorkletGlobalScope.

So 2. is not the case, but 1. is unclear, because it depends on the user how big the data is they are trying to convert.
I don't know how Firefox usually handles cases like this, but currently the spec, at least to me, seems to clearly allow TextDe/Encoder.
Does Firefox follow the spec until it changes, or does it preempt for planned changes?

Flags: needinfo?(krosylight)

So I missed that you don't have the land permission, I thought someone would push that button for me, sorry!

We already have TextDecoder/EncoderStream and [De]CompressionStreams that also can grow a lot if one put a big chunk of input, so I'd say it shouldn't be the problem. Scripts already can freely do naughty things like new ArrayBuffer(10000000) and blocking the whole API for that doesn't make much sense.

NI'ing :padenot in case there's a different opinion from audio worklet side.

Flags: needinfo?(krosylight) → needinfo?(padenot)

The answer to 1. and 2. are yes, because Streams. Hongchan says "significant", but I'd drop the word, if it allocates, it's bad. Thankfully there's an API to not allocate, which would be fine to run. Exposing ArrayBuffer and friends is necessary for the AudioWorkletProcessor to function, so that's that.

In the grand scheme of things, if you're using this API in an AudioWorkletProcessor, you're doing something extremely incorrect and folks are going to open bugs saying "such and such browser are broken".

Exposed=* shouldn't include AudioWorklet.

Flags: needinfo?(padenot)

(In reply to Paul Adenot (:padenot) from comment #7)

Thankfully there's an API to not allocate, which would be fine to run.

Which one is that?

(In reply to Paul Adenot (:padenot) from comment #9)

e.g. https://encoding.spec.whatwg.org/#dom-textencoder-encodeinto

Fun.
This would still require exposing TextDe/Encoder, but hide de/encode().
I'm happy to adapt the patch to do exactly that.

It seems quite strange to me though, I understand where this is coming from, but this still requires allocation, its just that the allocation now has to explicitly happen from the user instead of implicitly by the TextDe/Encoder API.
(I guess the allocation could also happen on a different thread and then transferred through a port)

(I guess the allocation could also happen on a different thread and then transferred through a port)

Yes, this is generally something like that that is done in real-time bits of programs. It's also fine to perform allocation when the AudioContext is suspended, e.g. during a setup phase. This can also use views on the WASM heap, etc.

http://www.rossbencina.com/code/real-time-audio-programming-101-time-waits-for-nothing is the classic article that explains the type of constraints we're dealing with here.

(In reply to Paul Adenot (:padenot) from comment #11)

(I guess the allocation could also happen on a different thread and then transferred through a port)

Yes, this is generally something like that that is done in real-time bits of programs. It's also fine to perform allocation when the AudioContext is suspended, e.g. during a setup phase. This can also use views on the WASM heap, etc.

http://www.rossbencina.com/code/real-time-audio-programming-101-time-waits-for-nothing is the classic article that explains the type of constraints we're dealing with here.

I didn't realize the goal is to avoid any allocation at all, it does make sense where all this is coming from then.

(In reply to daxpedda from comment #10)

This would still require exposing TextDe/Encoder, but hide de/encode().

I will proceed with that approach then.

I don't think we should proceed without spec discussion in https://github.com/whatwg/encoding/ as it can be a source of webcompat issue. Can you open an issue there?

Flags: needinfo?(daxpedda)

I requested some official clarification in https://github.com/WebAudio/web-audio-api/issues/2499#issuecomment-2002470074 in Web Audio first.
Will update when we have a response.

Flags: needinfo?(daxpedda)

Unassigning myself here, as I don't plan further work in this area at the moment.

Assignee: mgaudet → nobody
Status: ASSIGNED → NEW
Attachment #9328452 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: