fix(youtube): extra outputs can persist when disabled (#5395)

* fix(youtube): extra outputs can persist when disabled

Refactor all the handling of `extraOutputs` to live mostly in the
Dual Output service as single source of truth.

Should fix:

* Multiple streams going live when disabling YouTube while it
was previously set to "Both" outputs.
* Double horizontal/vertical validation in Go Live window.
* Display should reset back to horizontal when disabling a platform.

* fix(youtube): visual bug when display is not set

* fix(diag): extra output platforms

Author: blackxored

app/components-react/shared/DisplaySelector.tsx

@@ -4,6 +4,8 @@ import { RadioInput } from './inputs';
 import { TDisplayType } from 'services/settings-v2';
 import { TPlatform } from 'services/platforms';
 import { useGoLiveSettings } from 'components-react/windows/go-live/useGoLiveSettings';
+import { useVuex } from 'components-react/hooks';
+import { Services } from 'components-react/service-provider';
 
 interface IDisplaySelectorProps {
   title: string;
@@ -21,14 +23,21 @@ export default function DisplaySelector(p: IDisplaySelectorProps) {
     updatePlatform,
     isPrime,
     enabledPlatforms,
+    updateShouldUseExtraOutput,
   } = useGoLiveSettings();
 
+  // TODO: find a way to integrate into goLiveSettings that's reactive (currently not working that way)
+  const { hasExtraOutput } = useVuex(() => ({
+    hasExtraOutput: Services.DualOutputService.views.hasExtraOutput(p.platform!),
+  }));
+
   const setting = p.platform ? platforms[p.platform] : customDestinations[p.index];
 
   // If the user has Ultra, add extra output for YT, if not, check that we only have
   // a single platform enabled, hopefully YouTube.
   // Might need better validation.
-  const hasExtraOutputs = p.platform === 'youtube' && (isPrime || enabledPlatforms.length === 1);
+  const supportsExtraOutputs =
+    p.platform === 'youtube' && (isPrime || enabledPlatforms.length === 1);
 
   const displays = [
     {
@@ -41,7 +50,7 @@ export default function DisplaySelector(p: IDisplaySelectorProps) {
     },
   ];
 
-  if (hasExtraOutputs) {
+  if (supportsExtraOutputs) {
     // TODO: TS doesn't infer types on filter(id) so we're mutating array here
     displays.push({
       label: $t('Both'),
@@ -54,18 +63,20 @@ export default function DisplaySelector(p: IDisplaySelectorProps) {
     if (p.platform) {
       const display: TDisplayType =
         // Use horizontal display, vertical stream will be created separately
-        hasExtraOutputs && val === 'both' ? 'horizontal' : (val as TDisplayType);
-      updatePlatform(p.platform, { display, hasExtraOutputs: val === 'both' });
+        supportsExtraOutputs && val === 'both' ? 'horizontal' : (val as TDisplayType);
+      updatePlatform(p.platform, { display });
+
+      // Add or remove the platform from the Dual Output's extra output platforms list
+      updateShouldUseExtraOutput(p.platform, val);
     } else {
       updateCustomDestinationDisplay(p.index, val as TDisplayType);
     }
   };
 
   // TODO: Fake accessor, improve, if nothing else, fix type
-  const value =
-    setting?.display === 'horizontal' && (setting as any)?.hasExtraOutputs
-      ? 'both'
-      : setting?.display;
+  // display can be undefined on first window load
+  const isDefaultDisplay = setting?.display === 'horizontal' || setting?.display === undefined;
+  const value = isDefaultDisplay && hasExtraOutput ? 'both' : setting?.display;
 
   return (
     <RadioInput

app/components-react/windows/go-live/useGoLiveSettings.ts

@@ -62,16 +62,44 @@ class GoLiveSettingsState extends StreamInfoView<IGoLiveSettingsState> {
     Object.assign(this.state, { ...newSettings, platforms, customDestinations });
   }
   /**
-   * Update settings for a specific platforms
+   * Update settings for a specific platform
    */
   updatePlatform(platform: TPlatform, patch: Partial<IGoLiveSettings['platforms'][TPlatform]>) {
+    // TODO: find or create an observer for platform enabling/disabling behavior
+    const isDisablingPlatform =
+      Object.prototype.hasOwnProperty.call(patch, 'enabled') && patch?.enabled === false;
+
+    const hasExtraOutputs = Services.DualOutputService.views.hasExtraOutput(platform);
+
     const updated = {
       platforms: {
         ...this.state.platforms,
-        [platform]: { ...this.state.platforms[platform], ...patch },
+        [platform]: {
+          ...this.state.platforms[platform],
+          ...this.updateDisplayIfNeeded(patch, isDisablingPlatform, hasExtraOutputs),
+        },
       },
     };
     this.updateSettings(updated);
+
+    /*
+     * Reset display and extra outputs when disabling a platform, go live checks aren't enough.
+     * When disabling a platform, the extra output state remains true since its display
+     * `onChange` selector isn't triggered.
+     * Coupled with some bugs we've seen with go live settings persistence, this
+     * is the most practical place we've found to handle.
+     */
+    if (isDisablingPlatform) {
+      Services.DualOutputService.actions.removeExtraOutputPlatform(platform);
+    }
+  }
+
+  private updateDisplayIfNeeded(
+    patch: Partial<IGoLiveSettings['platforms'][TPlatform]>,
+    isDisablingPlatform: boolean,
+    hasExtraOutputs: boolean,
+  ) {
+    return isDisablingPlatform && hasExtraOutputs ? { ...patch, display: 'horizontal' } : patch;
   }
 
   switchPlatforms(enabledPlatforms: TPlatform[]) {
@@ -398,6 +426,24 @@ export class GoLiveSettingsModule {
   get recommendedColorSpaceWarnings() {
     return Services.SettingsService.views.recommendedColorSpaceWarnings;
   }
+
+  /**
+   * Add or remove a platform from Dual Output's extra output list
+   * according to display.
+   * If display is set to `both` it would add it, otherwise would remove it
+   * from the list if present.
+   */
+  updateShouldUseExtraOutput(platform: TPlatform, display: TDisplayType | 'both') {
+    if (display === 'both') {
+      Services.DualOutputService.actions.return.addExtraOutputPlatform(platform);
+    } else {
+      Services.DualOutputService.actions.return.removeExtraOutputPlatform(platform);
+    }
+  }
+
+  hasExtraOutput(platform: TPlatform) {
+    return Services.DualOutputService.views.hasExtraOutput(platform);
+  }
 }
 
 export function useGoLiveSettings() {

app/services/diagnostics.ts

@@ -1062,9 +1062,7 @@ export class DiagnosticsService extends PersistentStatefulService<IDiagnosticsSe
      * don't feel too happy about hacking it
      */
     const streamingPlatforms = (this.streamingService as any)?.views?.settings?.platforms || [];
-    const platformsUsingExtraOutputs = Object.keys(streamingPlatforms).filter(p => {
-      return streamingPlatforms[p].hasExtraOutputs;
-    });
+    const platformsUsingExtraOutputs = this.dualOutputService.views.extraOutputPlatforms;
 
     return new Section('Dual Output', {
       'Dual Output Active': this.dualOutputService.views.dualOutputMode,

app/services/dual-output/dual-output.ts

@@ -38,6 +38,8 @@ interface IDualOutputServiceState {
   dualOutputMode: boolean;
   videoSettings: IDisplayVideoSettings;
   isLoading: boolean;
+  // TODO: use Set
+  extraOutputPlatforms: TPlatform[];
 }
 
 enum EOutputDisplayType {
@@ -278,6 +280,27 @@ class DualOutputViews extends ViewHandler<IDualOutputServiceState> {
     const nodeMap = sceneId ? this.sceneNodeMaps[sceneId] : this.activeSceneNodeMap;
     return !!nodeMap && Object.keys(nodeMap).length > 0;
   }
+
+  /**
+   * List of platforms that use extra outputs via the new streaming API
+   */
+  get extraOutputPlatforms() {
+    return this.state.extraOutputPlatforms;
+  }
+
+  /**
+   * Check if a given platform is using extra outputs with the new streaming API
+   */
+  hasExtraOutput(platform: TPlatform) {
+    return this.state.extraOutputPlatforms.includes(platform);
+  }
+
+  /**
+   * Check if there are any platforms using extra outputs
+   */
+  get hasExtraOutputs() {
+    return !!this.state.extraOutputPlatforms.length;
+  }
 }
 
 @InitAfter('ScenesService')
@@ -303,6 +326,8 @@ export class DualOutputService extends PersistentStatefulService<IDualOutputServ
       },
     },
     isLoading: false,
+    // TODO: I would like to use `Set` but seem to be having issues with it
+    extraOutputPlatforms: [] as TPlatform[],
   };
 
   sceneNodeHandled = new Subject<number>();
@@ -820,6 +845,22 @@ export class DualOutputService extends PersistentStatefulService<IDualOutputServ
     this.SET_IS_LOADING(status);
   }
 
+  /**
+   * Add a platform to the list of platforms that use extra outputs
+   * (in addition to horizontal/vertical).
+   */
+  addExtraOutputPlatform(platform: TPlatform) {
+    this.ADD_EXTRA_OUTPUT_PLATFORM(platform);
+  }
+
+  /**
+   * Remove a platform from the list of platforms that use extra outputs
+   * (in addition to horizontal/vertical).
+   */
+  removeExtraOutputPlatform(platform: TPlatform) {
+    this.REMOVE_EXTRA_OUTPUT_PLATFORM(platform);
+  }
+
   @mutation()
   private SET_SHOW_DUAL_OUTPUT(status?: boolean) {
     this.state = {
@@ -853,4 +894,20 @@ export class DualOutputService extends PersistentStatefulService<IDualOutputServ
   private SET_IS_LOADING(status: boolean) {
     this.state = { ...this.state, isLoading: status };
   }
+
+  @mutation()
+  private ADD_EXTRA_OUTPUT_PLATFORM(platform: TPlatform) {
+    // TODO: this is more complex that it needs to be without using Sets
+    if (!this.state.extraOutputPlatforms.includes(platform)) {
+      this.state.extraOutputPlatforms = [...this.state.extraOutputPlatforms, platform];
+    }
+  }
+
+  @mutation()
+  private REMOVE_EXTRA_OUTPUT_PLATFORM(platform: TPlatform) {
+    // TODO: this is more complex that it needs to be without using Sets
+    if (this.state.extraOutputPlatforms.includes(platform)) {
+      this.state.extraOutputPlatforms = this.state.extraOutputPlatforms.filter(p => p !== platform);
+    }
+  }
 }

app/services/platforms/youtube.ts

@@ -45,8 +45,6 @@ export interface IYoutubeStartStreamOptions extends IExtraBroadcastSettings {
   privacyStatus?: 'private' | 'public' | 'unlisted';
   scheduledStartTime?: number;
   mode?: TOutputOrientation;
-  /** Use extra output to stream a vertical context to a separate broadcast */
-  hasExtraOutputs?: boolean;
 }
 
 /**

app/services/settings/streaming/stream-settings.ts

@@ -146,7 +146,7 @@ export class StreamSettingsService extends PersistentStatefulService<IStreamSett
         if (
           this.streamingService.views.enabledPlatforms.length > 1 &&
           ytSettings?.enabled &&
-          ytSettings.hasExtraOutputs
+          this.dualOutputService.views.hasExtraOutput('youtube')
         ) {
           return 'StreamSecond';
         }

app/services/streaming/streaming-view.ts

@@ -244,8 +244,7 @@ export class StreamInfoView<T extends Object> extends ViewHandler<T> {
     const platforms = settings?.platforms || this.settings.platforms;
     // If any platform is configured as "Both" for outputs we technically should satisfy
     // this requirement and ignore the warning
-    // TODO: fix types
-    if (Object.values(platforms).some(platform => (platform as any).hasExtraOutputs)) {
+    if (this.dualOutputView.hasExtraOutputs) {
       return true;
     }
 

app/services/streaming/streaming.ts

@@ -435,10 +435,9 @@ export class StreamingService
       if (
         this.views.isDualOutputMode &&
         this.views.enabledPlatforms.includes('youtube') &&
-        this.views.getPlatformSettings('youtube')?.hasExtraOutputs &&
+        this.dualOutputService.views.hasExtraOutput('youtube') &&
         /*
          * Super safe so we don't ever bypass validation due to the toggles
-         * or `hasExtraOutputs` not being reset.
          * TODO: might not cover free TikTok, but probably by design
          */
         (this.userService.views.isPrime || this.views.enabledPlatforms.length === 1)
@@ -697,7 +696,10 @@ export class StreamingService
      * it for tracking, in the future, we'd rather track extraOutputs from
      * StreamingService themselves
      */
-    if (settings.platforms.youtube?.enabled && settings.platforms.youtube?.hasExtraOutputs) {
+    if (
+      settings.platforms.youtube?.enabled &&
+      this.dualOutputService.views.hasExtraOutput('youtube')
+    ) {
       this.usageStatisticsService.recordFeatureUsage('StreamToYouTubeBothOutputs');
     }
   }
@@ -1000,10 +1002,11 @@ export class StreamingService
 
       NodeObs.OBS_service_setVideoInfo(horizontalContext, 'horizontal');
       const ytSettings = this.views.getPlatformSettings('youtube');
+      // TODO: this can probably be more generic now
       if (
         this.views.enabledPlatforms.length > 1 &&
         ytSettings?.enabled &&
-        ytSettings.hasExtraOutputs
+        this.dualOutputService.views.hasExtraOutput('youtube')
       ) {
         NodeObs.OBS_service_setVideoInfo(horizontalContext, 'vertical');
       } else {