Skip to content

Conversation

@FelonEkonom
Copy link
Member

No description provided.

@FelonEkonom FelonEkonom requested a review from mat-hek February 12, 2025 10:35
@FelonEkonom FelonEkonom self-assigned this Feb 12, 2025
@FelonEkonom FelonEkonom marked this pull request as ready for review February 12, 2025 10:35
Comment on lines 127 to 133
Membrane.Logger.warning("""
Cannot select expicitly video codec, while various video codecs have been negotiated. \
This might be caused be passing `Membrane.RTP` to `Membrane.WebRTC.Sink` and allowing \
to negotiate more than one video codec.

Negotiated video codecs: #{inspect(supported_video_codecs, pretty: true)}
""")
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to fail randomly, like before the fix? Shouldn't we raise here?

Comment on lines 169 to 173
defp get_video_transceiver(pc) do
pc
|> PeerConnection.get_transceivers()
|> Enum.find(&(&1.kind == :video))
end
Copy link
Member

Choose a reason for hiding this comment

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

This finds the first video transceiver, while we can have multiple video tracks. Shouldn't we find the transceiver corresponding to the particular track?

Comment on lines 128 to 131
Cannot select expicitly video codec, while various video codecs have been negotiated. \
This might be caused be passing `Membrane.RTP` to `Membrane.WebRTC.Sink` and allowing \
to negotiate more than one video codec.
Negotiated video codecs: #{inspect(supported_video_codecs, pretty: true)}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Cannot select expicitly video codec, while various video codecs have been negotiated. \
This might be caused be passing `Membrane.RTP` to `Membrane.WebRTC.Sink` and allowing \
to negotiate more than one video codec.
Negotiated video codecs: #{inspect(supported_video_codecs, pretty: true)}
Cannot determine the video codec of the stream. It's necessary, as multiple video codecs have been negotiated. \
This error might be caused by passing `Membrane.RTP` to `Membrane.WebRTC.Sink` and allowing \
to negotiate more than one video codec.
Negotiated video codecs: #{inspect(supported_video_codecs, pretty: true)}

@FelonEkonom FelonEkonom merged commit c0a4c66 into master Feb 19, 2025
3 checks passed
@FelonEkonom FelonEkonom deleted the set-rtp-sender-codec branch February 19, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants