fix(scte35): Condition descriptor-based signals correctly - #10
Merged
Conversation
Two defects made the conditioning path unreliable for signals that carry a segmentation descriptor, which is how modern SCTE-35 usually arrives: a time_signal plus a descriptor rather than a splice_insert. Both were found while verifying the example documented in the README. Duration was read in the wrong unit. The parser stores BreakDuration.duration in 90kHz ticks but keeps SegmentationDescriptor.segmentation_duration in seconds, as threefive returns it. The rule engine and the stateful break expiry divided both by 90000, so a 60 second placement opportunity was read as 0 seconds. Every "duration" condition therefore failed to match on descriptor-based signals, and stateful mode calculated no expiry for those breaks. Placement opportunity Start descriptors failed to encode. threefive requires sub_segment_num and sub_segments_expected to be set when the segmentation type is 0x34, 0x36, 0x38 or 0x3A. Sources are not required to send those fields and decoding leaves them unset, so encoding raised, the encoder fell back to returning the unmodified signal, and the response still reported the modification as applied. They now default to 0, the value the specification uses for "not used". The README example also targeted breakDuration while matching on segmentationTypeId. Those belong to different signal forms, so the modification silently did nothing on the very signal the example describes. It now targets segmentationDuration, and the guidance warns that choosing the wrong target leaves the payload unchanged while the response still reports "replace". This change also completes the manual administrator fallback, which listed only admin-create-user. That command leaves the account outside the admin group, and the handlers authorize writes from the "cognito:groups" claim, so the operator could read channels and logs but could not change them, manage users, or view encoder credentials. Adds regression coverage for the units, the expiry calculation, encoding of all four Start types, and the documented rule end to end. Verified that the new tests fail without these fixes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two defects that made SCTE-35 conditioning unreliable for signals carrying a segmentation descriptor, which is how modern SCTE-35 usually arrives: a
time_signalplus a descriptor rather than asplice_insert.Both were found while verifying the example documented in the README, after PR #9 aligned the documentation with the implementation. The example did not work on the signal it describes.
Defect 1: duration read in the wrong unit
The parser stores the two duration carriers in different units:
BreakDuration.durationSegmentationDescriptor.segmentation_duration_get_duration()andcalculate_break_expiry_time()divided both by 90000. A 60 second placement opportunity was therefore read as60.0 // 90000 == 0seconds.Consequences:
durationcondition failed to match on descriptor-based signals. A rule such asduration > 30could never fire, so the signal fell through to the channel's default action.None.The
splice_insertpath was unaffected, which is why this went unnoticed.Defect 2: placement opportunity Start descriptors failed to encode
threefive requires
sub_segment_numandsub_segments_expectedto be set whensegmentation_type_idis0x34,0x36,0x38or0x3A:Sources are not required to send those fields, and decoding leaves them unset. Encoding then raised
ValueError: sub_segment_num is not set, andencode_scte35caught it and returned the unmodified signal:The response still reported
replaceand the log still said "Signal modified", so the conditioning silently did nothing. They now default to0, the value the specification uses for "not used".Documentation
The README example matched on
segmentationTypeIdbut modifiedbreakDuration. Those belong to different signal forms, so the modification did nothing on the very signal the example describes. It now targetssegmentationDuration, and the guidance warns that choosing the wrong target leaves the payload unchanged while the response still reportsreplace.This also completes the manual administrator fallback, which listed only
admin-create-user. That command leaves the account outside theadmingroup, and the handlers authorize writes from thecognito:groupsclaim, so the operator could read channels and logs but could not change them, manage users, or view encoder credentials. Confirmed against a Cognito pool configured like the deployment: group membership is empty untiladmin-add-user-to-groupruns.Testing
New regression suite in
backend/tests/unit/test_descriptor_signal_conditioning.pycovering the duration units, the expiry calculation, encoding of all four Start types, and the documented rule end to end.Verified that the new tests fail without these fixes:
Full suite:
Behavior confirmed on a real payload, a
time_signalwith a0x34descriptor of 60 seconds:segmentationDurationbreakDuration