Skip to content
This repository was archived by the owner on Jun 29, 2026. It is now read-only.

frontend: Auto-discovery of registered services - #1

Merged
emilsas merged 2 commits into
moq-dev:masterfrom
emilsas:feature/service-discovery
Jan 14, 2026
Merged

frontend: Auto-discovery of registered services#1
emilsas merged 2 commits into
moq-dev:masterfrom
emilsas:feature/service-discovery

Conversation

@emilsas

@emilsas emilsas commented Jan 14, 2026

Copy link
Copy Markdown
Collaborator

Duplicated from: obsproject#12911

@coderabbitai

coderabbitai Bot commented Jan 14, 2026

Copy link
Copy Markdown

Walkthrough

The changes introduce a new structured type ServiceItemData to represent service dropdown items with metadata including type classification, service identifier, and display name. This type replaces simpler enum-based service selection throughout the streaming settings. Three new public methods are added to OBSBasicSettings to query custom service types and locate services by predicate. The implementation in the stream settings file is refactored to populate dropdown entries with ServiceItemData objects and update stream loading and saving flows to handle the enriched service representation, enabling more detailed service type tracking and selection logic.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'frontend: Auto-discovery of registered services' accurately reflects the main objective of the changeset, which introduces new service discovery functionality and refactors service handling in OBSBasicSettings.
Description check ✅ Passed The description references the original PR #12911 from obs-studio, which is directly related to the changes implementing service auto-discovery and service handling refactoring.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
frontend/settings/OBSBasicSettings_Stream.cpp (1)

306-318: Service ID handling is memory-safe.

The serviceIdUtf8 QByteArray correctly remains in scope while service_id points to its data via constData(). This avoids dangling pointer issues.

♻️ Optional: Extract common service_id resolution logic

The same pattern appears in both SaveStream1Settings (lines 309-318) and SpawnTempService (lines 837-846). Consider extracting to a helper if this pattern is needed elsewhere:

// Helper to get service_id based on current selection
// Returns a pair of (service_id, storage) where storage keeps the string alive
std::pair<const char*, QByteArray> GetCurrentServiceId() const;

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 526c892 and 70b50de.

📒 Files selected for processing (2)
  • frontend/settings/OBSBasicSettings.hpp
  • frontend/settings/OBSBasicSettings_Stream.cpp
🔇 Additional comments (10)
frontend/settings/OBSBasicSettings.hpp (2)

40-72: Well-structured service item data type.

The ServiceItemData struct provides a clean abstraction for service dropdown items with proper type enumeration and helper methods. The Qt meta-type registration enables seamless QVariant integration.

One minor note: the default constructor initializes type to Custom but leaves serviceId and displayName as empty QStrings. This is fine for the current usage but consider whether a default-constructed ServiceItemData should be distinguishable from an explicitly constructed Custom type.


240-243: LGTM on new API declarations.

The new public methods IsCustomServiceType(), GetCustomServiceTypeId(), and FindService() with predicate support provide a flexible API for querying service items. The std::function predicate pattern enables clean, composable lookups.

frontend/settings/OBSBasicSettings_Stream.cpp (8)

34-55: LGTM on service type query methods.

The implementations consistently extract ServiceItemData from the combo box's current data and delegate to the appropriate helper method. The QVariant::value<>() call safely returns a default-constructed ServiceItemData if the conversion fails, which would have type == Type::Custom - a safe fallback.


57-74: Clean predicate-based service lookup.

The FindService implementation correctly uses canConvert<ServiceItemData>() as a safety check before attempting the conversion. The predicate pattern enables flexible, composable queries throughout the codebase. The IsWHIP() simplification to use GetCustomServiceTypeId() is a nice cleanup.


142-196: Service selection logic correctly refactored.

The three-way branching (rtmp_custom, rtmp_common, custom service types) is handled cleanly with predicate-based lookups. The fallback behavior of inserting missing services at index 1 is safe since LoadServices() always inserts "Custom" at index 0.

Note that line 142 now sets customServer text for all non-rtmp_common services, which correctly applies to both rtmp_custom and custom service types (like WHIP).


530-554: Custom service type enumeration with protocol validation.

The enumeration correctly filters service types by checking for a registered protocol before adding them to the dropdown. Creating temporary services with nullptr settings (line 544) is handled safely since the if (temp_service) check guards against services that require mandatory settings.


556-564: Special entries correctly positioned.

The "Custom" entry is consistently inserted at index 0, which is essential for the fallback insertion logic in LoadStream1Settings. The "Show All" entry is correctly omitted when already showing all services.


639-648: ShowAll detection cleanly integrated.

The "Show All" selection is now detected via ServiceItemData::isShowAll() rather than checking a magic index value. The early return after triggering LoadServices(true) correctly prevents downstream processing of the transient selection.


690-725: UI field visibility correctly differentiated.

The distinction between custom (rtmp_custom) and customServiceType (WHIP, etc.) is correctly applied:

  • Auth fields (lines 709-714) are only visible for custom services
  • Server input (lines 715-724) is visible for both custom and customServiceType

This correctly reflects that custom service types like WHIP need a server/endpoint but don't use the same auth mechanism as rtmp_custom.


418-421: Correct extension for custom service types.

Hiding the "more info" button for custom service types (in addition to custom services) is appropriate since these services won't have associated service metadata from rtmp_common.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@emilsas
emilsas merged commit aacfcbf into moq-dev:master Jan 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant