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

Migrate video settings to realm and remove video settings service. #5285

Merged
merged 13 commits into from
Apr 10, 2025

Conversation

michelinewu
Copy link
Contributor

No description provided.

@michelinewu michelinewu added the needs QA requires QA testing on the branch before merging label Jan 21, 2025
Copy link

bundlemon bot commented Jan 21, 2025

BundleMon

Files updated (1)
Status Path Size Limits
renderer.(hash).js
7.07MB (-2.37KB -0.03%) -
Unchanged files (3)
Status Path Size Limits
vendors~renderer.(hash).js
4.67MB -
updater.js
115.29KB -
guest-api.js
40.19KB -

Total files change -2.37KB -0.02%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@michelinewu michelinewu marked this pull request as ready for review February 6, 2025 14:09
@michelinewu michelinewu removed the needs QA requires QA testing on the branch before merging label Feb 6, 2025
@@ -23,6 +24,9 @@ export default function DisplaySelector(p: IDisplaySelectorProps) {
} = useGoLiveSettings();

const setting = p.platform ? platforms[p.platform] : customDestinations[p.index];
const label = p.platform
? platformLabels(p.platform)
: (setting as ICustomStreamDestination).name;
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like we aren't referencing p.label anymore? should we just get rid of that prop?


const videoSettings = useRealmObject(Services.VideoService.state);
const dualOutputMode = DualOutputService.views.dualOutputMode;
const cantEditFields = StreamingService.views.isStreaming || StreamingService.views.isRecording;
Copy link
Contributor

Choose a reason for hiding this comment

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

i dont think these are reactive but also i dont think they'd change while this window is open so its probably ok

await this.gameOverlayService.destroy();
this.videoService.shutdown();
Copy link
Contributor

Choose a reason for hiding this comment

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

curious about this change but not a blocker

this.videoService.setBaseResolution(this.data.baseResolutions);
this.streamingService.setSelectiveRecording(!!this.data.selectiveRecording);
this.streamingService.setDualOutputMode(this.data.dualOutputMode);
this.videoService.setBaseResolution(this.data.baseResolutions);
Copy link
Contributor

Choose a reason for hiding this comment

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

love to see the simplification here :D


return videoSettings;
} catch (e: unknown) {
console.warn('Error fetching video settings from video factory', e);
Copy link
Contributor

Choose a reason for hiding this comment

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

not a big deal but i think this error was copypasted from the above one

// const base = `${settings.baseWidth}x${settings.baseHeight}`;
// const output = `${settings.outputWidth}x${settings.outputHeight}`;
// }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this function is no longer used?

// const output = `${this.state.horizontal.video.outputWidth}x${this.state.horizontal.video.outputHeight}`;
// this.settingsService.setSettingValue('Video', 'Output', output);
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

@gettinToasty gettinToasty merged commit cf49fcc into master Apr 10, 2025
8 of 11 checks passed
@gettinToasty gettinToasty deleted the mw_migrate_video_realm branch April 10, 2025 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants