-
Notifications
You must be signed in to change notification settings - Fork 88
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
gots.InsertPTS improvements #121
base: master
Are you sure you want to change the base?
Conversation
@@ -129,7 +129,7 @@ func InsertPTS(b []byte, pts uint64) { | |||
b[4] = byte(pts&0xff) << 1 // PTS[7..0] | |||
|
|||
// Set the marker bits as appropriate | |||
b[0] |= 0x21 | |||
b[0] |= 0x01 |
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.
I'm not really sure about this change. The tests pass with and without it.
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.
Oh, I'm glad you commented on this, as we don't want to change from 0x21
to 0x01
. 0x21
sets the PTS_DTS_flags
field to "has PTS". However, I will agree that's a bit sneaky and we should probably include a clarifying comment.
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.
I see, so that means this function is not suitable for writing DTS. Don't the number of marker bits required vary depending on whether DTS is being written also?
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.
You could use this function to write DTS, but you would have to set the mask on b[0]
to 0x11
for the DTS portion of the PTS_DTS field and then append the two generated byte buffers together. Realistically I don't think that's the best plan if you need to write both, but it would be your code, not mine :) The actual marker_bits are the same between the two fields, and they don't change position or anything like that if you have both PTS and DTS.
If you need to write a PTS_DTS field, rather than just PTS, I would probably recommend implementing a InsertPTSDTS(b []byte, pts uint64, dts uint64)
where you could call InsertPTS(b[:5], pts)
and then insert the DTS and modify the PTS_DTS_flag (b[0]
) accordingly.
@@ -129,7 +129,7 @@ func InsertPTS(b []byte, pts uint64) { | |||
b[4] = byte(pts&0xff) << 1 // PTS[7..0] | |||
|
|||
// Set the marker bits as appropriate | |||
b[0] |= 0x21 | |||
b[0] |= 0x01 |
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.
Oh, I'm glad you commented on this, as we don't want to change from 0x21
to 0x01
. 0x21
sets the PTS_DTS_flags
field to "has PTS". However, I will agree that's a bit sneaky and we should probably include a clarifying comment.
cleaned up the tests a bit, and fixed what appears to be a bug in the marker bit insertion.