Skip to content

Conversation

@Conan-Kudo
Copy link
Contributor

@Conan-Kudo Conan-Kudo commented Mar 25, 2023

Description

This allows users to leverage the OpenH264 codec from Cisco to encode H.264 video content. It is significantly reduced in capability from alternatives, but it does the job.

This also provides a framework for adding support for other H.264 software codecs provided through FFmpeg.

Motivation and Context

A number of distributions use OpenH264 distributed by Cisco to provide a fully licensed H.264 implementation (in particular, both Fedora Linux and openSUSE Linux do). However, OBS Studio cannot currently use it.

This change allows OBS Studio to use OpenH264 through FFmpeg if it's available.

How Has This Been Tested?

  • Tested recording a screencast using OpenH264 + AAC with both Simple and Advanced settings modes
  • Tested streaming a screencast using OpenH264 + AAC with both Simple and Advanced settings modes

This was tested on Fedora Linux 38 for x86_64 with ffmpeg 6.0.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@WizardCM WizardCM added the New Feature New feature or plugin label Mar 25, 2023
@RytoEX
Copy link
Member

RytoEX commented Mar 25, 2023

Does this require #8015?

@Conan-Kudo
Copy link
Contributor Author

Does this require #8015?

Not yet. This is only the wiring for the ffmpeg plugin, which I think means it only shows up in the "advanced view" right now? I literally just banged this out and am taking a bit of a break before continuing to iterate on it. I've got local builds going so I can test the functionality...

@Conan-Kudo Conan-Kudo force-pushed the obs-ffmpeg-h264 branch 7 times, most recently from 3182c09 to 64c44cd Compare March 26, 2023 03:40
@Conan-Kudo
Copy link
Contributor Author

Conan-Kudo commented Mar 26, 2023

@RytoEX That said, having #8015 land first would make it tremendously easier for the UI wiring I need to do...

@Conan-Kudo Conan-Kudo force-pushed the obs-ffmpeg-h264 branch 4 times, most recently from 7afe715 to cc43f38 Compare March 26, 2023 12:42
@Conan-Kudo Conan-Kudo changed the title obs-ffmpeg: Add initial support for the OpenH264 H.264 software codec Add initial support for the OpenH264 H.264 software codec Mar 26, 2023
@Conan-Kudo
Copy link
Contributor Author

I've pulled in #8015 into this pull request to simplify things for me and to show the full context of this change.

@Conan-Kudo Conan-Kudo force-pushed the obs-ffmpeg-h264 branch 9 times, most recently from 6164cb4 to de4b18d Compare March 26, 2023 19:40
@Conan-Kudo Conan-Kudo marked this pull request as ready for review March 26, 2023 20:08
@Conan-Kudo
Copy link
Contributor Author

I've now tested that this works, thanks to @Gawdl3y's help! 🎉

@DemiMarie
Copy link

But distribution-specific fixes need to be implemented/patched by distributors. Adding a global (as in: affecting all platforms) fallback on OpenH264 if the actual fallback x264 (which is commonly shipped by OBS Studio itself or expected to be available on the platform) is not a solution IMO.

On desktop Linux, OpenH264 is the worst-case fallback. Software patent restrictions mean that it is the only H.264 codec that all distros can redistribute.

@Conan-Kudo Conan-Kudo force-pushed the obs-ffmpeg-h264 branch 5 times, most recently from a4251ab to 66974a1 Compare October 5, 2024 21:13
Copy link
Collaborator

@tytan652 tytan652 left a comment

Choose a reason for hiding this comment

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

Translation nitpik that covers both PR.

@Conan-Kudo Conan-Kudo marked this pull request as ready for review July 1, 2025 08:55
@Conan-Kudo
Copy link
Contributor Author

This pull request has been rebased on top of current master and has the remaining commits to merge (frontend commits).

@Conan-Kudo
Copy link
Contributor Author

While this doesn't have a direct dependency on @AsahiLina's #12326, it does benefit from it.

