Skip to content

Conversation

@Gawor270
Copy link
Member

When the first RTP packet containing codec information is received, we update our track_data to include this codec.

@Gawor270 Gawor270 requested a review from sgfn August 28, 2025 11:14
@Gawor270 Gawor270 requested a review from sgfn September 2, 2025 09:23
}
end

defp parse_codec(nil), do: nil
Copy link
Member

Choose a reason for hiding this comment

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

I believe this will be "nil" if read from the serialized JSON.

If you have the time, you could consider writing unit tests for this module -- since it looks like we'll continue extending the Recorder, adding tests will help ensure basic functionality like this keeps working with further changes

Copy link
Member Author

Choose a reason for hiding this comment

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

I assume the value returned by Jason.decode!(json_string) is passed to from_json!() (see converter.ex:134); Jason.decode/1 converts JSON null to Elixir nil, and I added tests to confirm this.

Copy link
Member

Choose a reason for hiding this comment

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

Right, my bad. I assumed it behaved the same way as our rid_map, where we do "nil" -> nil ourselves

@Gawor270 Gawor270 requested a review from sgfn September 3, 2025 08:56
Copy link
Member

@sgfn sgfn left a comment

Choose a reason for hiding this comment

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

Nicely done 👍

Copy link
Member

Choose a reason for hiding this comment

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

🥇

@Gawor270 Gawor270 merged commit 2320456 into main Sep 3, 2025
1 check passed
@Gawor270 Gawor270 deleted the codec_in_manifest branch September 3, 2025 10:13
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.

3 participants