Depends on D33760

Depends on D33761

For any onlookers, I'm holding this until the spec issue at [1] is resolved.

[1] https://github.com/w3c/mediacapture-record/issues/169

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:pehrsons, could you have a look please?
For more information, please visit auto_nag documentation.

Note that this will also bring MediaRecorder up to spec in general for the Constructor and the start method. Mime type handling is not up to spec yet; that's bug 1512175; but the rest should be.

Depends on D33762

This brings the MediaRecorder constructor and start() to spec on most parts
except mime type handling (bug 1512175). The flow is also improved to better
follow along in the algorithms of the spec.

Depends on D41584

This aligns it better with MediaRecorder's timeslice which was changed from
int32 to uint32 earlier.

Depends on D41585

Depends on D41586

Depends on D41587

Depends on D41588

Depends on D41589

Looking for clarification given the spec changes. It looks like the UA has a lot of liberty to decide on the appropriate bits per second depending on the scenario.

• The UA could conceptually pick 0 bits/s for audio|video even if they are present in a stream and no options for rates were passed? This seems silly, so I figure we can ignore this, but I want to clarify that my reading is correct.
• If a stream with only audio or video is passed and only bitsPerSecond are passed as an option, it looks like the UA has quite a lot of wiggle room. For example, UA gets a stream with only 1 audio track, bitsPerSecond = 1000 and can then decide to set audioBitsPerSecond to ~1000 and videoBitsPerSecond to 0, or divide them in some other way. Is this correct?

(In reply to Bryce Seager van Dyk (:bryce) from comment #14)

Looking for clarification given the spec changes. It looks like the UA has a lot of liberty to decide on the appropriate bits per second depending on the scenario.

• The UA could conceptually pick 0 bits/s for audio|video even if they are present in a stream and no options for rates were passed? This seems silly, so I figure we can ignore this, but I want to clarify that my reading is correct.
• If a stream with only audio or video is passed and only bitsPerSecond are passed as an option, it looks like the UA has quite a lot of wiggle room. For example, UA gets a stream with only 1 audio track, bitsPerSecond = 1000 and can then decide to set audioBitsPerSecond to ~1000 and videoBitsPerSecond to 0, or divide them in some other way. Is this correct?

It picks at two different occasions with slightly different instructions:

• At construction and when inactivated: pick a "reasonable" bitrate for audio and video.
• At start(): pick a "reasonable" bitrate for audio and video given the current track set.

Yes, "reasonable" gives room to assign anything the UA likes. But IMO it should at least be possible to argue that the assigned value is indeed reasonable. Bitrate 0 for a certain type is not, if you're actually going to record that type, as that wouldn't result in an actual recording.

Also note that both cases contain a "such that the sum of videoBitsPerSecond and audioBitsPerSecond is close to the value of recorder’s [[ConstrainedBitsPerSecond]] slot" ([[ConstrainedBitsPerSecond]] is the bitsPerSecond that was passed in to the ctor). This makes it hard to make a case for choosing 0 for both audio and video for instance.

More detailed comments inlined below:

• The UA could conceptually pick 0 bits/s for audio|video even if they are present in a stream and no options for rates were passed? This seems silly, so I figure we can ignore this, but I want to clarify that my reading is correct.

Yes, silly, but nothing explicitly forbidding it.

• If a stream with only audio or video is passed and only bitsPerSecond are passed as an option, it looks like the UA has quite a lot of wiggle room. For example, UA gets a stream with only 1 audio track, bitsPerSecond = 1000 and can then decide to set audioBitsPerSecond to ~1000 and videoBitsPerSecond to 0, or divide them in some other way. Is this correct?

I assume you're talking about start() (as the only place the algorithm cares about tracks). The intention with the spec's "for recording all tracks in tracks" is that since tracks are now known, we can distribute the bits over all the tracks. In cases such as 1: (0xa, 1xv), 2: (3xa, 3xv), or 3: (19xa, 1xv) the results would be wildly different. In (1) we intended to allocate 0 bits for audio since there won't be any audio recorded, so reducing the video bits would be against the intention of application. This is again not called out explicitly to keep the spec from getting too complex. Also in (3) you'd want to allocate a larger share of bits for audio than in (2), since you'll be recording such a larger share of audio tracks.

I'll also comment on the new bitrate WPT -- since there are no strict requirements outlined by the algorithm I tried to write something with as reasonable numbers as possible, so that 1) old implementations fail it, and 2) gecko under the new spec passes it. When vendors implement the spec it's easy enough to relax the test should they need to to satisfy the way they've implemented the algorithm under the new spec.

Let me know if some parts are still unclear.

Thanks for the clarifications.

(In reply to Andreas Pehrson [:pehrsons] from comment #15)

It picks at two different occasions with slightly different instructions:

• At construction and when inactivated: pick a "reasonable" bitrate for audio and video.
• At start(): pick a "reasonable" bitrate for audio and video given the current track set.

I think this is where part of my confusion stems from. The distinction here was throwing me -- it wasn't clear that the recorder shouldn't inspect the stream's tracks prior to starting. I'd read into the "reasonable" part as that that the recorder could perform inspection of the stream to make a judgement, and then that it should explicitly do so again when starting in case the tracks had changed. Rather than that it shouldn't do so before starting (because the tracks might change), and then only do so when starting.

I assume you're talking about start() (as the only place the algorithm cares about tracks). The intention with the spec's "for recording all tracks in tracks" is that since tracks are now known, we can distribute the bits over all the tracks. In cases such as 1: (0xa, 1xv), 2: (3xa, 3xv), or 3: (19xa, 1xv) the results would be wildly different. In (1) we intended to allocate 0 bits for audio since there won't be any audio recorded, so reducing the video bits would be against the intention of application. This is again not called out explicitly to keep the spec from getting too complex. Also in (3) you'd want to allocate a larger share of bits for audio than in (2), since you'll be recording such a larger share of audio tracks.

I think I've misunderstood some of the tests for case 1 above, as I read them as allocating > 0 bits for audio when there is none. I'll take another look shortly now that I have a clearer understanding, as I suspect I was dealing with cases where the recorder had not yet started.

This is ready to land, but I'm planning to hold it until 71 so that bug 1512175 has a chance to land in the same version.

https://hg.mozilla.org/mozilla-central/rev/1d75b63d3a00
https://hg.mozilla.org/mozilla-central/rev/09d7666761b5
https://hg.mozilla.org/mozilla-central/rev/c12cc9f6c937
https://hg.mozilla.org/mozilla-central/rev/f4219cbc29e7
https://hg.mozilla.org/mozilla-central/rev/67e59d2a26dc
https://hg.mozilla.org/mozilla-central/rev/6005f7761315
https://hg.mozilla.org/mozilla-central/rev/c60d1d4fe9f4
https://hg.mozilla.org/mozilla-central/rev/afa7200292eb
https://hg.mozilla.org/mozilla-central/rev/6ce6f0e0471b
https://hg.mozilla.org/mozilla-central/rev/044cfbab987a

Documentation updates:

• Minor touching up to the onwarning page.
• Updated https://wiki.developer.mozilla.org/en-US/docs/Web/API/MediaRecorder/start to include some missing errors and other information.
• Submitted BCD PR 5133 which includes the needed changes.

That should do the job. LMK if more is needed or if you spot any errors!

FWIW we've unshipped onwarning. Not sure if the compat table supports removals or how this should be handled.

:pehrsons - do you have any idea when we stopped sending warning to MediaRecorder? That needs to be updated as well, and I've been going through code and so far haven't found where this happened. That should be the version number set as the "version_removed" for the warning event on MediaRecorder.

For now, I've set it to "71" but it should be updated to the real number when possible.

I did some archaeology and I can't find that we ever fired it. It looks like we just implemented the webidl for the handler and that was it.