Bug 1975032 Comment 7 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Hamish Willee from comment #6)
> 1. Is set of supported metadata the same for the getMetadata and the new constructors?

Yes. Tests for that are coming in bug 1993997.

> 2. For Video, do we support all the properties in the [RTCEncodedVideoFrame.webidl](https://searchfox.org/firefox-main/source/dom/webidl/RTCEncodedVideoFrame.webidl#18-29)?

All except `timestamp` which is outdated and unpopulated. WPT tests were unfortunately lacking, so I plan to improve them in bug 1993997, but for now you can use https://jsfiddle.net/jib1/5t1mehn4/ to verify support. In Firefox it yields the following for video:
```js
main {
  "contributingSources": [],
  "dependencies": [],
  "frameId": 1,
  "height": 240,
  "payloadType": 120,
  "spatialIndex": 0,
  "synchronizationSource": 4251100087,
  "temporalIndex": 0,
  "width": 320
}
```
>    - My assumption is that they are all supported in the same version.

`frame.getMetadata()` was added in bug 1631263, copy constructor in this bug.

>    - The structure is different than spec IDL and it seems we omit `rtpTimestamp`, `receiveTime`, `captureTime`, `senderCaptureTimeOffset`, `mimeType`

Yes we have at least bug 1944338 to track this.

> 3. Similarly do we support all the metadata properties in [RTCEncodedAudioFrame.webidl](https://searchfox.org/firefox-main/source/dom/webidl/RTCEncodedAudioFrame.webidl#10-15)

We [don't appear to populate](https://searchfox.org/firefox-main/rev/d8e526c7b71d9e29b68bb61ccfcd3318e75e4aa1/dom/media/webrtc/jsapi/RTCEncodedAudioFrame.cpp#73) `sequenceNumber` which is surprising. Byron, is this expected?  The fiddle shows:
```js
main {
  "contributingSources": [],
  "payloadType": 109,
  "synchronizationSource": 721873839
}
```

>   - The IDL doesn't quite match the spec, but we pass the [`audioLevel`](https://wpt.live/webrtc-encoded-transform/tentative/RTCEncodedAudioFrame-audiolevel.html) WPT test, so wanted to check. Assumption is we still don't support it.

Good catch. That's a [false positive](https://searchfox.org/firefox-main/rev/d8e526c7b71d9e29b68bb61ccfcd3318e75e4aa1/testing/web-platform/tests/webrtc-encoded-transform/tentative/RTCEncodedAudioFrame-audiolevel.html#26-29) since `undefined < 0 || undefined > 1` is always `false`:
```js
      if (metadata.audioLevel < 0 || metadata.audioLevel > 1) {
        self.postMessage("Invalid audioLevel value");
        return;
      }
```
I'll fix it.
(In reply to Hamish Willee from comment #6)
> 1. Is set of supported metadata the same for the getMetadata and the new constructors?

Yes. Tests for that are coming in bug 1993997.

> 2. For Video, do we support all the properties in the [RTCEncodedVideoFrame.webidl](https://searchfox.org/firefox-main/source/dom/webidl/RTCEncodedVideoFrame.webidl#18-29)?

All except `timestamp` which is outdated and unpopulated. WPT tests were unfortunately lacking, so I plan to improve them in bug 1993997, but for now you can use https://jsfiddle.net/jib1/5t1mehn4/ to verify support. In Firefox it yields the following for video:
```js
main {
  "contributingSources": [],
  "dependencies": [],
  "frameId": 1,
  "height": 240,
  "payloadType": 120,
  "spatialIndex": 0,
  "synchronizationSource": 4251100087,
  "temporalIndex": 0,
  "width": 320
}
```
>    - My assumption is that they are all supported in the same version.

`frame.getMetadata()` was added in bug 1631263, copy constructor in this bug.

>    - The structure is different than spec IDL and it seems we omit `rtpTimestamp`, `receiveTime`, `captureTime`, `senderCaptureTimeOffset`, `mimeType`

Yes we have at least bug 1944338 to track this.

> 3. Similarly do we support all the metadata properties in [RTCEncodedAudioFrame.webidl](https://searchfox.org/firefox-main/source/dom/webidl/RTCEncodedAudioFrame.webidl#10-15)

We [don't appear to populate](https://searchfox.org/firefox-main/rev/d8e526c7b71d9e29b68bb61ccfcd3318e75e4aa1/dom/media/webrtc/jsapi/RTCEncodedAudioFrame.cpp#73) `sequenceNumber` which is surprising. Byron, is this expected?  The fiddle shows:
```js
main {
  "contributingSources": [],
  "payloadType": 109,
  "synchronizationSource": 721873839
}
```

>   - The IDL doesn't quite match the spec, but we pass the [`audioLevel`](https://wpt.live/webrtc-encoded-transform/tentative/RTCEncodedAudioFrame-audiolevel.html) WPT test, so wanted to check. Assumption is we still don't support it.

Good catch. That's a [false positive](https://searchfox.org/firefox-main/rev/d8e526c7b71d9e29b68bb61ccfcd3318e75e4aa1/testing/web-platform/tests/webrtc-encoded-transform/tentative/RTCEncodedAudioFrame-audiolevel.html#26-29) since `undefined < 0 || undefined > 1` is always `false`:
```js
      if (metadata.audioLevel < 0 || metadata.audioLevel > 1) {
        self.postMessage("Invalid audioLevel value");
        return;
      }
```
I'll fix it in bug 1993997.

Back to Bug 1975032 Comment 7