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.,
|
| <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 |
--
| 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 |
|