Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stream matching #1314

Open
sipsorcery opened this issue Feb 6, 2025 · 3 comments
Open

Stream matching #1314

sipsorcery opened this issue Feb 6, 2025 · 3 comments
Labels

Comments

@sipsorcery
Copy link
Member

This logic here is not quite right. Whenever dealing with Audio and Video it will always return VideoStream when reached here even if the ssrc belongs actually to some AudioStream. This causes the VideoStream to receive RTPPacket which is not a video packet and it reaches to a point where RemoteEndPoint is adjusted and (Video) RemoteTrack.Ssrc will be set to hdr.SyncSource (Audio) which causes audio packets to be treated by VideoEncoder and having warnings all over the place: "Video depacketisation logic for codec PCMU has not been implemented, PR's welcome!".

I guess it is safe to return AudioStream when only HasAudio is true, return VideoStream in the else statement as it was before the change. Because when GetMediaStream (by ssrc) return null it's okay, because right after that it will try to get GetMediaStreamFromPayloadType which has found the correct stream in my cases.

Originally posted by @xBasov in 09555fb

@sipsorcery sipsorcery added the rtp label Feb 6, 2025
@ispysoftware
Copy link
Contributor

The original code was giving me repeating errors about being unable to find a media stream
Could we not just remove this logic entirely? It seems pretty hacky - just let GetMediaStreamFromPayloadType sort it out if that's the fallback?

@xBasov
Copy link
Contributor

xBasov commented Feb 11, 2025

I would not mind removing it entirely. Since for RTPPackets there is a fallback as you mentioned as well using GetMediaStreamFromPayloadType and for RTCPPackets the fallback should be GetMediaStream (by RTCPCompoundPacket).

@sipsorcery
Copy link
Member Author

Do you mean removing the block below? That would make sense to me. I'd say the only exception would be if there is only a single media stream it should be returned but then that could be the first check without any need for ssrc matching attempts.

if (HasAudio)
{
    if (!HasVideo)
    {
        return AudioStream;
    }
}

if (HasVideo)
{
    return VideoStream;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants