decklink: Output BGRA only when supported by DDRBoxman · Pull Request #10392 · obsproject/obs-studio

DDRBoxman · 2024-03-16T23:31:13Z

Description

Currently if you select a mode that doesn't support RGBA on Decklink output the image is corrupted.

Fixes #10380

Motivation and Context

#10380

How Has This Been Tested?

Sent video to the Ultra Studio 4k mini with multiple video modes

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.

RytoEX

Commit message nits:

doesnt -> doesn't

If this fixes #10380, use "Fixes #10380" in the PR description to automatically link them.

plugins/decklink/decklink-device-instance.cpp

plugins/decklink/decklink-output.cpp

RytoEX

Fix the commit message so that the subject is not wrapped on GitHub (70 characters or less, I think).

If you need to elaborate, use the commit message body.

Lain-B

Is it intentional to always lock out BGRA/RGBA if it's not HDR? Or am I misunderstanding the PR?

DDRBoxman · 2024-03-24T03:30:37Z

I tweaked the logic in decklink-output.cpp to actually match correctly on HDR and keyer. It worked on my device since it supports both but this should fix compatibility on more devices.

norihiro · 2024-03-25T05:10:40Z

The commit message has a too long description in a line, 107 characters, limit 72.

Not all devices support the BGRA pixel format in all video modes. Make sure we fallback to YUV when needed.

RytoEX

@norihiro is right. Wrap the commit message body at 72 characters.
Also, nits:

fallback -> fall back
Only output BGRA when supported -> Output BGRA only when supported

On "fallback", the former is a noun. The latter is a verb.

If nits are addressed and @Lain-B or @jpark37 are happy with the PR itself, this should be good afterward.

RytoEX

Still nit on commit message subject to place verb first:

Only output BGRA when supported -> Output BGRA only when supported

decklink: Output BGRA only when supported

Not all devices support the BGRA pixel format in all video modes. Make sure we fall back to YUV when needed.

RytoEX

Nits fixed. Will await other reviews.

Lain-B · 2024-03-29T16:40:19Z

plugins/decklink/decklink-output.cpp

+
 		struct video_scale_info to = {};
-		to.format = VIDEO_FORMAT_BGRA;
+		to.format = (modeSupportsBGRA || supportsHDR)


Noticed that this code is "if supports BGR or HDR, use BGRA" -- wouldn't this just be "if supports BGR, use BGR"?

It's possible I'm misunderstanding this expression though.

yeah that probably makes more sense, I think I got confused since HDR was setup for bgra

Hmm HDR is using bmdFormat10BitRGBXLE on the device. We should also probably check if that is supported for the current format too. And then figure out like YUV HDR?

Probably set this back to always RGB and then do conversions on the GPU here for when we need YUV

obs-studio/UI/frontend-plugins/decklink-output-ui/decklink-ui-main.cpp

Line 400 in 004253f

gs_effect_t *const effect = obs_get_base_effect(OBS_EFFECT_DEFAULT);

RytoEX · 2024-04-05T00:29:13Z

Setting back to draft per off-thread conversation that the open review thread requires writing new shaders to resolve.

RytoEX · 2025-01-16T16:52:11Z

Setting back to draft per off-thread conversation that the open review thread requires writing new shaders to resolve.

I presume this is still the case?

DDRBoxman force-pushed the decklink_pixelformat branch from 7e329d2 to cd0ea9c Compare March 16, 2024 23:33

WizardCM added the Bug Fix Non-breaking change which fixes an issue label Mar 16, 2024

RytoEX requested changes Mar 16, 2024

View reviewed changes

DDRBoxman force-pushed the decklink_pixelformat branch from cd0ea9c to 1ee36ad Compare March 17, 2024 16:09

DDRBoxman changed the title ~~decklink: Use the YUV pixel format if the device doesnt support keying~~ decklink: Use the YUV pixel format if the device doesn't support keying Mar 17, 2024

DDRBoxman force-pushed the decklink_pixelformat branch 4 times, most recently from dbbdfa1 to 5a2e609 Compare March 17, 2024 17:06

DDRBoxman requested a review from RytoEX March 17, 2024 17:16

RytoEX added this to the OBS Studio (Next Version) milestone Mar 17, 2024

RytoEX self-assigned this Mar 17, 2024

