Skip to content

gots.InsertPTS improvements #121

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.

Sign up for GitHub

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

Open
tmm1 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

gots.InsertPTS improvements #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e499af9
pts: flesh out tests
tmm1 May 8, 2019
553f7a4
pts: ensure only one bit is set during marking
tmm1 May 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

@tmm1 tmm1 May 16, 2019

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.

Copy link
Collaborator

@BlakeOrth BlakeOrth May 16, 2019

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.

Copy link
Contributor Author

@tmm1 tmm1 May 16, 2019

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?

Copy link
Collaborator

@BlakeOrth BlakeOrth May 16, 2019

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.

b[2] |= 0x01
b[4] |= 0x01
}
13 changes: 10 additions & 3 deletions pts_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func TestPTSIsAfterWithoutRollover(t *testing.T) {

func TestPTSIsAfterWithRollover(t *testing.T) {
p := PTS(1)
other := PTS(8589934591) // MaxPtsValue
other := PTS(MaxPtsValue)
if !p.After(other) {
t.Errorf("PTS=%v, not After other=%v", p, other)
}
Expand Down Expand Up @@ -125,8 +125,15 @@ func TestAdd(t *testing.T) {
func TestInsertPTS(t *testing.T) {
var pts uint64 = 0x1DEADBEEF
b := make([]byte, 5)

InsertPTS(b, pts)
if ots := ExtractTime(b); ots != pts {
t.Errorf("Insert PTS test 1 failed: %v != %v", ots, pts)
}

pts = MaxPtsValue
InsertPTS(b, pts)
if ExtractTime(b) != 0x1DEADBEEF {
t.Error("Insert PTS test 1 failed")
if ots := ExtractTime(b); ots != pts {
t.Errorf("Insert PTS test 2 failed: %v != %v", ots, pts)
}
}