Title: TQI - Technology and Quality on Information
Hi Vadim!

Considering block code starting on line 2803 from "phapi-old.c", the "opayloads[0]" info will be the same from "ipayloads[0]", so what you sugest is that I execute method "ph_update_media_payloads_fmtp" before the final assignment "msp->opayloads[0] = msp->ipayloads[0];" so both "ipayloads" and "opayloads" have "fmtp" value assigned at the same time, right ?
Or you think it would be better to change "ph_update_media_payloads_fmtp" so new strings are allocated for "opayloads" "fmtp" field on the same way it's done to "ipayloads" ?

About your second indication, that I should pass to encoder/decoder initialization methods the correct payload type, can you please tell me how can I select the right one from the array ?
I mean, which field or paramater should I use to select the correct payload to be informed on encoder/decoder init methods ?
I couldn't find a field or parameter on "ph_msession_audio_stream_hardstart" that could indicate this to me. Maybe we can pass the entire "ph_mstream_params_t" variable and let the codec that "knows" which is its ID to find it out on the payloads array.

Thanks,
Mauro.



Vadim Lebedev escreveu:
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.




--
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
_______________________________________________
QuteCom-dev mailing list
[email protected]
http://lists.qutecom.org/mailman/listinfo/qutecom-dev

Reply via email to