Data: Check for empty tracks map during data gathering or rebroadcasts fail on Windows#39
Open
danrossi wants to merge 1 commit into
Open
Data: Check for empty tracks map during data gathering or rebroadcasts fail on Windows#39danrossi wants to merge 1 commit into
danrossi wants to merge 1 commit into
Conversation
…f the video tracks map is filled or new broadcasts fail on Windows. - Clear the bytes total sent on stop.
WalkthroughThis pull request adds two defensive improvements to the MoQOutput class in 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/moq-output.cpp`:
- Around line 153-156: The early return when video_tracks.empty() causes the
first video packet (and thus track initialization in VideoData()) to be dropped
and prevents any future initialization; instead of returning from that block,
change the logic to avoid discarding the first packets—either remove the return
and skip only shutdown/failure paths, or replace the return with a
no-op/continue and only perform the guard after VideoData() runs (or after a
dedicated initialized flag is set). Locate the guard referencing video_tracks
(and the VideoData() consumer) and adjust it so the check does not run before
track initialization completes, ensuring VideoData() can populate video_tracks
on the first packet.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+153
to
+156
| // OBS tries to gather data on signalling a stop success. Check once if the video tracks map is filled or new broadcasts fail on Windows. | ||
| if (video_tracks.empty()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Guard drops the first video packet and prevents track initialization forever.
This return runs before VideoData(), so video_tracks never gets initialized and all packets are skipped indefinitely.
Suggested fix
- // OBS tries to gather data on signalling a stop success. Check once if the video tracks map is filled or new broadcasts fail on Windows.
- if (video_tracks.empty()) {
+ // Ignore non-video packets until at least one video track is initialized.
+ // This still allows the first video packet through to create the track.
+ if (video_tracks.empty() && packet->type != OBS_ENCODER_VIDEO) {
return;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/moq-output.cpp` around lines 153 - 156, The early return when
video_tracks.empty() causes the first video packet (and thus track
initialization in VideoData()) to be dropped and prevents any future
initialization; instead of returning from that block, change the logic to avoid
discarding the first packets—either remove the return and skip only
shutdown/failure paths, or replace the return with a no-op/continue and only
perform the guard after VideoData() runs (or after a dedicated initialized flag
is set). Locate the guard referencing video_tracks (and the VideoData()
consumer) and adjust it so the check does not run before track initialization
completes, ensuring VideoData() can populate video_tracks on the first packet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: #38
OBS tries to gather data on signalling a stop success. Check once if the video tracks map is filled during data gathering or new broadcasts fail on Windows. Data gathering continues on signal stop and causes an error. Which then affects video track data on rebroadcasting causing a "decode failed: moq: protocol violation"
Other plugins check if video and audio data is set but just the one map check before should be fine.
The bytes total sent is now reset on stop.