Scte35 creation #94

alextarasov1 · 2018-07-26T21:55:37Z

Added functionality that allows for the creation and modification of SCTE35. Exposed some private fields from SCTE35 related interfaces, such as mid. Segmentation Descriptors, Splice Commands, and SCTE35 are able to decode all of the fields instead of just skipping over them. Added Interfaces and structures that were necessary to achieve this. Every field that can be decoded into a struct can now also be encoded back into bytes.

PSI interface was removed because nothing implemented it. It was replaced by a TableHeader struct.

Tests were added for both the new SCTE35 functions and the TableHeader functions.

Example of how to use the new SCTE35 functions:

scte := scte35.CreateSCTE35()
scte.SetTier(0xFFF)
cmd := scte35.CreateTimeSignalCommand()
scte.SetCommandInfo(cmd)
cmd.SetHasPTS(true)
scte.SetPTS(0x86DF7550)

descriptor := scte35.CreateSegmentationDescriptor()
descriptor.SetEventID(0x41424344)
descriptor.SetIsEventCanceled(false)
descriptor.SetHasProgramSegmentation(true)
descriptor.SetHasDuration(false)
descriptor.SetIsDeliveryNotRestricted(false)
descriptor.SetIsWebDeliveryAllowed(false)
descriptor.SetHasNoRegionalBlackout(true)
descriptor.SetIsArchiveAllowed(true)
descriptor.SetDeviceRestrictions(scte35.RestrictNone)
descriptor.SetUPIDType(scte35.SegUPIDNotUsed)

descriptor.SetTypeID(0x10)
descriptor.SetSegmentNumber(1)
descriptor.SetSegmentsExpected(1)

scte.SetDescriptors([]scte35.SegmentationDescriptor{descriptor})

fmt.Printf("%X\n", scte.UpdateData())

WillGunther

My main concern about this PR is the exploding interfaces that are being created. If we really do want to support the next SCTE spec these are useful, but if not a lot of this code can go away. All the accessors can go away, and we can export the fields of the struct. I'm curious if other's have guidance on this.

WillGunther · 2018-08-09T22:34:28Z

psi/psi.go