RytoEX requested changes Mar 17, 2024

View reviewed changes

plugins/decklink/decklink-device-instance.cpp Outdated Show resolved Hide resolved

plugins/decklink/decklink-output.cpp Outdated Show resolved Hide resolved

DDRBoxman force-pushed the decklink_pixelformat branch from 5a2e609 to 43d9742 Compare March 17, 2024 18:00

DDRBoxman requested a review from RytoEX March 17, 2024 18:00

DDRBoxman force-pushed the decklink_pixelformat branch 2 times, most recently from 7f04e11 to 4164313 Compare March 17, 2024 21:03

jpark37 reviewed Mar 21, 2024

View reviewed changes

plugins/decklink/decklink-output.cpp Show resolved Hide resolved

DDRBoxman changed the title ~~decklink: Use the YUV pixel format if the device doesn't support keying~~ decklink: Use the YUV pixel format if the device doesn't support RBGA for the video mode Mar 21, 2024

DDRBoxman force-pushed the decklink_pixelformat branch 2 times, most recently from 4c6a2f4 to 927204e Compare March 21, 2024 17:54

RytoEX requested a review from jpark37 March 23, 2024 23:04

RytoEX requested changes Mar 23, 2024

View reviewed changes

Lain-B reviewed Mar 23, 2024

View reviewed changes

DDRBoxman force-pushed the decklink_pixelformat branch from 927204e to 17e0db8 Compare March 24, 2024 03:28

DDRBoxman changed the title ~~decklink: Use the YUV pixel format if the device doesn't support RBGA for the video mode~~ decklink: Only output BGRA when supported Mar 24, 2024

DDRBoxman force-pushed the decklink_pixelformat branch from 17e0db8 to 3372f6b Compare March 24, 2024 03:29

DDRBoxman requested review from Lain-B and RytoEX March 24, 2024 03:31

RytoEX requested changes Mar 27, 2024

View reviewed changes

DDRBoxman force-pushed the decklink_pixelformat branch from 3372f6b to e502eb9 Compare March 27, 2024 22:12

DDRBoxman requested a review from RytoEX March 27, 2024 22:13

RytoEX requested changes Mar 27, 2024

View reviewed changes

decklink: Output BGRA only when supported

ebd5aa9

Not all devices support the BGRA pixel format in all video modes. Make sure we fall back to YUV when needed.

DDRBoxman force-pushed the decklink_pixelformat branch from e502eb9 to ebd5aa9 Compare March 28, 2024 00:18

DDRBoxman requested a review from RytoEX March 28, 2024 00:18

RytoEX approved these changes Mar 28, 2024

View reviewed changes

DDRBoxman changed the title ~~decklink: Only output BGRA when supported~~ decklink: Output BGRA only when supported Mar 28, 2024

jpark37 approved these changes Mar 29, 2024

View reviewed changes

Lain-B reviewed Mar 29, 2024

View reviewed changes

RytoEX marked this pull request as draft April 5, 2024 00:28

RytoEX modified the milestones: OBS Studio (Next Version), OBS Studio (Next Major Version) Jun 7, 2024

Uh oh!

Conversation

DDRBoxman commented Mar 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

RytoEX left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RytoEX left a comment

Choose a reason for hiding this comment

Uh oh!

Lain-B left a comment

Choose a reason for hiding this comment

Uh oh!

DDRBoxman commented Mar 24, 2024

Uh oh!

norihiro commented Mar 25, 2024

Uh oh!

RytoEX left a comment

Choose a reason for hiding this comment

Uh oh!

RytoEX left a comment

Choose a reason for hiding this comment

Uh oh!

RytoEX left a comment

Choose a reason for hiding this comment

Uh oh!

Lain-B Mar 29, 2024

Choose a reason for hiding this comment

Uh oh!

Lain-B Mar 29, 2024

Choose a reason for hiding this comment

Uh oh!

DDRBoxman Mar 29, 2024

Choose a reason for hiding this comment

Uh oh!

DDRBoxman Mar 29, 2024

Choose a reason for hiding this comment

Uh oh!

DDRBoxman Mar 29, 2024

Choose a reason for hiding this comment

Uh oh!

RytoEX commented Apr 5, 2024

Uh oh!

RytoEX commented Jan 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DDRBoxman commented Mar 16, 2024 •

edited

Loading