Skip to content
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

jbuf: frame fixes #806

Merged
merged 2 commits into from
May 17, 2023
Merged

jbuf: frame fixes #806

merged 2 commits into from
May 17, 2023

Conversation

sreimers
Copy link
Member

@sreimers sreimers commented May 16, 2023

Since packets can be lost or late/reordered, this can lead to multiple counting.

@sreimers sreimers marked this pull request as ready for review May 16, 2023 14:07
@sreimers sreimers changed the title jbuf: avoid multiple frame count jbuf: fix frame count May 16, 2023
@sreimers sreimers marked this pull request as draft May 16, 2023 19:37
@sreimers sreimers force-pushed the jbuf_video_adaptive branch 2 times, most recently from be7bb95 to 98a7ea6 Compare May 17, 2023 08:09
@sreimers sreimers changed the title jbuf: fix frame count jbuf: frame fixes May 17, 2023
@sreimers sreimers marked this pull request as ready for review May 17, 2023 08:10
Since packets can be lost or late/reorderd, this can lead to multiple counting.
@sreimers sreimers merged commit 43269c8 into main May 17, 2023
@sreimers sreimers deleted the jbuf_video_adaptive branch May 17, 2023 08:20
if (f->le.prev) {
struct packet *pre_f = f->le.prev->data;
jb->nf = 1;
LIST_FOREACH(&jb->packetl, le)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to avoid O(n) effort for jbuf_put()?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have not found a reliable way. Out-of-order, delayed packets lead to double or missed frame counts, and incorrect frame counts have a big impact on latency and jitter calculations.

Copy link
Member Author

@sreimers sreimers May 22, 2023

Choose a reason for hiding this comment

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

One way maybe could a separate list with frame timestamps

Edit: but this optimizes only video frame counting.

Copy link
Collaborator

@cspiel1 cspiel1 May 22, 2023

Choose a reason for hiding this comment

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

The packets are inserted sorted by the sequence number. The sorted insert needs normally only one to a few checks.
Timestamps and sequence number should have the same order. So after the insert only one or a few checks are needed to find packets with same timestamp. Did I miss something?

Edit: I'll try a PR draft for a closer look.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Draft: #821

Copy link
Member Author

@sreimers sreimers May 22, 2023

Choose a reason for hiding this comment

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

The problem is, we don't know if a sequence number is missing, because its lost or because it arrives later. This makes a simple increment unreliable. Maybe a list of missing sequence numbers can work. I plan to add RTCP Feedback NACK to trigger a resend for missing packets (video only).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants