Add multi encoder output support

[Qizot]

No description provided.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 69cc0da6-4aaa-44f5-93c6-a0386fa80725

📥 Commits

Reviewing files that changed from the base of the PR and between f9ba758 and 4487944.

📒 Files selected for processing (1)

• src/moq-output.cpp

---

Walkthrough

The output module was refactored from single scalar media track handles to per-encoder track maps (video_tracks, audio_tracks). VideoInit and AudioInit now accept an obs_encoder_t * parameter for encoder-specific initialization. Start() probes all encoder indices for any available video encoder; Stop() closes and clears all tracks in the maps. AudioData()/VideoData() lazily initialize tracks keyed by packet->encoder and drop frames if initialization fails. register_moq_output() adds multi-track video and audio flags.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to evaluate relevance to the changeset.	Add a pull request description explaining the purpose, motivation, and impact of adding multi-encoder output support for better context.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title 'Add multi encoder output support' accurately reflects the main change: implementing multi-encoder track mapping to support multiple video and audio encoders simultaneously.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

✨ Simplify code

• Create PR with simplified code

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (2)

src/moq-output.cpp (2)

158-166: Consider reducing redundant map lookups.

The current pattern performs two find() calls when the encoder is new. You could combine the check and retrieval using try_emplace or cache the iterator.

♻️ Optional: Single lookup pattern

-	if (audio_tracks.find(encoder) == audio_tracks.end())
-		AudioInit(encoder);
-
-	auto it = audio_tracks.find(encoder);
-	if (it == audio_tracks.end() || it->second < 0) {
+	auto [it, inserted] = audio_tracks.try_emplace(encoder, -1);
+	if (inserted)
+		AudioInit(encoder);
+
+	if (it->second < 0) {
 		// We failed to initialize the audio track, so we can't write any data.
 		return;
 	}

Note: This would require AudioInit to update the existing map entry rather than insert.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/moq-output.cpp` around lines 158 - 166, The code does two
audio_tracks.find(encoder) lookups; change to a single lookup and update path:
call AudioInit(encoder) only when necessary and then obtain the iterator once,
or use audio_tracks.try_emplace(encoder, /*default value*/ ) before inspecting
the value so you can check and read the entry via the returned iterator. Ensure
AudioInit either writes the map entry (so try_emplace is appropriate) or, if
AudioInit performs initialization externally, call it conditionally and then
call audio_tracks.find(encoder) only once to get the iterator (use the iterator
result to check it->second and assign handle).

---

16-18: Initializer list order doesn't match declaration order.

Members are initialized in declaration order (origin, session, broadcast), but the init list specifies broadcast before session. This mismatch may trigger -Wreorder warnings and is misleading to readers.

♻️ Proposed fix to match declaration order

 MoQOutput::MoQOutput(obs_data_t *, obs_output_t *output)
 	: output(output),
 	  server_url(),
 	  path(),
 	  total_bytes_sent(0),
 	  connect_time_ms(0),
 	  origin(moq_origin_create()),
-	  broadcast(moq_publish_create()),
-	  session(0)
+	  session(0),
+	  broadcast(moq_publish_create())
 {
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/moq-output.cpp` around lines 16 - 18, The initializer list orders members
as origin, broadcast, session but the class declares them as origin, session,
broadcast; reorder the initializer list to match declaration order by placing
session(0) before broadcast(moq_publish_create()) so the members are initialized
in the declared sequence (adjust the constructor that currently initializes
origin, broadcast, session accordingly).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/moq-output.cpp`:
- Around line 158-166: The code does two audio_tracks.find(encoder) lookups;
change to a single lookup and update path: call AudioInit(encoder) only when
necessary and then obtain the iterator once, or use
audio_tracks.try_emplace(encoder, /*default value*/ ) before inspecting the
value so you can check and read the entry via the returned iterator. Ensure
AudioInit either writes the map entry (so try_emplace is appropriate) or, if
AudioInit performs initialization externally, call it conditionally and then
call audio_tracks.find(encoder) only once to get the iterator (use the iterator
result to check it->second and assign handle).
- Around line 16-18: The initializer list orders members as origin, broadcast,
session but the class declares them as origin, session, broadcast; reorder the
initializer list to match declaration order by placing session(0) before
broadcast(moq_publish_create()) so the members are initialized in the declared
sequence (adjust the constructor that currently initializes origin, broadcast,
session accordingly).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 385986ea-3ef3-4740-917c-ea7bf03f043e

📥 Commits

Reviewing files that changed from the base of the PR and between 38a1495 and f9ba758.

📒 Files selected for processing (2)

• src/moq-output.cpp
• src/moq-output.h

[kixelated]

thanks mr @Qizot