Skip to content

Conversation

@Kn0ax
Copy link
Contributor

@Kn0ax Kn0ax commented Feb 12, 2026

Changes include:

  • Wait for ICE gathering to complete before sending SDP offer (with fallback timeout)
  • Replace createOffer() with getLocalDescription() for proper ICE candidates
  • Use relative RTP timestamps instead of absolute for H.264
  • Simplify unused delegate parameters and audio RTP packetization

Changes include:
- Wait for ICE gathering to complete before sending SDP offer (with fallback timeout)
- Replace createOffer() with getLocalDescription() for proper ICE candidates
- Use relative RTP timestamps instead of absolute for H.264
- Simplify unused delegate parameters and audio RTP packetization
Copilot AI review requested due to automatic review settings February 12, 2026 01:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors WhipStream’s WHIP/WebRTC offer flow to better align SDP generation with ICE candidate gathering, and adjusts RTP timestamp handling/packetization for audio/video.

Changes:

  • Add ICE gathering state handling and a fallback timeout before sending the WHIP SDP offer.
  • Replace createOffer() with getLocalDescription() to send the current local SDP (including gathered candidates).
  • Switch H.264 RTP timestamps to be relative (session-based) and simplify audio/video encoder delegate handling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@eerimoq
Copy link
Owner

eerimoq commented Feb 12, 2026

the PR worked for me as well, but I'll implement it differently.

The problem on main was that the wrong video timestamp was used (it was always 0), and that ffplay does only work if starting RTP timestamp at a value close to 0 (I think). The AI fixed those, but other changes in the PR were not needed, at least not for me. Also, the AI:s timestamps fixes works in the normal case, but I'm sure audio and video would go out of sync if a few audio buffers are dropped somewhere is the audio pipeline.

Either way, AI helped a lot in figuring out what was wrong, but it's fix was kinda poor imo

@eerimoq eerimoq closed this Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants