[Asterisk-code-review] SDP: Ensure SDPs "merge" properly. (asterisk[master])

[Joshua Colp]

Joshua Colp has posted comments on this change. ( https://gerrit.asterisk.org/5415 )

Change subject: SDP: Ensure SDPs "merge" properly.
......................................................................


Patch Set 2: Code-Review-1

(7 comments)

Few minor comments, and it seems as though one (or more) of your tests has registration problems which is causing the unit test for test registration to fail.

https://gerrit.asterisk.org/#/c/5415/2/include/asterisk/sdp_options.h
File include/asterisk/sdp_options.h:

PS2, Line 411: unsigned int ast_sdp_options_get_rtcp_mux(const struct ast_sdp_options *options);
             : void ast_sdp_options_set_rtcp_mux(struct ast_sdp_options *options, unsigned int value);
Documentation plz?


https://gerrit.asterisk.org/#/c/5415/2/main/sdp.c
File main/sdp.c:

Line 719: 	stream = ast_stream_alloc("blah", media_type_from_str(m_line->type));
I think naming it based on the type is suitable in the case where we don't have an identifier (in WebRTC we should if I remember right)


https://gerrit.asterisk.org/#/c/5415/2/main/sdp_state.c
File main/sdp_state.c:

Line 199: 		state_stream->instance = create_rtp(options,
This should only occur if the type is audio or video


Line 413: 	joint_stream = ast_stream_alloc("goobers", ast_stream_get_type(remote));
I can haz type as name?


Line 567: 			joint_state_stream->instance = create_rtp(options, new_stream_type);
This should only occur if it's audio or video


Line 578: 			joint_stream = ast_stream_alloc("dummy", new_stream_type);
will the dummy stream end up in the channel's streams in the end?


Line 806: 		if (state_stream->instance) {
This should only occur if audio or video


-- 
To view, visit https://gerrit.asterisk.org/5415
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5938c2be3c6f0a003aa88a39a59e0880f8b2df3d
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-HasComments: Yes