[RSDK-6469] Set x264 bitrate based on resolution and fps #4769
Conversation
viambot
added
safe to test
| @@ -622,6 +622,11 @@ func (c *webcam) Properties(ctx context.Context) (camera.Properties, error) { | |||
| if err != nil { | |||
| return camera.Properties{}, err | |||
| } | |||
|
|
|||
| if c.conf.FrameRate > 0 { | |||
| props.FrameRate = c.conf.FrameRate | |||
There was a problem hiding this comment.
Moved framerate props setter outside of conditional IntrinsicParams == nil block.
!!! Need to rebase here with latest webcam for same fix. Fixed in latest. ✅
viambot
added
safe to test
viambot
added
safe to test
| // calcBitrateFromResolution calculates the bitrate based on the given resolution and framerate. | ||
| func calcBitrateFromResolution(width, height int, framerate float32) int { | ||
| bitrate := int(float32(width) * float32(height) * framerate * encodeCompressionRatio) | ||
| if bitrate < minBitrate { |
There was a problem hiding this comment.
| if bitrate < minBitrate { | |
| // this accounts for zero bitrates too | |
| if bitrate < minBitrate { |
There was a problem hiding this comment.
Sounds good, added comment.
| // We walk the updated set of `videoSources` and ensure all of the sources are "created" and | ||
| // "started". | ||
| config := gostream.StreamConfig{ | ||
| Name: name, | ||
| VideoEncoderFactory: server.streamConfig.VideoEncoderFactory, | ||
| TargetFrameRate: framerate, |
There was a problem hiding this comment.
What happens if this is 0? Can we add a test?
There was a problem hiding this comment.
encoder pipeline will default to 30fps in bitrate calculation (the same as the key-frame interval) if it cant pick up framerate.
Will see about adding a test for this.
|
|
||
| // calcBitrateFromResolution calculates the bitrate based on the given resolution and framerate. | ||
| func calcBitrateFromResolution(width, height int, framerate float32) int { | ||
| bitrate := int(float32(width) * float32(height) * framerate * encodeCompressionRatio) |
There was a problem hiding this comment.
Does bitrate scale linearly with width and height? Apparently the answer is complicated https://www.reddit.com/r/explainlikeimfive/comments/hlrwcu/eli5_what_is_the_relationship_between_bit_rate/
There was a problem hiding this comment.
I am currently scaling the bitrate linearly with the number of pixels.
Interesting graphs in the thread that show logarithmic quality gains with increasing bitrate for a given size/fps. Will try to verify that we are hitting these sweet-spots. The amount of scene change also comes into play here so it gets quite complicated.
There was a problem hiding this comment.
If you look at the example bitrates in the description we seem to be pretty bang on to the recommended settings linked in the jira ticket: https://support.google.com/youtube/answer/2853702?hl=en
| @@ -507,11 +507,16 @@ func (server *Server) AddNewStreams(ctx context.Context) error { | |||
| server.logger.Warn("video streaming not supported on Windows yet") | |||
| break | |||
| } | |||
| framerate, err := server.getFramerateFromCamera(name) | |||
There was a problem hiding this comment.
will this new logic break streaming for cameras that don't provide framerate in getProperties?
There was a problem hiding this comment.
Will add comment here -- encoder pipeline will default to 30fps in bitrate calculation (the same as the key-frame interval) if it cant pick up framerate.
viambot
added
safe to test
viambot
added
safe to test
Description
This PR gives a more accurate bitrate estimation to our libx264 pipeline based on resolution and framerate. The idea here is that smaller streams will use less bandwidth and we will get better quality out of larger streams.
Examples bitrates:
Tests