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.

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

UI: Add support for additional canvases to multitrack video output #10635

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

palana
Copy link
Contributor

@palana palana commented May 3, 2024

Description

Adds frontend API calls to allow plugins to register additional canvases/video outputs that will participate in multitrack video configuration generation and multitrack video streams

Motivation and Context

Currently additional views/canvases have to use separate rtmp connections or similar to be streamed, where the multitrack video output allows both multiple renditions of the same view and also different views to be streamed

How Has This Been Tested?

Twitch Enhanced Broadcasting beta

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.

@palana palana force-pushed the ruwen/vertical-canvas-integration branch 2 times, most recently from ce1b263 to c03b4fb Compare May 3, 2024 16:22
@RytoEX RytoEX self-assigned this May 3, 2024
@WizardCM WizardCM added the New Feature New feature or plugin label May 18, 2024
@palana palana force-pushed the ruwen/vertical-canvas-integration branch 2 times, most recently from 6dcd68c to d404b1b Compare June 5, 2024 12:55
@RytoEX RytoEX added this to the OBS Studio 31 milestone Jun 19, 2024
@palana palana force-pushed the ruwen/vertical-canvas-integration branch 2 times, most recently from 420b8d7 to 60f2951 Compare July 16, 2024 13:32
@RytoEX
Copy link
Member

RytoEX commented Jul 29, 2024

#10633 was merged some time ago. This will need to be rebased and get CI to pass.

@palana palana force-pushed the ruwen/vertical-canvas-integration branch from 60f2951 to 7c05363 Compare August 1, 2024 11:11
@palana
Copy link
Contributor Author

palana commented Aug 1, 2024

#10633 was merged some time ago. This will need to be rebased and get CI to pass.

@RytoEX: rebased and updated the description

@RytoEX
Copy link
Member

RytoEX commented Aug 6, 2024

@palana It looks like the build failures are still present.

@palana palana force-pushed the ruwen/vertical-canvas-integration branch from 7c05363 to 8dffc0a Compare August 7, 2024 14:59
@palana
Copy link
Contributor Author

palana commented Aug 7, 2024

@palana It looks like the build failures are still present.

@RytoEX: oh, yea, missed the bit about making CI pass; latest push should make it pass 😅

@RytoEX
Copy link
Member

RytoEX commented Aug 7, 2024

@tt2468 @derrod Any feedback on this?

@RytoEX RytoEX requested review from derrod and tt2468 August 8, 2024 20:15
docs/sphinx/reference-frontend-api.rst Outdated Show resolved Hide resolved
UI/window-basic-main.cpp Outdated Show resolved Hide resolved
@@ -11053,6 +11053,33 @@ QColor OBSBasic::GetSelectionColor() const
}
}

const std::vector<MultitrackVideoViewInfo> &
OBSBasic::GetAdditionalMultitrackVideoViews()
Copy link
Member

Choose a reason for hiding this comment

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

This does nothing but return a copy of the value. Probably fine to fetch the value directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure I follow, this returns a const reference, rather than a copy (i.e. you're allowed to read entries, but not modify them)

Copy link
Member

Choose a reason for hiding this comment

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

@tt2468 Could you clarify the concern here?

blog(LOG_WARNING,
"Failed to get obs_video_info while creating encoder %zu",
encoder_index);
throw MultitrackVideoError::warning(
QTStr("FailedToStartStream.FailedToGetOBSVideoInfo")
.arg(name_buffer->array, encoder_type));
}
obs_video_info *ovi = &ovi_storage;

const char *view_name = nullptr;
Copy link
Member

Choose a reason for hiding this comment

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

This is probably safer as std::string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

safer in what way? encoder_config can't be modified in here (unless someone adds a const cast?), so the pointer (once assigned) stays valid for this scope at least (and it's not used outside of this scope)

Copy link
Member

Choose a reason for hiding this comment

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

@tt2468 Could you clarify the concern here?

docs/sphinx/reference-frontend-api.rst Outdated Show resolved Hide resolved
@palana palana force-pushed the ruwen/vertical-canvas-integration branch from 3d41885 to dabc2f6 Compare September 17, 2024 12:29
@palana palana force-pushed the ruwen/vertical-canvas-integration branch from dabc2f6 to 212f177 Compare September 17, 2024 12:30
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.

6 participants