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.

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

obs-outputs: Fix AV1 packet priority in RTMP #10436

Merged
merged 2 commits into from
Mar 30, 2024
Merged

obs-outputs: Fix AV1 packet priority in RTMP #10436

merged 2 commits into from
Mar 30, 2024

Conversation

derrod
Copy link
Member

@derrod derrod commented Mar 26, 2024

Description

Adds parsing of AV1 frame type to AV1 OBU parsing to set drop priority of keyframes correctly.

Somewhat janky but matches what we did in 722ca84 and should be good enough™ for our use case.

Motivation and Context

Should fix #10383

How Has This Been Tested?

Verified priority now gets set.

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 Bug Fix Non-breaking change which fixes an issue Seeking Testers Build artifacts on CI labels Mar 26, 2024
@RytoEX RytoEX mentioned this pull request Mar 26, 2024
Closed
@derrod derrod changed the title Av1 priority obs-outputs: Fix AV1 packet priority in RTMP Mar 26, 2024
@RytoEX RytoEX added this to the OBS Studio (Next Version) milestone Mar 26, 2024
@RytoEX RytoEX self-assigned this Mar 27, 2024
@derrod derrod force-pushed the av1-priority branch 4 times, most recently from 4f0f5c1 to 0250d74 Compare March 27, 2024 22:36
@derrod
Copy link
Member Author

derrod commented Mar 27, 2024

Adjusted the priorities based on feedback from people that know more than me about AV1. Should hopefully be good enough to keep us running for a while :)

RytoEX
RytoEX requested changes Mar 27, 2024
View reviewed changes
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.

Nit only, otherwise seems fine.

plugins/obs-outputs/rtmp-av1.c Outdated Show resolved Hide resolved
@norihiro
Copy link
Contributor

The prefixes of the commit titles should be obs-outputs instead of obs-output.

@RytoEX RytoEX self-requested a review March 29, 2024 17:37
RytoEX
RytoEX requested changes Mar 29, 2024
View reviewed changes
plugins/obs-outputs/rtmp-av1.c Outdated Show resolved Hide resolved
@RytoEX RytoEX self-requested a review March 29, 2024 21:29
RytoEX
RytoEX approved these changes Mar 29, 2024
View reviewed changes
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.

Looks okay to me. User tests were promising.

@Lain-B Lain-B merged commit 4e18bb6 into obsproject:master Mar 30, 2024
14 checks passed
@derrod derrod deleted the av1-priority branch March 30, 2024 23:15
@derrod derrod mentioned this pull request Jul 16, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RytoEX RytoEX RytoEX approved these changes

@Lain-B Lain-B Lain-B approved these changes

Assignees

@RytoEX RytoEX

Labels
Bug Fix Non-breaking change which fixes an issue Seeking Testers Build artifacts on CI
Projects
None yet
Milestone
OBS Studio 30.2
Development

Successfully merging this pull request may close these issues.

AV1 YouTube bitrate drop until stream restart
4 participants
@derrod @norihiro @RytoEX @Lain-B