Skip to content

Conversation

@derrod
Copy link
Member

@derrod derrod commented Feb 18, 2023

Description

Adds support for manually selecting the recording audio encoder in the UI.

Currently adds 16, 24, and 32 bit PCM as well as 24-bit ALAC and 16-bit FLAC (their respective default bit-depths in FFmpeg's order of formats).

Screenshot (note: out of date, minor changes with RFC 45, but overall still the same UX):
2023-02-18_06-19-28_xvjboa

#8498 adds the necessary UI to prevent users from selecting invalid container and audio codec combinations.

Motivation and Context

Lossless audio recording is a highly requested feature (e.g. ideas page posts 271, 1342, 1680, and probably many more) due to the additional flexibility in editing and avoidance of generational loss.

Additionally, PCM floating point audio allows for some additional flexibility when it comes to editing and fixing of audio in post, as clipping audio (i.e. >1.0f or <-1.0f) is retained and can be adjusted down to no longer be clipping. Aside from being able to make up for wrong recording levels in editing, this is also useful in situations where constructive interference resulted in the final output clipping while individually sources were fine (this currently happens invisibly due to the lack of a master mixer).

How Has This Been Tested?

Recorded some videos.

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.

@derrod derrod added the Seeking Testers Build artifacts on CI label Feb 18, 2023
@WizardCM WizardCM added the New Feature New feature or plugin label Feb 18, 2023
@derrod derrod force-pushed the more-audio-codecs branch 3 times, most recently from db95485 to 02c0219 Compare February 18, 2023 05:34
@TianQiBuTian
Copy link
Contributor

@derrod derrod force-pushed the more-audio-codecs branch 2 times, most recently from 3cac02c to 7428878 Compare February 18, 2023 20:01
@derrod
Copy link
Member Author

derrod commented Feb 18, 2023

Crash on open settings. Log: https://obsproject.com/logs/OwXxp9HwmUE0BRiz Crash log: https://obsproject.com/logs/fkiO7swIjOTcfcJs

I believe this was due to no default config entry for AudioEncoder being set, this should be fixed now.

@derrod
Copy link
Member Author

derrod commented Feb 19, 2023

One thing that is left to be done (ignoring conflicts with #8090) is a compatibility check.

Right now we have the following situation:

Containers AAC ALAC FLAC Opus PCM
FLV
MP4 1 1 2
MOV
MKV
M3U8 (HLS)
TS

The following issues are worth noting:

Edit: I've listed all valid audio, video, and container combinations with this PR here: https://github.com/derrod/obs-studio/wiki/Container-Codec-Support

Footnotes

  1. Current obs-deps FFmpeg version requires "experimental" standards compliance (-strict -2), no longer necessary in latest FFmpeg 2

  2. Support is standardised, but FFmpeg doesn't support it yet. See https://trac.ffmpeg.org/ticket/10185

@derrod derrod changed the title UI/obs-ffmpeg: Add option for recording lossless audio UI,libobs,obs-ffmpeg: Add option for recording lossless audio Feb 20, 2023
@derrod derrod marked this pull request as draft March 11, 2023 00:58
@derrod
Copy link
Member Author

derrod commented Mar 11, 2023

Drafting until RFC 45 stuff (especially #8090) is merged so it can be rebased and adjusted to those changes (if they make it into 29.1, otherwise I'd prefer if we can get this in first).

Edit: On second thought, let's leave it open in case 45-5 needs more time.

@derrod derrod force-pushed the more-audio-codecs branch from 7428878 to 661d247 Compare March 14, 2023 21:27
@derrod derrod marked this pull request as ready for review March 14, 2023 22:57
@alialkhatib
Copy link

I don't have anything to contribute except that I'm very excited about this. i've been recording audio separately to get 32-bit floating point audio from my audio interface (zoom f3), so this would simplify my recording setup pretty dramatically. thank you so much for doing the legwork on this

@derrod derrod force-pushed the more-audio-codecs branch 2 times, most recently from f87b1b1 to 799061d Compare March 15, 2023 02:13
@derrod
Copy link
Member Author

derrod commented Mar 15, 2023

Updated to split out FFmpeg and PCM32 into their own commits to make them easier to skip. I'm personally not entirely sold on keeping FLAC around. Support for it in MP4 seems bad in editing software and we already have ALAC anyway.
Perhaps one of the PCM variants could also be removed, lthough compatibility is of course great there.

Also added a compatibility error message in case a non-supported combination of codec and container is chosen:
2023-03-15_03-11-52_hEJKDK

In my testing I've found that with multiple audio tracks you can get stuck on starting the recording, but #8175 fixes that.

@TianQiBuTian
Copy link
Contributor

Can you add FLAC 24-bit format?
Because FLAC supports 24-bits, and some websites need at least 24-bits to enable the Hi-Res option.

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.

Just a couple of nitpik.

@derrod derrod force-pushed the more-audio-codecs branch from 799061d to 888fca5 Compare March 20, 2023 11:03
@derrod
Copy link
Member Author

derrod commented Mar 20, 2023

Rebased now that RFC 45 PRs have been merged, still requires #8496 and ideally should be considered with #8498 to communicate to users what is and isn't compatible.

@derrod derrod force-pushed the more-audio-codecs branch from 888fca5 to 6807009 Compare March 20, 2023 14:48
@derrod derrod changed the title UI,libobs,obs-ffmpeg: Add option for recording lossless audio libobs,obs-ffmpeg: Add option for recording lossless audio Mar 20, 2023
@derrod derrod requested review from jp9000 and notr1ch March 20, 2023 16:28
@derrod derrod requested review from pkviet and removed request for notr1ch March 20, 2023 16:32
Copy link
Member

@pkviet pkviet left a comment

Choose a reason for hiding this comment

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

LGTM, thought it would take more code.

@derrod derrod force-pushed the more-audio-codecs branch from 6807009 to 3de1773 Compare March 21, 2023 00:02
@derrod derrod requested a review from RytoEX March 22, 2023 02:50
@derrod
Copy link
Member Author

derrod commented Mar 23, 2023

As @flaeri noted this will also prevent people from (having to) use Custom FFmpeg output for these features and reduce support requests related to it. So I'm hoping we can squeeze this into 29.1 together with #8498.

derrod added 5 commits March 25, 2023 10:22
- Do not set sample rate (not required here, but can be 24/32 now)
- Only set bit_rate for lossless codecs
- Only set frame_size for codecs using a fixed one
This is useful for formats such as 32-bit float PCM which providers
greater flexibility in editing by retaining information that would
otherwise be clipped.
@derrod derrod force-pushed the more-audio-codecs branch from 3de1773 to 1a51aad Compare March 25, 2023 09:40
@jp9000 jp9000 merged commit 7a30d53 into obsproject:master Mar 25, 2023
@RytoEX RytoEX added this to the OBS Studio 29.1 milestone Mar 25, 2023
Copy link
Member

@RytoEX RytoEX left a comment

Choose a reason for hiding this comment

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

Seemed fine at a glance.

@derrod derrod deleted the more-audio-codecs branch March 26, 2023 00:15
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 Seeking Testers Build artifacts on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants