Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve bitrate control in MediaRecorder #20

Closed
mreavy opened this issue Sep 17, 2015 · 11 comments
Closed

improve bitrate control in MediaRecorder #20

mreavy opened this issue Sep 17, 2015 · 11 comments

Comments

@mreavy
Copy link

mreavy commented Sep 17, 2015

We should replace bitRate with a bitsPerSecond parameter to MediaRecorder and provide the ability to specify separate audio and video bitrates. Suggested interface in MediaRecorderOptions would be:
unsigned long audioBitsPerSecond;
unsigned long videoBitsPerSecond;
unsigned long bitsPerSecond;

@tdaede
Copy link

tdaede commented Sep 17, 2015

I don't think the "total" bitsPerSecond parameter should be included, it's not clear how the API can make an intelligent decision when only provided that parameter. It's also important to specify the units, the current spec doesn't.

Assuming the application is stored media, these are going to go into the unconstrained VBR parameters of the encoders, so it would also be good to mention that these are hints and that the encoder may deviate from the requested bitrate.

@mreavy
Copy link
Author

mreavy commented Sep 17, 2015

I believe "total" is going to be a common use case for people using this API - they don't know what an intelligent split would be, and even if they do they'd need to check the sampling rate and channels for audio, and number of video tracks - and adapt in mid-encoding if those change. I believe the majority of users of this API will basically say "I want an N bps stream/file, you figure out how to do that".

Right now in Firefox we calculate the default audio bitrate when we get incoming data, based on sampling rate and channels, for example. At that point we would subtract that from the total requested, and Video will get the rest (and should be divided by the number of video tracks being encoded). If no audio comes in, video would (continue) to get everything. If audio changes mid-encoding, the bits for video would adjust. Etc.

@tdaede
Copy link

tdaede commented Sep 17, 2015

Maybe then have the bitrates be in per video stream / per audio channel pair?

I think most API users more of want a certain baseline of quality - something a bitrate API doesn't really give, especially with video encoders, but we can get pretty close. We're encoding for disk here, hitting an exact size target isn't important. Specifying the rates per stream / channel pair is closer to that.

Plus the bitrate for video and audio heavily depends on what codecs the user chose, so they already have to have knowledge of what they are encoding. I think the total bitrate parameter is just confusing - it would be better to have sane defaults for both, and then the user can override either one separately.

@mreavy
Copy link
Author

mreavy commented Sep 17, 2015

What you're really championing here is a quality parameter, and I can see use cases that would like to have that (and not have which codec, how efficient it is, etc - the math -- most applications that want this would just want a quality level, not to embed the logic to figure that out (and perhaps have that get broken by underlying changes)). That makes sense - and is a separate issue/pull-request.

Encoded size can be important for mobile storage, and also for streaming the blobs to a server for analysis or storage. Certainly we get requests for the ability to set the bitrate, and I haven't gotten requests to set the quality so far.

@tdaede
Copy link

tdaede commented Sep 17, 2015

Yeah, I'd really like a quality parameter, however I didn't suggest it as I don't see a good encoder-agnostic way to support it at this time. At least with bitrate, you have some idea of what you're getting.

I imagine a common split to use would be 96kbps for stereo audio, and whatever remains for video. Do you want to codify how the split is done, or leave it up to the implementation? Also, what happens if all three parameters are specified?

@yellowdoge
Copy link
Member

My two pennies is that bitrate and bits per second are terms used by different folks (i.e. codec/network and signal processing people, respectively) to refer to the same abstract quality concept. In any case I agree with mreavy@ that JS apps should not be forced to think about such nitty gritty details, and perhaps could be offered a low/mid/high bitrate instead?

Oh and this would be easier to accommodate if the mimeType, where bitrate/bps is supposed to be specified, would be a Dictionary as per #19.

@tdaede
Copy link

tdaede commented Sep 18, 2015

Bitrate is allowed to change on the fly, so it's distinct from the constructor parameter.

How would you define what the three bitrates are? Limiting it to three options seems totally useless. I also disagree that JS app developers can't understand such concepts, if they managed to integrate WebRTC into their application.

@yellowdoge
Copy link
Member

Yeah, I didn't mean to limit it to 3 bitrates but thought of it as an extra convenience to a less-than-pro user. The idea would be to offer some simple presets while letting a pro-user define whatever bitrate they think convenient according to, possibly, their scene composition etc. The point about ctor versus on the fly is very much open.

@yellowdoge
Copy link
Member

I propose closing this particular discussion (constructor parameters) and reopening to discuss the getter API.

@yellowdoge
Copy link
Member

sorry I closed by mistake :)
@mreavy can you comment on #43 and close this issue if https://rawgit.com/w3c/mediacapture-record/master/MediaRecorder.html#MediaRecorderOptions satisfies you?

@yellowdoge yellowdoge reopened this Jan 18, 2016
@mreavy
Copy link
Author

mreavy commented Jan 19, 2016

Commented in #43, closing this issue.

@mreavy mreavy closed this as completed Jan 19, 2016
@jnoring jnoring mentioned this issue Apr 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants