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 is broken and inconsistent #3462

Closed
sandeshtreeleaf opened this issue Oct 23, 2024 · 5 comments
Closed

[1.x] janus-pp-rec is broken and inconsistent #3462

sandeshtreeleaf opened this issue Oct 23, 2024 · 5 comments
Labels
multistream Related to Janus 1.x

Comments

@sandeshtreeleaf
Copy link

What version of Janus is this happening on?
Put the version and the commit identifier (available in version.c or from an info request) here
Version: 1.3.0
Identifier: 5f4fa0c

Have you tested a more recent version of Janus too?
Yes

Was this working before?
Yes, Before updating/pulling latest changes from meetecho/janus-gateway the tool was working fine,
I can still checkout to cefca79 and build then the tool works as expected

Is there a gdb or libasan trace of the issue?
NO

Additional context
janus-pp-rec --header file.mjr used to produce info that included Created and Written timestamp, latest version of it does not give that info and is very inconsistent.

Same command(janus-pp-rec --header file.mjr) produces different outputs for same file

on latest version, executing exactly same command on same file multiple times yeilds different result
image

after git checkout cefca79

[~/playground/janus-gateway cefca797]$ ./src/janus-pp-rec --header sample.mjr
Janus version: 1203 (1.2.3)
Janus commit: cefca79700bdadd32d759ce65ba3805552a4d312
Compiled on:  Wed Oct 23 05:32:21 PM +0545 2024

Logging level: 4

Source file: sample.mjr
  -- Showing header only

File is 145432 bytes
Pre-parsing file to generate ordered index...
This is a video recording:
  -- Codec:   vp8
  -- Created: 1729584192292151
  -- Written: 1729584192320744
@sandeshtreeleaf sandeshtreeleaf added the multistream Related to Janus 1.x label Oct 23, 2024
@lminiero
Copy link
Member

lminiero commented Oct 23, 2024

I can't replicate this with the recordings I have: it always shows the full (and same) details. If it aways happens for you with a specific recording, please share the mjr file so that we can check what's wrong with it.

That said, I don't think the comment you mentioned has anything to do with it, since it's a minimum change to address an FFmpeg API change, and audio only. If you know a commit where it always worked, and one where it never does, you can use git bisect to find out the exact commit that caused the issue for you. There are tutorials around on how to use git bisect.

@sandeshtreeleaf
Copy link
Author

Upon using git bisect, commit ed15555d0df831a0dcccee603fb0a62bc3a996d6 seems to be the first bad commit.

I am using videoroom's enable_recording api to record and the problem is for all files, but everything works fine before this commit. I don't have much knowledge of janus's internal working so cannot find what the problem exactly is.

Sample Mjr

Platform info if relevant:
Fedora Linux 40 (Workstation Edition) x86_64 with Kernel Linux 6.11.4
gcc version 14.2.1

@lminiero
Copy link
Member

@sandeshtreeleaf the commit you pointed out is our refactoring of the logger functionality (see #3428 for details), which I kinda suspected was what was hitting you. In my tests I eventually managed to replicate the problem, but only for like 1% of the times (on a loop of 1000 attempts, only about 10 of those were cut short). I pushed a tentative fix for that in the commit above and now I can't replicate it anymore, please pull the latest version and let us know if that fixes it for you too.

@sandeshtreeleaf
Copy link
Author

@lminiero I can confirm it has been fixed.
The numbers(1%) are interesting though as I was facing it for every attempt. Can you share your platform/device just for information?
Thank you.

@lminiero
Copy link
Member

Can you share your platform/device just for information?

Same setup as yours, probably, since my laptop runs Fedora 40 too.
I'll close this as fixed, then, thanks for the feedback!

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
Projects
None yet
Development

No branches or pull requests

2 participants