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

[1.x] janus-pp-rec creates opus that doesn't start at 0 #3486

Open
benbro opened this issue Dec 9, 2024 · 6 comments
Open

[1.x] janus-pp-rec creates opus that doesn't start at 0 #3486

benbro opened this issue Dec 9, 2024 · 6 comments
Labels
multistream Related to Janus 1.x pr-exists

Comments

@benbro
Copy link

benbro commented Dec 9, 2024

What version of Janus is this happening on?
version: 1301 (1.3.1)
commit: c08c219

Was this working before?
Same result with version 1.1.2

Additional context
When I post-process the attached mjr file I'm getting an opus file that starts at 0.86 seconds instead of 0. The first packet in the mjr has Recvd Time = 0.
ffprobe shows:
Duration: 00:00:04.40, start: 0.860000, bitrate: 19 kb/s

test.zip

@benbro benbro added the multistream Related to Janus 1.x label Dec 9, 2024
@atoppi
Copy link
Member

atoppi commented Dec 10, 2024

@benbro can you take a look at #3488 ?
The patch does not "fix" the opus starting time, since that is probably due to the bitstream and unrelated to pp-rec.
Anyway while checking your mjr we noticed that it's basically silence with DTX and we found some issues in the way we handle this scenario in pp-rec.
If you have other problematic mjrs with actual audio and not just silence, please try to share them.

@atoppi
Copy link
Member

atoppi commented Dec 10, 2024

It's also worth mentioning that there was a bug in VideoRoom (fixed in e296d1a) that enabled DTX negotiation even when disabled (default).

@benbro
Copy link
Author

benbro commented Dec 10, 2024

@atoppi thank you for #3488.
Do I need to use it with the --restamp 1 flag? Any other flags I need to use for this specific mjr?
janus-pp-rec --restamp 1 test.mjr test.opus

The patch does not "fix" the opus starting time, since that is probably due to the bitstream and unrelated to pp-rec.

If it doesn't fix the opus starting time, how should I test this PR?
Since the first packet has Recvd Time = 0 I thought the starting time should be zero. What do you mean by "due to the bitstream"?

If you have other problematic mjrs with actual audio and not just silence, please try to share them.

This is an actual few minutes mjr with real audio. Since it has private info I trimmed it and replaced the payload with dummy data so I can share it.

@atoppi
Copy link
Member

atoppi commented Dec 10, 2024

There's no need for any flag, dtx should be detected automatically.

With testing I mean post processing your recordings and check if total length is like expected, lip sync is okay etc.

Can you share the actual recording? You can send an email to [email protected]

@benbro
Copy link
Author

benbro commented Dec 10, 2024

Tested with #3488 (without any flags) and now lip sync is good.
The opus file has the same duration after the patch and the start time is zero as expected.
Thank you.
This issue can be closed.

@atoppi
Copy link
Member

atoppi commented Dec 10, 2024

Thanks for reporting.
We'll close the issue after reviewing and merging the PR.

@atoppi atoppi added pr-exists and removed needinfo labels Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
multistream Related to Janus 1.x pr-exists
Projects
None yet
Development

No branches or pull requests

2 participants