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.

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

Jump to bottom

Samplebuilder: add tests for padding packets #2328

Merged
merged 1 commit into from
Sep 15, 2023
Merged

Samplebuilder: add tests for padding packets #2328

merged 1 commit into from
Sep 15, 2023

Conversation

tmatth
Copy link
Contributor

@tmatth tmatth commented Oct 10, 2022 •
edited
Loading

Refs #2323

Co-authored-by: Juliusz Chroboczek [email protected]

@tmatth tmatth mentioned this pull request Oct 10, 2022
Closed
2 tasks
@tmatth tmatth force-pushed the samplebuilder/padding-test branch 2 times, most recently from 7dd45af to a21d879 Compare November 3, 2022 15:28
@codecov
Copy link

codecov bot commented Nov 3, 2022 •
edited
Loading

Codecov Report

Patch coverage is 100.00% of modified lines.

Files Changed Coverage
pkg/media/samplebuilder/samplebuilder.go 100.00%

📢 Thoughts on this report? Let us know!.

@tmatth tmatth force-pushed the samplebuilder/padding-test branch 4 times, most recently from ae50000 to 698797a Compare November 3, 2022 17:36
@tmatth tmatth changed the title Samplebuilder: add test for padding packets Samplebuilder: add tests for padding packets Nov 3, 2022
@tmatth tmatth force-pushed the samplebuilder/padding-test branch 4 times, most recently from 010661f to c9e2ec3 Compare November 7, 2022 20:59
@tmatth tmatth force-pushed the samplebuilder/padding-test branch 2 times, most recently from ec8291b to 7fbacfe Compare November 18, 2022 21:06
@Sean-Der Sean-Der force-pushed the samplebuilder/padding-test branch 2 times, most recently from a892e80 to 507a983 Compare September 15, 2023 21:08
@Sean-Der Sean-Der marked this pull request as ready for review September 15, 2023 21:08
@Sean-Der Sean-Der force-pushed the samplebuilder/padding-test branch 2 times, most recently from 9e336ad to 07328f3 Compare September 15, 2023 21:10
@Sean-Der
Copy link
Member

@tmatth Mind looking at this one more time before I merge?

I rebased and removed the new public API members. No one has requested them, so don't want to expand the public API unless needed!

Sorry this took so long :( just working on getting PRs down to zero now.

@tmatth
Copy link
Contributor Author

tmatth commented Sep 15, 2023

@tmatth Mind looking at this one more time before I merge?

I rebased and removed the new public API members. No one has requested them, so don't want to expand the public API unless needed!

Sorry this took so long :( just working on getting PRs down to zero now.

Thanks! LGTM, I still never really got to the bottom of the main issue I wanted to address whereby A/V sync would diverge if there was a long interruption in video packets (not lost, just nothing being sent by the publisher for a few seconds). Any thoughts?

@Sean-Der
Copy link
Member

@tmatth i think you will need to delay using the sender reports? I can help implement that

@Sean-Der Sean-Der merged commit 14eb615 into master Sep 15, 2023
19 checks passed
@Sean-Der Sean-Der deleted the samplebuilder/padding-test branch September 15, 2023 23:39
@tmatth
Copy link
Contributor Author

tmatth commented Sep 15, 2023

@tmatth i think you will need to delay using the sender reports? I can help implement that

Can this be done internally in SampleBuilder or is it up to users of SampleBuilder to handle that?

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

@k0nserv k0nserv k0nserv left review comments

@Sean-Der Sean-Der Sean-Der approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tmatth @Sean-Der @k0nserv