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

Fix capitalization in video profiles causing invalid transcodes #3933

Merged
merged 1 commit into from
Aug 25, 2024

Conversation

nielsvanvelzen
Copy link
Member

After merging #3932 some videos stopped playing for me with this error:

[hevc_nvenc @ 0000023043dc9f40] [Eval @ 00000039cbdfe4b0] Undefined constant or missing '(' in 'Main'
[hevc_nvenc @ 0000023043dc9f40] Unable to parse option value "Main"
[hevc_nvenc @ 0000023043dc9f40] Error setting option profile to value Main.
[vost#0:0/hevc_nvenc @ 000002303caf3700] Error initializing output stream: Error while opening encoder for output stream #0:0 - maybe incorrect parameters such as bit_rate, rate, width or height
[libfdk_aac @ 000002303cae4500] 2 frames left in the queue on closing
Conversion failed!

Turns out, our server doesn't sanitize shit and de nvenc encoder expects fully lowercase profiles. The server itself compares those strings everywhere ignoring casing though. Ideally this must be fixed serverside too.

Changes

  • Fix capitalization in video profiles causing invalid transcodes

Issues

@nielsvanvelzen nielsvanvelzen added bug Something isn't working backportable Change may be backported to a point release (remove label once cherrypicked) labels Aug 25, 2024
@nielsvanvelzen nielsvanvelzen added this to the v0.18.0 milestone Aug 25, 2024
Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

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

Wack

@nielsvanvelzen nielsvanvelzen merged commit b116db0 into jellyfin:master Aug 25, 2024
6 checks passed
@nielsvanvelzen nielsvanvelzen deleted the fix-nvenc branch August 25, 2024 20:28
@MichaelRUSF
Copy link
Contributor

I had checked what the server was taking, and saw it was ignoring case. And the profiles were already a mixture up of lower and uppercase entries. All being the same I decided that uppercase looked better, so I went with that. Go figure.

@nielsvanvelzen
Copy link
Member Author

Yeah I also wouldn't expect it to cause problems, but when it did I checked the other clients and all send the profiles in lowercase.

@nielsvanvelzen nielsvanvelzen modified the milestones: v0.18.0, v0.17.5 Sep 8, 2024
@nielsvanvelzen nielsvanvelzen removed the backportable Change may be backported to a point release (remove label once cherrypicked) label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants