-
Notifications
You must be signed in to change notification settings - Fork 113
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
[RSDK-6469] Set x264 bitrate based on resolution and fps #4769
base: main
Are you sure you want to change the base?
Changes from all commits
7eaf687
1780a0c
11f81ae
3573204
4bbc895
e789a17
c11fb11
86affca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -6,6 +6,17 @@ import ( | |||||||
"go.viam.com/rdk/logging" | ||||||||
) | ||||||||
|
||||||||
const ( | ||||||||
encodeCompressionRatio = 0.15 // bits per pixel when encoded | ||||||||
// For very small resolutions, we need to ensure that the vbv buffer size is large enough to | ||||||||
// handle frame bursts. This is the minimum bitrate that we can use without causing the encoder | ||||||||
// to spew out warnings about the buffer size being too small. | ||||||||
minBitrate = 300_000 // 300kbps | ||||||||
// Setting a reasonable max bitrate to prevent the encoder from using too much bandwidth. | ||||||||
// 4K resolution at 20fps is around 24.8Mbps. | ||||||||
maxBitrate = 25_000_000 // 25Mbps | ||||||||
) | ||||||||
|
||||||||
// DefaultStreamConfig configures x264 as the encoder for a stream. | ||||||||
var DefaultStreamConfig gostream.StreamConfig | ||||||||
|
||||||||
|
@@ -27,3 +38,16 @@ func (f *factory) New(width, height, keyFrameInterval int, logger logging.Logger | |||||||
func (f *factory) MIMEType() string { | ||||||||
return "video/H264" | ||||||||
} | ||||||||
|
||||||||
// 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) | ||||||||
// This accounts for zero bitrates too. | ||||||||
if bitrate < minBitrate { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, added comment. |
||||||||
return minBitrate | ||||||||
} | ||||||||
if bitrate > maxBitrate { | ||||||||
return maxBitrate | ||||||||
} | ||||||||
return bitrate | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -507,11 +507,18 @@ func (server *Server) AddNewStreams(ctx context.Context) error { | |
server.logger.Warn("video streaming not supported on Windows yet") | ||
break | ||
} | ||
// Attempt to look up the framerate for the camera. If the framerate is not available, we'll | ||
// end up with a framerate of 0. This is fine as gostream will default to 30fps in this case. | ||
framerate, err := server.getFramerateFromCamera(name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will this new logic break streaming for cameras that don't provide framerate in getProperties? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
if err != nil { | ||
server.logger.Debugf("error getting framerate from camera %q: %v", name, err) | ||
} | ||
// 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if this is 0? Can we add a test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Will see about adding a test for this. |
||
} | ||
// Call `createStream`. `createStream` is responsible for first checking if the stream | ||
// already exists. If it does, it skips creating a new stream and we continue to the next source. | ||
|
@@ -758,6 +765,18 @@ func (server *Server) startAudioStream(ctx context.Context, source gostream.Audi | |
}) | ||
} | ||
|
||
func (server *Server) getFramerateFromCamera(name string) (int, error) { | ||
cam, err := camera.FromRobot(server.robot, name) | ||
if err != nil { | ||
return 0, fmt.Errorf("failed to get camera from robot: %w", err) | ||
} | ||
props, err := cam.Properties(context.Background()) | ||
if err != nil { | ||
return 0, fmt.Errorf("failed to get camera properties: %w", err) | ||
} | ||
return int(props.FrameRate), nil | ||
} | ||
|
||
// GenerateResolutions takes the original width and height of an image and returns | ||
// a list of the original resolution with 4 smaller width/height options that maintain | ||
// the same aspect ratio. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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