Hi Mauro,
see the comments inline
Mauro Sérgio Ferreira Brasil wrote:
Hi Vadim!
Thanks for the reply.
In fact I need some help of more experienced guys like you to define
the points I haven't still decided.
They were detailed on email below, and I'll list them shortly here:
1- Are you fine with my choice to invoke method
"eXosip_get_sdp_media_format_info" from method
"eXosip_event_add_sdp_info" instead of calling it from method
"eXosip_get_sdp_media_info" ?
OK
2-
Should I create a field on struct "ph_mstream_param_s" (using a
global approach, like on ptime), or should I use the "mode" field
already present on elements of "ipayloads" field ? I'm not completely
sure how to handle this situation by now, because I don't know all
scenarios (like I've said on prior email, always I have just one
"ph_mstream_param_s" on my session (of 4 maximum), that have configured
just one "ph_media_payload_s" on "ipayloads" field (of 16 maximum).
Considering this scenario, maybe a global "mode" field on
"ph_mstream_param_s" struct will provide a cleaner solution. But
whether we do need to consider another streams, we will have to map the
"mode" on the respective "ph_media_payload_s" field.
I think it would be better to implement it per
payload.
I was thinking to add a const 'char *fmtp' and int ptime
ph_media_payload_s.
if we could pass ph_media_payload to the codec intializer it could fill
ptime based on fmtp. What do you think?
3-
During the calculation of allocation blocks using
"ph_astream_decoded_framesize_get", I'm thinking to change the
algorithm just to validate first the existence of a "mode" attribute
that will have priority agains "ptime".
This is the only change this method will probably have.
In line with my previous comment, the ph_astream_decoded_framesize_get
will use ptime
from ph_media_payload_s structure if nonzero
For the
sake of simplicity I've omitted changes pointed like the
creation
of "format_modifiers" on "eXosip_event" struct, and the names given to
new methods (like "eXosip_get_sdp_media_format_info"), considering that
you have agreed with them.
Please confirm that to me too.
Ok for me
This
approach only makes me unconfortable considering that "mode"
attribute value is dependent of the codec or format. So maybe we should
in fact extend the "phcodec_t" struct to accept a new field that is a
method responsible to return the frame size considering a parameter of
type "ph_media_payload_s".
If we choose to follow this idea, maybe we should remove "ptime" field
of "ph_mstream_params_s" struct and update its value on "ptime" field
of each valid "ph_media_payload_s" element.
This is valid approach too, i think that what
i'm proposing (setting ph_media_payload_s.ptime in the codec
initializer based on fmtp) will require less modification to the code,
but otherwise your approach is equivalent so it is your call.
Thanks
Vadim
Thanks
and best regards,
Mauro.
Vadim Lebedev escreveu:
Mauro
It's an excellent idea,
please submit the patch
Thanks
Vadim
Mauro Sérgio Ferreira Brasil wrote:
Hello there!
We work with a Windows application here that uses Openwengo libraries to
provide Voip services, and we are dealing with some packetization
problems where an offerer supports only 30ms packetization for iLBC codec.
Having a look on iLBC implementation on "phApi", I've noticed that
almost all codecs support only 20ms packetization which could maybe
became dynamic with use of just one parameter that isn't being applied
by now.
For iLBC at least, the packetization is configured on codec
initialization method "phcodec_t::encoder_init" which receives "NULL" by
now for a "void *" parameter.
The general idea is:
1- On the same way the "ptime" attribute was added to "eXosip_event"
struct, we could add a new field (maybe called "format_modifiers"), that
will hold a list of all "fmtp" attribute values collected from the media
negotiation.
For the negotiation below, for example, the result list will have the
items: "111 mode=30" and "101 0-16".
Media Description, name and address (m): audio 15738 RTP/AVP 111 101
Connection Information (c): IN IP4 200.221.10.68
Media Attribute (a): rtpmap:111 iLBC/8000
Media Attribute (a): fmtp:111 mode=30
Media Attribute (a): rtpmap:101 telephone-event/8000
Media Attribute (a): fmtp:101 0-16
Media Attribute (a): ptime:20
Media Attribute (a): maxptime:20
Media Attribute (a): direction:active
This data extraction could happen on "eXosip_get_sdp_media_info" method,
or inside method "eXosip_event_add_sdp_info", by a new method called
"eXosip_get_sdp_media_format_info", on the same way it's done with
"ptime" attribute.
I choose to put it inside method "eXosip_event_add_sdp_info" to avoid a
new change on method "eXosip_get_sdp_media_info" signature without a
good reason. Please point your options and objections about any details
of the approach like this one.
2- The second pass envolves the processing of the list of collected
attrubutes on a usefull field indicating the mode used by the codec. I
choose to make this task inside method "ph_call_media_start" with the
help of a new method called "eXosip_get_format_mode".
Here I'm with a pair of questions on mind that you guys can help me out:
1- Should I create a new field "mode" on "ph_mstream_param_s" struct on
the same way that happened with "ptime" (which will be interpreted as a
global configuration), or can I use the "mode" atrubute present on
"ipayloads" first element (that on my testes always have the selected
payload paramaters) ? 2- If I choose to store this info on a per-payload
basis (using "ipayloads" field), must I consider that "ipayloads" will
have more than one element and make a one-to-one analysis with
formatation and payload (this could seem a dumb question considering
that "ipayloads" is a array of 16 elements, but on my tests it always
have only one item assigned) ?
For now, I'm using "mode" field from "ipayloads" first element, but I
think maybe a new field should be created on "ph_mstream_param_s" struct
(on a global fashion) considering that, at least on my tests, generally
the streams envolved are just one (maybe on video this could be
different ??? ).
3- With the "mode" collected and available on "ph_mstream_param_s" (or
on one of "ipayloads" elements), it becomes easy to get this info on
method "ph_msession_audio_stream_hardstart" and pass it during encoder
and decoder initialization that for now receives always a "NULL" on it's
"void *" only parameter. Maybe a new struct could be created to pass the
required info to the encoder and decoder initialization methods.
On the same way the "mode" must be propagated to "phastream_t" created
on method "ph_msession_audio_stream_hardstart", so this field could be
used on "ph_astream_decoded_framesize_get" too to avoid conflicted
scenarios like the example above where "ptime" indicates 20ms and "mode"
indicates 30ms for packetization.
4- On each codec initialization, this info will be handled accordingly
to get the expected frame packetization. For now I'm proposing only to
change "iLBC" initialization considering that it's the only one we have
problems until now.
Does anyone see any problems with this approach ?
Is there any possibility of such maintenance being added to main Qutecom
codes ?
Thanks and best regards!
_______________________________________________
QuteCom-dev mailing list
[email protected]
http://lists.qutecom.org/mailman/listinfo/qutecom-dev
--
TQI - Technology and Quality on Information
| At.,
|
 |
| Technology
and Quality
on Information |
| Mauro Sérgio
Ferreira Brasil |
| Coordenador
de Projetos e
Analista de Sistemas |
| + [EMAIL PROTECTED] |
| : www.tqi.com.br |
| ( + 55 (34)3291-1700 |
| ( + 55 (34)9971-2572 |
|
