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