@Conan-Kudo Conan-Kudo requested review from derrod and tytan652 July 3, 2025 13:50
@Conan-Kudo Conan-Kudo force-pushed the obs-ffmpeg-h264 branch 2 times, most recently from e962f04 to 4f52ec5 Compare July 8, 2025 03:18
@Lain-B
Copy link
Collaborator

Lain-B commented Jul 12, 2025

Just briefly looking at it, it seems pretty trivial now that everything else has been merged. Anyone else want to give further feedback?

@Fenrirthviti
Copy link
Member

Fenrirthviti commented Jul 15, 2025

For the fallback part of this, I would prefer if we just removed the entire fallback system in general in favor of a better error reporting that the encoder is missing. I don't want to make our already tenuous and fragile fallback that... really doesn't make sense anymore, more complex.

@Fenrirthviti
Copy link
Member

Also, can you please explain why we're intrinsically linking OpenH264's existence to x264 in the first place?

I would prefer that if the encoder is available, it just shows up. If it's not, it doesn't. I'm not sure what value there is in NOT displaying OpenH264 if x264 is available instead of just showing them both?

@Conan-Kudo
Copy link
Contributor Author

I was asked last time around to hide it if x264 is available. If we don't need that anymore, I can drop the commit that does that.

@Fenrirthviti
Copy link
Member

Thanks, found the comment in one of the old reviews. That is not something we'd prefer, it can be removed, yes.

OpenH264 exists as the codec of last resort, so it is implemented
such that it is only used as the software codec if x264 is not
available.
@Conan-Kudo
Copy link
Contributor Author

I dropped it and rebased onto current master.

@Conan-Kudo
Copy link
Contributor Author

For the fallback part of this, I would prefer if we just removed the entire fallback system in general in favor of a better error reporting that the encoder is missing. I don't want to make our already tenuous and fragile fallback that... really doesn't make sense anymore, more complex.

I would prefer we do this separately. I agree with you that the fallback logic is crufty and fragile, but purging it is currently beyond my capabilities. That said, @AsahiLina has taken the first steps to allow eliminating it with #12326.

@Fenrirthviti
Copy link
Member

Fenrirthviti commented Jul 16, 2025

I agree that it's a separate change, yes, and outside scope of what this PR is attempting to accomplish.

We're doing some internal checking of the relevant code paths to determine if this PR is a change we're willing to accept short term, and if it is not, have a reasonable alternative proposal.

@PatTheMav
Copy link
Member

Given that the change seems to be mostly held up by the topic of how to properly handle the fallback, I did a little dive into our UI and settings code to check how deeply integrated the fallback selection actually is, and the answer is: "Very".

Without going into too much detail here, there are many pieces in the settings code that depend on a valid encoder being set as the "current" encoder at all times, and there are further parts of OBS Studio that require those parts being available as well, etc.


So my current thinking (after some deliberation) is that I still don't like openh264 becoming a de-facto global fallback if x264 isn't found, because it would only ever really be a working(!) fallback on Fedora systems.

So I'd propose that instead of this (pseudocode):

if (some encoder) {
   use some encoder
} else if (some other encoder) {
   use some other encoder
} else if (x264) {
   use x264
} else {
   use openh264
}

we'd do this:

if (some encoder) {
   use some encoder
} else if (some other encoder) {
   use some other encoder
} else {
   // Time to find a fallback
   if (x264) {
      use x264
   } else if (openh264) {
      use openh264
   } else {
      gracefully abort OBS with an error message because the app is in an unworkable state
   }
}

For one it decouples x264 from being the "de-facto" fallback encoder, but instead demotes it to a "fallback option" together with open264. Either would be fine, one of them has to be present. And the code just has to ensure that some fallback encoder is provided.

This also means that the code for simple mode output cannot just add "x264" as the very first encoder, but has to check for availability just like it has to do for all other encoders.

As far as the function of the encoder drop down is concerned, I agree that it requires a fix to allow people to change a possibly broken state, but wouldn't hoist that responsibility onto this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Feature New feature or plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants