Consider changing MediaRecorderOpionts.*bitsPerSecond to signed long or double

[yellowdoge]

These parameters [1] are unsigned long; it might be better to have them signed integer, since they are potentially engaged in arithmetic operations.

[1] http://w3c.github.io/mediacapture-record/MediaRecorder.html#dictionary-mediarecorderoptions-members
[2] https://www.w3.org/TR/WebIDL/#EnforceRange
[3] https://www.w3.org/TR/WebIDL/#Clamp

[foolip]

Or just double? Note that WebIDL will throw for Infinity and NaN, so you don't need to worry about those, what you get into your implementation code is a finite number.

[yellowdoge]

I like double. @mreavy WDYT?

[mreavy]

I don't understand the reasoning of making bitsPerSecond a double. bitsPerSecond is inherently a non-negative number; so even if it is used in arithmetic operations, bitsPerSecond should never be negative. What's more, making this a signed-long or double would remove the possibility of preventing/blocking negative numbers from being passed in. So, for these reasons, keeping it an unsigned long makes more sense to me.

[foolip]

Negative values would be clamped to the lowest possible bitrate, just like 0, which IMHO is better than the current situation, where -1 becomes 4294967295 in type conversion, and is then clamped to the highest possible bitrate. This could also be fixed with [Clamp] I think.

But really, the reason is that unsigned integers are discouraged by Google's C++ style guide, so for every bit of IDL that uses unsigned long (which means uint32_t) it's likely the case that Blink internally uses int32_t, so the range between 2^31 and 2^32-1 becomes a problem.

[mreavy]

I don't think that C++ style guides or the details of C++ integer math should define the correct interface to use from Javascript, so I want to stick with unsigned long, and if makes sense annotate it with clamp, etc.

[yellowdoge]

No consensus and anyway we have been working with the current situation for a while with no major incidents, so I guess we can close this issue.