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

Use ffmpeg parser for H.264 #431

Merged
merged 2 commits into from
Jan 18, 2025
Merged

Use ffmpeg parser for H.264 #431

merged 2 commits into from
Jan 18, 2025

Conversation

j0sh
Copy link
Collaborator

@j0sh j0sh commented Jan 15, 2025

Fixes a number of things with the nvidia built-in parser (cuvid) including a LPMS crash, choppy video quality, green screen flashing during rotation, inconsistent frame counts vs software decoding, etc. We also apparently gained GPU support for MPEG2 decoding.

This is a massive change: we can no longer add outputs up front due to the ffmpeg hwaccel API, so we have to wait until we receive a decoded video frame in order to add outputs. This also means properly queuing up audio and draining things in the same order.

This also unfortunately adds ~13 MB of new samples to the repo for test cases, since these are hard to generate synthetically.

Fixes a number of things including a LPMS crash, choppy video
quality, green screens during rotation, inconsistent frame counts
vs software decoding, etc.

This is a massive change: we can no longer add outputs up front
due to the ffmpeg hwaccel API, so we have to wait until we receive
a decoded video frame in order to add outputs. This also means
properly queuing up audio and draining things in the same order.
Copy link
Contributor

@leszko leszko left a comment

Choose a reason for hiding this comment

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

I skimmed through the PR and looks good. Saying that, I don't have enough LPMS knowledge to review it in depth, so if you need a "better" review, then you can wait for approves from some other folks.

fname := dir + "/mpeg2.ts"
oname := dir + "/out.ts"
fname := dir + "/test.flv"
//oname := dir + "/out.ts"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Remove commented-out code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the spot, fixed and cleaned up in 9e51cee

@@ -603,6 +603,7 @@ func TestNvidia_API_AlternatingTimestamps(t *testing.T) {
tc := NewTranscoder()
idx := []int{1, 0, 3, 2}
for _, i := range idx {
// TODO this breaks with nvidia acceleration on the input!
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that this is not a new problem with this PR; the old code had the same issue

@mjh1
Copy link
Contributor

mjh1 commented Jan 16, 2025

I skimmed through the PR and looks good. Saying that, I don't have enough LPMS knowledge to review it in depth, so if you need a "better" review, then you can wait for approves from some other folks.

Maybe @emranemran you could take a look at the C code? Can't remember if you've reviewed much of lpms in the past..

Copy link
Contributor

@mjh1 mjh1 left a comment

Choose a reason for hiding this comment

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

Similar to Rafal I'm afraid I don't know enough about this stuff, maybe we could do a knowledge transfer call or something?

@j0sh j0sh merged commit 79e6dcf into master Jan 18, 2025
2 of 3 checks passed
@j0sh j0sh deleted the ja/ffmpeg-h364-parser branch January 18, 2025 01:43
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.

3 participants