Here's a proposal for sdp parsing inside the openser core. Most of the parsing functions and helper functions were ported from the nathelper module.
The parser will parse the SDP and it will fill up the sdp_info structure (parser/sdp/sdp.h).
Each sdp may have multiple sessions (the start of a session is identified by an `v=' line - see sdp_session_cell). Inside each session we may have multiple streams (each stream is identified by an `m=' line - see sdp_stream_cell). Each stream may have many payloads (see sdp_payload_attr).
For each stream the following fields are parsed:
- media type,
- port,
- transport type.
For each payload the following fields are parsed:
- rtp payload,
- rtp encoding,
- rtp clock,
- rtp params,
- send and receive mode,
- packetization.
The structure will be initialized when parse_sdp() is invoked and the structure will be available until the "receive_msg clean up" (just like the SIP headers).
Streams are accessible via:
sdp_stream_cell_t* get_sdp_stream(struct sip_msg* _m, int session_num, int stream_num).
Payloads are accessible via:
sdp_payload_attr_t* get_sdp_payload4payload(sdp_stream_cell_t *stream, str *rtp_payload)
sdp_payload_attr_t* get_sdp_payload4index(sdp_stream_cell_t *stream, int index)
Files for this patch:
A parser/sdp
A parser/sdp/sdp_helpr_funcs.c
A parser/sdp/sdp_helpr_funcs.h
A parser/sdp/sdp.c
A parser/sdp/sdp.h
M parser/msg_parser.c
M parser/msg_parser.h
M Makefile.sources
TODO:
Add support for multi-part body parsing.
Regards,
Ovidiu Sas
Logged In: YES
user_id=1275325
Originator: NO
Hi Ovidiu,
I will take a look and come back with some feedback.
Regards,
Bogdan
Logged In: YES
user_id=1395524
Originator: YES
Patch file updated with multipart/mixed support.
File Added: sdp_parser.patch.gz
Logged In: YES
user_id=1395524
Originator: YES
Patch updated.
Regards,
Ovidiu Sas
File Added: sdp_parser.patch.gz
Logged In: YES
user_id=1395524
Originator: YES
Patch updated. If there are no comments/objections, I will push this in by the end of the week.
Regards,
Ovidiu Sas
File Added: sdp_parser.patch.gz
Logged In: YES
user_id=337916
Originator: NO
Hi Ovidiu,
i did a short review, overall the code looks quite good. But i have a few points, mainly to make this code more maintainable:
You have some code that is outcommented in the sdp.c file (on the top), perhaps this can be removed if its not used at all? I also miss some comments in this files, that describe what does this internal (static) helper functions does, whats the purpose of the parameter is and such. Also the external API (spd.h) could be perhaps better documented. E.g. what are valid values for the input parameters, what happens if an error occurs, do they allocate memory (and what type of)..
It would be great if you can also do some little changes that allows that this are recognized from doxygen? Basically you only need to add a single '*' and sometimes a '<' for the comments. E.g.:
/**
* some function foo
*/
int foo(){}
struct foo{
int bar /**< number of bars */
}
The #defines in sdp_helpr_func.c, "READ", "advance" are also defined in some other parts in the server, for example n_helpr_funcs.c from nathelper. The function ser_memmem is also defined in this module. Perhaps it makes sense to integrate this code into a common place if both need them?
Cheers,
Henning
Logged In: YES
user_id=1395524
Originator: YES
Thank you Henning for the review.
The new functionality has been added. I fixed part of the documentation and I will work more on it.
Also, I will address the duplication code issue.
The sdp parser is based on the existing sdp parsing methods from the nathelper module and hence the duplication :)
Regards,
Ovidiu Sas