Skip to content

Data tracks core - #990

Open
ladvoc wants to merge 9 commits into
ladvoc/data-track-channelfrom
ladvoc/data-track-core
Open

Data tracks core#990
ladvoc wants to merge 9 commits into
ladvoc/data-track-channelfrom
ladvoc/data-track-core

Conversation

@ladvoc

@ladvoc ladvoc commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Adds core types (e.g., info, frame, schema, etc.) and packetizer/depacketizer.

Areas to review:

  • Should some of this implementation be moved to livekit/protocol?

Closes BOT-541

@ladvoc
ladvoc force-pushed the ladvoc/data-track-core branch from f15a229 to b7b6532 Compare September 4, 2026 16:04
@ladvoc
ladvoc marked this pull request as ready for review September 4, 2026 16:07
@ladvoc
ladvoc requested a review from a team as a code owner September 4, 2026 16:07
@ladvoc
ladvoc requested a review from boks1971 September 4, 2026 16:07

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 4 potential issues.

Devin Review

Comment thread datatrack/depacketizer.go
Comment thread datatrack/packetizer.go
Comment thread datatrack/packetizer.go
Comment thread datatrack/clock.go
@ladvoc
ladvoc force-pushed the ladvoc/data-track-core branch from b7b6532 to 1c85018 Compare September 4, 2026 22:58

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Devin Review

Comment thread datatrack/depacketizer.go
@ladvoc
ladvoc force-pushed the ladvoc/data-track-core branch from 5572d61 to dc69ba0 Compare September 8, 2026 21:55
Comment thread datatrack/clock.go
"time"
)

const timestampRate = 90_000

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename to clockRate? as same as media clockrate

Comment thread datatrack/clock.go
}

func (t timestamp) isBefore(other timestamp) bool {
return int32(t-other) < 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Both a.isBefore(b) and b.isBefore(a) are true when the distance of a and b is 2^31 . Maybe add check:

      if t-other == 1<<31 {
              return t < other
      }

Comment thread datatrack/depacketizer.go
func (d *depacketizer) frameFromSingle(packet dtp.Packet, options depacketizerPushOptions) depacketizerPushResult {
var result depacketizerPushResult
if len(d.partials) >= options.maxPartialFrames {
result.drop = d.evictOldest(packet.FrameNumber)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single frame has not been appended to the partials, do we still need to evict one?

Comment thread datatrack/depacketizer.go
}

func finalize(frameNumber uint16, partial *partialFrame, endSequence uint16) depacketizerPushResult {
received := uint16(len(partial.payloads))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a quick check if received != 1 + endSequece - partial.StartSequence for the incomplete frame and return earlier.

Comment thread datatrack/handle.go
value uint16
}

func (a *handleAllocator) get() (trackHandle, bool) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use an explicit error like handle reaches limits?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants