Wicpar
commented
1 year ago do you think #358 could be related to this ?
Open KillingSpark opened 1 year ago
Wicpar
commented
1 year ago do you think #358 could be related to this ?
KillingSpark
commented
1 year ago I'd doubt it. #358 seems to be about a re-ordering of the media sections, which I had never had issues with
k0nserv
commented
1 year ago Hmm this does seem like a bug. I wonder if the #377 might fix it.
KillingSpark
commented
1 year ago I looked into this a bit more and I think it has to do with how the media engine deals with update_from_remote_description
Excerpt: (the comments are mine)
for media in &desc.media_descriptions {
let typ = if !self.negotiated_audio.load(Ordering::SeqCst)
&& media.media_name.media.to_lowercase() == "audio"
{
self.negotiated_audio.store(true, Ordering::SeqCst); // <---- Makes it so that only one media description influences the media engines audio codecs
RTPCodecType::Audio
} else if !self.negotiated_video.load(Ordering::SeqCst)
&& media.media_name.media.to_lowercase() == "video"
{
self.negotiated_video.store(true, Ordering::SeqCst); // <---- Makes it so that only one media description influences the media engines video codecs
RTPCodecType::Video
} else {
continue;
};If I understand correctly only the first media section of a type (video/audio) influences which codecs will be added to the media engines negotiated_{audio,video}_codecs list. This has at least two consequences I encountered:
I think if these checks could just be dropped and every mediasection gets its chance to add the necessary codecs to the negotiated codecs this could be solved. I'll try that out with a local copy tomorrow.
Edit: After thinking about this more this seems like a conceptual problem. Should the media engine even have these fields?
pub struct MediaEngine {
...
pub(crate) negotiated_video_codecs: Mutex<Vec<RTCRtpCodecParameters>>,
pub(crate) negotiated_audio_codecs: Mutex<Vec<RTCRtpCodecParameters>>,
...
}In my understanding these need to be tracked for each media section separately because each media section is allowed to negotiate different codecs.
It's me again doing weird stuff with this crate :) This is possibly related to #308
The most minimal example I could come up with is an offer with two m-lines, one with only codec VP9 and one with only codec VP8: (full SDPs attached at the end)
If I set this as a remote offer on a peer connection it produces these transceiver states: Both of these have (only) the codec video/VP9 selected
I didn't add a track on the peer connection in this minimal example but when I encountered it, the RTCRtpSender also had the VP9 codec selected.
The answer produced is this:
If a track (and by that a RTCRtpSender) exists on MID 1 this causes an error when setting the answer because of incompatible codecs. In my minimal example I did not add a track and then this doesn't even return an error.
Details