Fixing SCTE-35 implicit signal closing rules. #63
Conversation
…ending. - Treat the open events slice like a stack, with the oldest event on the bottom since the logic that closes events iterates from the tail of the slice to the head. - Added unit test that illustrated this issue and verifies the fix.
…r implicit signal rules. - Modified code around segCloseEventIDNotNested to account for expected segments. - Changed segCloseNotNested to deal with 0x34 and 0x36 w/subsegments only. - Changed segmentation descriptor equality to account for segments/subsegments. - Created unit test for segmentation descriptor to get full coverage of CanClose(). - Added unit tests for state.go to increase coverage of state handling.
mikereedell
changed the title
|
I haven't forgotten about this... will get to it tomorrow. |
scte35/segmentationdescriptor.go
Outdated
| // this should also consider segnum == segexpected for IN signals closing an out signal. | ||
| if d.IsIn() && d.EventID() == out.EventID() && d.SegmentNumber() == d.SegmentsExpected() { | ||
| return true | ||
| } | ||
| if d.EventID() != out.EventID() { | ||
| return false | ||
| } | ||
| fallthrough |
There was a problem hiding this comment.
Is this fallthrough still needed? From cog esam 2.0 doc, it seems like the logic has diverged enough that only the first if block is needed and all the old code can be deleted.
There was a problem hiding this comment.
The if d.EventId()... statement is extraneous. As is the fallthrough.
scte35/segmentationdescriptor.go
Outdated
| // if in, last desc is x/x | ||
| if d.segNum == d.segsExpected { | ||
| case segCloseNotNested: // only applies to 0x34 and 0x36 with subsegments. | ||
| if d.IsOut() && (d.TypeID() == SegDescProviderPOStart || d.TypeID() == SegDescDistributorPOStart) { |
There was a problem hiding this comment.
I feel like this should be dictated by the closeRules map only 0x34 and 0x36 should have segCloseNotNested, the extra logic here is unneeded, and two places will have to be updated if another signal gets this close rule footnote.
scte35/segmentationdescriptor.go
Outdated
| } else if d.IsOut() { | ||
| // if out, first descriptor in set closes existing open | ||
| if d.segNum == 1 { | ||
| if d.TypeID() == out.TypeID() { |
There was a problem hiding this comment.
Same story here, I think this is already done in the close rules map and this is just extra, unneeded logic.
- Removed extra logic around segCloseEventIDNotNested and segCloseNotNested. - Removed fallthrough statement in switch statement. - Cleaned up logic in segCloseNotNested.
logic that closes events iterates from the tail of the slice to the head.