-
Notifications
You must be signed in to change notification settings - Fork 178
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
Control scene classification with T command line args #2686
Conversation
…results on B, even if not requested for stream
if len(res.Detections) > 0 { | ||
for _, detection := range res.Detections { | ||
switch x := detection.Value.(type) { | ||
case *net.DetectData_SceneClassification: |
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.
Worth logging in a default
case to catch other cases?
actualProfile.(*ffmpeg.SceneClassificationProfile).Classes = DetectorProfile.(*ffmpeg.SceneClassificationProfile).Classes | ||
} | ||
} | ||
if actualProfile != nil && actualProfile.(*ffmpeg.SceneClassificationProfile).SampleRate < math.MaxUint32 { |
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.
What's the reason for comparison to MaxUint32
rather than having it be zero?
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.
That value ends up in something like select=not(mod(Nframe, SampleRate))
in Ffmpeg filter, so we don't want to have 0 here. Hence, MAX as default value.
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.
👍 Gotcha
actualProfile := DetectorProfile | ||
if aiEnabled { | ||
presetSampleRate := DetectorProfile.(*ffmpeg.SceneClassificationProfile).SampleRate | ||
if md.DetectorEnabled && len(md.DetectorProfiles) == 1 { |
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.
Is there a case where there can be more than one DetectorProfiles?
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.
Yes, these structures were designed with an aim to have multiple AI capabilities some day.
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.
Yep for sure, I was more checking that there was no chance we'd unintentionally hit md.DetectorEnabled && len(md.DetectorProfiles) > 1
at some point
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.
This all looks good, left a few questions that were more for my understanding than problems with the PR
Codecov Report
@@ Coverage Diff @@
## master #2686 +/- ##
===================================================
+ Coverage 56.33537% 56.36373% +0.02836%
===================================================
Files 88 88
Lines 19107 19124 +17
===================================================
+ Hits 10764 10779 +15
+ Misses 7758 7757 -1
- Partials 585 588 +3
Continue to review full report at Codecov.
|
-detectionSampleRate
parameter to control non-stream specific scene classification on T