Move from using PUSH_END to signal to end of the segmenting flow to using RECORDING_END #78
Conversation
Codecov Report
@@ Coverage Diff @@
## main #78 +/- ##
===================================================
- Coverage 39.36430% 38.92023% -0.44407%
===================================================
Files 18 18
Lines 1227 1241 +14
===================================================
Hits 483 483
- Misses 688 702 +14
Partials 56 56
Continue to review full report at Codecov.
|
…sing RECORDING_END
The segmented video's duration should closely match the input video's duration. If it's within a tolerable range, then transcode can be kicked off. Otherwise, the VOD flow should bail.
emranemran
force-pushed
the
transcode-end
branch
from
e026c6c
to
6a79a72
Compare
| if math.Abs(float64(inputVideoLengthMillis-p.StreamMediaDurationMillis)) > 500 { | ||
| _ = config.Logger.Log("msg", "Input video duration does not match segmented video duration", | ||
| "input video duration (ms):", inputVideoLengthMillis, "segmented video duration (ms):", p.StreamMediaDurationMillis) | ||
| return |
There was a problem hiding this comment.
I'm thinking we should have a callback to Studio here to indicate the segmented file's duration doesn't match the input file's duration. Currently arbitrarily set a 500ms delta as "acceptable". wdyt @victorges @thomshutt?
There was a problem hiding this comment.
Yeah, we definitely need an error callback anywhere we're returning early. Delta sounds good, could maybe even be more lenient to begin with
There was a problem hiding this comment.
Agreed, if this is a terminal error there must be an error callback. Might even be an unretriable one? Or is this a transient error?
The contract is that the upload process will always end with a callback. It can't just disappear unless something unexpected happens with the service like the machine or pod shutting down (then it's the regular "task lost" which we'll retry).
There was a problem hiding this comment.
OK, will follow up in #82 to address these integration points.
|
Tested this PR locally and worked as expected - need to sanity check on canary prior to merging. |
|
Holding off on merging this until we settle on the issue with the small manifests issue |
No description provided.