Skip to content

obs-ffmpeg: Fix muxing non-AAC codecs / Opus in MP4 #8496

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
derrod merged 2 commits into from
Mar 20, 2023
Merged

obs-ffmpeg: Fix muxing non-AAC codecs / Opus in MP4 #8496

derrod merged 2 commits into from
Mar 20, 2023

Conversation

@derrod
Copy link
Member

@derrod derrod commented Mar 20, 2023

Description

Fixes FFmpeg mux for recording Opus in MP4.

(Commits adapted from #8289 with lossless specifics removed)

Motivation and Context

Recording Opus in MP4 should not fail with FFmpeg < 6.0 and be correctly tagged in files.

How Has This Been Tested?

Tested in #8289

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

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 Bug Fix Non-breaking change which fixes an issue label Mar 20, 2023
@derrod derrod mentioned this pull request Mar 20, 2023
Merged
6 tasks
pkviet
pkviet suggested changes Mar 20, 2023
View reviewed changes
Copy link
Member

@pkviet pkviet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The commit should spell out the reasons / full context for adding the flag. You've been lazy, Rodney !
In the commit wording , could you add reference to the commits allowing to drop the compliance flag ?
edit: this was for the compliance commit

pkviet
pkviet suggested changes Mar 20, 2023
View reviewed changes
Copy link
Member

@pkviet pkviet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please put some context in the commit message.
edit: this was a comment to the other commit

derrod added 2 commits March 20, 2023 11:38
@derrod
ffmpeg-mux: Set experimental compliance for FFmpeg < 6.0
3b62445 
With 5fe417b it became possible to use
Opus in local recordings, this could potentially have the user try to
record Opus in MP4.

FLAC in MP4 was marked as stable in FFmpeg 6.0
Opus in MP4 was marked as stable in FFmpeg 4.3

For Ubuntu 20.04 we still need the latter, for 22.04 (and potentially
other Linux distributions) the former.

While FLAC is not yet implemented, we may want to do that in the near
future so for simplicity just keep it at 6.0.
@derrod
obs-ffmpeg: Pass correct audio codec to muxer subprocess
fae688b
pkviet
pkviet approved these changes Mar 20, 2023
View reviewed changes
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.

Good to merge imho

@derrod derrod changed the title Fix ffmpeg mux obs-ffmpeg: Fix muxing Opus into MP4 Mar 20, 2023
@derrod derrod changed the title obs-ffmpeg: Fix muxing Opus into MP4 obs-ffmpeg: Fix muxing non-AAC codecs / Opus in MP4 Mar 20, 2023
@tytan652 tytan652 added this to the OBS Studio 29.1 milestone Mar 20, 2023
@derrod derrod merged commit f96ae65 into obsproject:master Mar 20, 2023
@derrod derrod deleted the fix-ffmpeg-mux branch March 20, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@pkviet pkviet pkviet approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug Fix Non-breaking change which fixes an issue

Projects

None yet

Milestone

OBS Studio 29.1

Development

Successfully merging this pull request may close these issues.

3 participants

@derrod @pkviet @tytan652