Mauro,
I've took more profound look into your patch and found a problem with it
The reason of the problem is (i think) that you misunderstood SDP
negotiation algorithm.
It is understandable because the algo is counter-intuitive.
When a SIP endpoint send and SDP (inside an INVITE or ACK or a SIP
responce) it describes what it is EXPECTING TO RECEIVE
and NOT what it is READY TO SEND as in RTSP.
Meaning, that in our case you need to copy the fmtp string in
opayloads structure
and the pass to the encoder_init the pointer to opayloads structure
and do decoder_init the pointer to ipayloads.
Btw to be on the safe side you should be passing to enc/dec_init not
the first structure but one
which contains the correct payload type
Thanks
Vadim
Le 20 nov. 08 à 20:37, Mauro Sérgio Ferreira Brasil a écrit :
Vadim,
Thanks.
It was great to work on it.
About your comments, please forgive my lack of comprehension of SDP
protocol and of Qutecom libraries, but I would like to understant it
completely.
I've just kept the payload and used it on the loop to avoid the
assignment of "fmtp" attribute to the wrong format/codec.
What you are telling me is that it won't happen because it will
exist only one payload media at a time ?
Thanks and best regards!
Mauro.
Vadim Lebedev escreveu:
Mauoro
I only took a quick look at it, and the only suggestion i have
is that we really don't need to copy the payload number from fmtp
string,
we need only copy what follows it.
so the test confirming presence of 111 in it is actually not needed.
Othewise - congratulations it ois GREAT work.
I'll try to integrate your patches into qutecom tomorrow and will
give you more feeback
Thanks
Vadim
Mauro Sérgio Ferreira Brasil wrote:
Hi Vadim!
Patch worked perfectly on my 2.1 changed version of Openwengo
source codes.
The changesets follow attached.
I've made them after paste the changes on current version 2.2 of
Qutecom source codes.
As I've said before, they need to be validated considering that
the version 2.2 codes with these patches were not even compiled
here.
Could you please give me a reply on that ?
Thanks and best regards,
Mauro.
Mauro Sérgio Ferreira Brasil escreveu:
Thanks a lot for the quicky response Vadim, and sorry by the too
frequent questions.
I agree with you to proceed with option 1.
Best regards,
Mauro.
Vadim Lebedev escreveu:
Mauro,
I agree that option 2 is better, however from practical
standpoint
option 1 is quicker to implement and less invasive.
So i would suggest option 1 for the moment, however this is your
decision
Thanks
Vadim
Mauro Sérgio Ferreira Brasil wrote:
Hi Vadim!
I have a new issue here.
On "ph_msession_audio_stream_hardstart" method, the field
"ptime" of "phastream_t" stream struct is initialized before
call to "open_audio_device".
Now that the final value of "ptime" field will be known just
after "encoder_init()", invocation, I need to choose one of two
approaches:
1- Get the "encoder_init()" and "decoder_init()" blocks up and
before "open_audio_device" invokation, taking care to execute
the "cleanup" method whether any error occurs;
2- Create a separated field on "phcodec_t" to a method
responsible to calculate the final frame size (20, 30, 40ms,
etc) that will be executed on beggining of
"ph_msession_audio_stream_hardstart" method, using the "int"
result on "open_audio_device" and encoder/decoder
initialization methods;
I prefer this approuch for the following reasons: 1- It will
reduce the calculation of final "ptime" to just one point (now
it is made on encoder and decoder initialization); 2- We can
use the result computed once on every points inside this
method; 3- We could initialize the "phcodec_t" respective field
with iLBC implementation, and left as NULL for the others
codecs that I don't know whether have especific packetization
rules through "fmtp" attribute;
Any ideas, sugestions ?
Thanks and best regards,
Mauro.
Vadim Lebedev escreveu:
Hi Mauro,
Le 19 nov. 08 à 21:11, Mauro Sérgio Ferreira Brasil a écrit :
Hi Vadim,
I've noticed just one more issue.
We are passing "ph_media_payload_s" to the "encoder_init" so
it can decide the packetization, but we are not considering
the global "ptime" value that comes on global media atribute
"ptime: 20".
The codec need to know this value as well, because it will be
used whether it's negotiated and no mode is informed.
I think we have two choices:
1- Replicate the "ptime" value to "ptime" field of
"ph_media_payload_s" before it's passed to "encoder_init";
YES
Thanks
Vadim
Vadim Lebedev escreveu:
YES, YES, YES
Thanks
Vadim
Mauro Sérgio Ferreira Brasil wrote:
Vadim,
I think I understood now why you've suggested to store the
"mode" info present on new "fmtp" field of the
"ph_media_payload_s" on field "ptime" by the "encoder_init"
of the associated codec.
This way the steps below could be considerable reduced with:
Step 3: the value will be stored on field "ptime" (and can
be stored on "mode" too for accuracy);
Step 4: We validate the "ptime" of the specific payload. If
the value is different from "0", this value will be
assigned to field "ptime" of "phastream_t". If it's equal
to "0", we use the global "ptime" present on
"ph_mstream_params_t";
Step 5: Is not necessary. Because we will deal with only
"ptime" field on "phastream_t".
I think this is really a better and simpler approach.
It was that you were thinking about ?
Best regards,
Mauro
Mauro Sérgio Ferreira Brasil escreveu:
Hi Vadim,
Let's try to close all the steps of the entire procedure:
1- "fmtp" media attributes will be stored on
"eXosip_event" structure during execution of method
"eXosip_event_add_sdp_info";
For now I'm saving only the value strings of the attribute
pair, like: "111 mode=30". Do you want to save the
complete attribute string, like "fmtp: 111 mode=30"?
2- The scope of "eXosip_event" structure on media
activation ends on method "ph_call_media_start". So, in it
we initialize the "fmtp" field of each
"ph_media_payload_s" structure of "ph_msession_s";
Just one question here. Is "fmtp" atribute unique for a
especific format/codec ? If so, everything is fine.
3- On "ph_msession_audio_stream_hardstart" method we will
inform the "ph_media_payload_s*" as a parameter to the
"encoder_init" method which, on the case of iLBC codec,
will update the "mode" field with the respective mode
collected from the "fmtp" field;
Any problems with that Vadim ? You sugested to put this
value on "ptime" field, but I think it would be better to
store the mode info on the respective "mode" field. Now
the decision is yours :-)
4- Still on this method, we will assign the "ptime" field
of the "phastream_t" struct with the global value of
"ptime" from "ph_mstream_params_t", and a new field "mode"
with the value of the especific payload "mode" value given
by "sp->ipayloads[0].mode";
Still ok until here ?
5- On "ph_astream_decoded_framesize_get" method, we will
priorize the "mode" field value to the calculation of fame
size, followed by the "ptime" field, both from
"phastream_t". The code will look like:
if (stream->mode)
{
// Mode changes the final frame sime the same way
"ptime" does. Modify the framesize to manage the informed
mode.
framesize = framesize * stream->mode/ 20;
}
else if (stream->ptime)
{
// Default ptime is 20ms, modify the framesize to
manage the new ptime.
framesize = framesize * stream->ptime / 20;
}
I think that's it.
Please let me know whether I forgot something.
Thanks,
Mauro.
Vadim Lebedev escreveu:
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
--
At.,
<mime-attachment.jpeg>
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
--
At.,
<CMMI_2.jpg>
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
_______________________________________________
QuteCom-dev mailing list
[email protected]
http://lists.qutecom.org/mailman/listinfo/qutecom-dev
