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

[RSDK-8903] - Implement simple keyframe force interval #18

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Conversation

seanavery
Copy link
Collaborator

@seanavery seanavery commented Oct 3, 2024

Description

There was an issue where the segmenter would flake out after some time. This was due to a keyframe not being correctly inserted into the beginning of the segment as expected. It turns out, the gop_size encoder settings are more of a recommendation than an absolutely firm keyframe interval. Dependenging on scene changes, a keyframe could be inserted earlier than expected throwing off the synchronization between the encoder and segmenter.

FFmpeg codec options actually include a force_key_frames expression handler, so we do not even have to hand roll the key frame forces in our image loop! Basically a one line opt call to fix this : )

Test

  • Long running test with viamrtsp camera of size 960x480 and 20FPS
    • ~5 hours, ~3000 segments
    • All playable so far...
2024-10-03_17-48-40.mp4

ffprobe -v error -select_streams v:0 -show_frames -print_format json 2024-10-03_17-48-40.mp4 > frames_info.json
frames_info.json

Update here after overnight test: ✅

  • 10 GB storage, 8000 segments, 12 hours runtime

@seanavery seanavery requested review from randhid and hexbabe October 3, 2024 22:07
Copy link
Collaborator

@hexbabe hexbabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad the fix could be done declaratively as a flag!

go.mod Outdated
@@ -14,6 +14,7 @@ require (
go.viam.com/rdk v0.40.0
go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2
go.viam.com/utils v0.1.98
golang.org/x/exp v0.0.0-20240103183307-be819d1f06fc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to run go mod tidy from last PR.

But thanks for pointing this out, for some reason I was using the external package instead of math/rand.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed that dep from example script.

cam/cam.go Show resolved Hide resolved
@@ -9,6 +9,7 @@ package main

import (
"context"
"math/rand"
Copy link
Collaborator Author

@seanavery seanavery Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small fix for a tidy mod.

@seanavery seanavery merged commit 4c26696 into main Oct 4, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants