Skip to content

stream.ffmpegmux: add loglevel #6161

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
Merged
bastimeyer merged 1 commit into from
Sep 5, 2024
Merged

stream.ffmpegmux: add loglevel #6161

bastimeyer merged 1 commit into from
Sep 5, 2024
+76 −2

Conversation

@soprano1125
Copy link
Contributor

@soprano1125 soprano1125 commented Sep 5, 2024

Hello.

When creating a plugin using FFMPEGMuxer, I wanted to display a finer log level (verbose, debug) than FFmpeg's info, so I added the argument “--ffmpeg-loglevel”.

ffmpeg Documentation: 5.2 Generic options

[soprano1125@localhost streamlink]$ ffmpeg -hide_banner -loglevel test
Invalid loglevel "test". Possible levels are numbers or:
"quiet"                                                                                                                                            
"panic"                                                                                                                                            
"fatal"                                                                                                                                            
"error"                                                                                                                                            
"warning"                                                                                                                                          
"info"                                                                                                                                             
"verbose"                                                                                                                                          
"debug"                                                                                                                                            
"trace"

bastimeyer
bastimeyer requested changes Sep 5, 2024
View reviewed changes
Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

Some changes are required before this can be merged.

Please ignore the failing tests for now. I will update them myself by adding to your PR branch (which btw is your fork's master branch) afterwards.

bastimeyer
bastimeyer reviewed Sep 5, 2024
View reviewed changes
@bastimeyer bastimeyer changed the title cli: add --ffmpeg-loglevel stream.ffmpegmux: add loglevel Sep 5, 2024
bastimeyer
bastimeyer approved these changes Sep 5, 2024
View reviewed changes
Copy link
Member

@bastimeyer bastimeyer left a comment

Choose a reason for hiding this comment

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

I just force-pushed onto your PR branch and fixed the remaining issues:
https://github.com/streamlink/streamlink/compare/353a3b3def65f0546067c141460fda21de74aac7..14d6bb999705295d9b58b12d2d6d13479077d441

Changes should be fine, but I'll have another quick look later today before merging.

Again, thanks for the PR.

@soprano1125
Copy link
Contributor Author

soprano1125 commented Sep 5, 2024

Thank you for sending your gracious response and review so promptly.

@soprano1125 @bastimeyer
stream.ffmpegmux: add loglevel
3c46e51 
- Add `ffmpeg-loglevel` session option
- Add `--ffmpeg-loglevel` CLI argument
- Add FFmpeg `-loglevel` argument in FFMPEGMuxer, default to `info`
- Update tests

Co-Authored-By: bastimeyer <mail@bastimeyer.de>
@bastimeyer
Copy link
Member

bastimeyer commented Sep 5, 2024

There's one issue which adding this CLI argument / session option introduces, and that is that the FFmpeg process will terminate immediately when choosing an invalid -loglevel value. The current FFMPEGMuxer implementation doesn't handle this and then needs to wait until each copy-to-pipe thread ends from a read timeout, which takes 60 seconds (default stream-timeout value). It's an old implementation which will need to be rewritten eventually. I'm not going to touch this now. Checking the user's loglevel input doesn't make sense, as not only log level names, but also log level numbers and flags are supported by FFmpeg, which would all need to be parsed correctly.

Let's merge this PR now. If the user inputs an incorrect log level value, then it's their own fault.

@bastimeyer bastimeyer merged commit c08c297 into streamlink:master Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bastimeyer bastimeyer bastimeyer approved these changes

Assignees

No one assigned

Labels

API: session CLI stream: ffmpegmux

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@soprano1125 @bastimeyer