@@ -60,3 +60,50 @@ func sectionSyntaxIndicator(psi []byte) bool {
 func sectionLength(psi []byte) uint16 {
 	return uint16(psi[1]&3)<<8 | uint16(psi[2])
 }
+
+func NewPointerField(size int) []byte {


A quick go doc style comment would be good here.

WillGunther · 2018-08-09T22:44:58Z

scte35/doc.go

+	SegUPADSINFO:       "SegUPADSINFO",
+	SegUPIDURN:         "SegUPIDURN",
+}
+
 // SCTE35 represent operations available on a SCTE35 message.
 type SCTE35 interface {


This interface is becoming a beast. Since we are in the business of breaking api's lately do we want to consider ditching this? Do we think there is a chance we will back this interface with SCTE 2015 and SCTE 20XX?

I think it is a good idea to remove the interfaces. But this big api breaking change should be done separately from this pull request. That way this pull request does not break any backwards compatibility.

I can get behind that

WillGunther · 2018-08-09T22:45:53Z

scte35/doc.go

@@ -204,42 +357,101 @@ type SegmentationDescriptor interface {
 	SCTE35() SCTE35
 	// EventID returns the event id
 	EventID() uint32
-	// TypeID returns the segmentation type for descriptor
-	TypeID() SegDescType
+	// SetEventID sets the event id


Same thing with this interface.

WillGunther · 2018-08-09T22:48:08Z

scte35/modify.go

+	s.commandInfo.SetHasPTS(true)
+}
+
+// HasPTS returns true if there is a pts time.


This comment doesn't match the fuction

WillGunther · 2018-08-09T23:18:10Z

scte35/scte35.go

+
+// String returns a string representation of the SCTE35 message.
+// String function is for debugging and testing.
+func (s *scte35) String() string {


We've already talked about this, but it might be worthwhile to output JSON instead of this crafted string. This could be addressed in another PR though.

Or have multiple methods: one for a formatted string and another for a JSON formatted string.

WillGunther · 2018-08-09T23:21:46Z

scte35/splicecommand.go

 func (c *spliceNull) PTS() gots.PTS {
 	return 0
 }

+type component struct {


It might be good to follow the convention that all type definitions remain in the doc.go file.

WillGunther · 2018-08-09T23:26:52Z

scte35/splicecommandmodify.go

+}
+
+// returns the bytes of this splice command.
+func (c *timeSignal) Data() []byte {


This and the func (c *spliceInsert) Data() []byte below need function names for godoc style comments

mikereedell · 2018-08-15T16:34:12Z

scte35/state.go

@@ -28,19 +28,6 @@ import "github.com/Comcast/gots"

 const receivedRingLen = 10

-type receivedElem struct {


While I don't agree with the pattern of moving private structs/fields to doc.go I'm not going to block the PR for it.

alextarasov1 added 28 commits July 13, 2018 11:51

PSI struct added

2660fec

added PSI tests and stuffing

831779d

PSI fix, added more tests

db52015

SCTE35 modification added

65f37eb

Splicecommand to byte slice conversion added

48acb27

added scte modify test and changed PSI reserved bits

90e22a2

Added support for segmentation descriptor creation

159e8c1

Added functions to interfaces to access new fields

8c2cad1

Added flag to update scte35 data only when it is changed

94cf2be

split PSI struct into two parts Pointer and TableHeader

70530ca

Added more functions to interfaces

b7eed4b

Implemented new interface functions

2962902

updated scte35 CRC

80d845f

Added subtraction capability to PTS

29010b6

removed pointer field from data function

a292ddd

added String function

95b8a94

added get and set functions

c679248

added test to create packet

2ab4407

added documentation. changed some function names.

f560291

added more tests

25c9bd5

Merge branch 'master' into scte35-creation

a4aeac2

fixed hasSubSegments

fe91f68

Fixed segmentation_duration to be a 40 bit unsigned integer, not PTS

d490224

Fixed PTS subtraction.

08d2a0b

Fixed comments, added MIT license.

c7b9819

segmentation_duration switched back to a pts.

20ec6a0

Added UpdateData function

480b309

Removed updateBytes flag in favor of UpdateData()

74c1af9

alextarasov1 mentioned this pull request Aug 8, 2018

SCTE35: segmentation duration should have 40 bits #100

Open

WillGunther requested changes Aug 9, 2018

View reviewed changes

alextarasov1 and others added 3 commits August 14, 2018 11:35

Fixed godocs

ca9f9ee

SpliceInfo is set in descriptors

2beea68

Merge branch 'master' into scte35-creation

22fd220

WillGunther approved these changes Aug 14, 2018

View reviewed changes

mikereedell reviewed Aug 15, 2018

View reviewed changes

mikereedell approved these changes Aug 15, 2018

View reviewed changes

Moved structs back to original files from doc.go

443b566

WillGunther merged commit 170a10f into Comcast:master Aug 15, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scte35 creation #94

Scte35 creation #94

alextarasov1 commented Jul 26, 2018 •

edited

Loading

WillGunther left a comment

WillGunther Aug 9, 2018

WillGunther Aug 9, 2018

alextarasov1 Aug 14, 2018

WillGunther Aug 14, 2018

WillGunther Aug 9, 2018

WillGunther Aug 9, 2018

WillGunther Aug 9, 2018

mikereedell Aug 15, 2018

WillGunther Aug 9, 2018

WillGunther Aug 9, 2018

mikereedell Aug 15, 2018

		@@ -28,19 +28,6 @@ import "github.com/Comcast/gots"

		const receivedRingLen = 10

		type receivedElem struct {

Scte35 creation #94

Scte35 creation #94

Conversation

alextarasov1 commented Jul 26, 2018 • edited Loading

WillGunther left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alextarasov1 commented Jul 26, 2018 •

edited

Loading