[Asterisk-video] several questions to source code of app_h324m

[Klaus Darilion]

Hi Sergio!

I'm reading the code to find out why some things don't work for me. Can 
you please comment on my questions?


static struct ast_frame* create_ast_frame(void *frame, struct video_tr *vtr)
{
   int mark = 0;
   struct ast_frame* send;

   /* Get data & size */
   unsigned char * framedata = FrameGetData(frame);
   unsigned int framelength = FrameGetLength(frame);

   /* Depending on the type */
   switch(FrameGetType(frame))
   {
     case MEDIA_AUDIO:
       /*Check it's AMR */
       if (FrameGetCodec(frame)!=CODEC_AMR)
         /* exit */
         return NULL;
       /* Create frame */
       send = (struct ast_frame *) malloc(sizeof(struct ast_frame) + 
AST_FRIENDLY_OFFSET + framelength + 2);

If I Understand Correctly (IIUC) you allocate the frame structure AND 
the buffer for the data with one malloc. Thus, the frist part of the 
allocated memory is the structure, followed by the data buffer?

       /* Set data*/
       send->data = (void*)send + AST_FRIENDLY_OFFSET;

I think this is wrong - it overwrites the structure.
send->data = (void*)send + sizeof(struct ast_frame) +
              AST_FRIENDLY_OFFSET;

       send->datalen = framelength;
       /* Set header cmr */
       ((unsigned char*)(send->data))[0] = 0xF0;
       /* Set mode */
       ((unsigned char*)(send->data))[1] = (framedata[0] & 0x78) | 0x04;


Here you set the Codec Mode Request 2 times? You set the byte 0 to 0xF0 
(what is that - is that something standardized or just a proprietary 
method for signaling something?).

Then you reset the bits 7,2,1,0 and then you set the bit 2. What for? I 
though the topmost 4 bits (7,6,5,4) are relevant for the codec mode?

       /* Copy */
       memcpy(send->data+2, framedata, framelength);

Although the cmr is already copy to framedata[1], it will be copied 
(with the original value) to framedata[2] too. Why?

       /* Set video type */
       send->frametype = AST_FRAME_VOICE;
       /* Set codec value */
       send->subclass = AST_FORMAT_AMRNB;
       /* Rest of values*/
       send->src = "h324m";
wouldn't it be nicer to define "h324m" as global variable?

       send->samples = 160;
       send->delivery.tv_usec = 0;
       send->delivery.tv_sec = 0;
       send->mallocd = 0;
IIUC setting mallocd=0 works as the data buffer is allocated together 
with the frame structure. Thus it will be freed when to frame structure 
is freed.


       /* Send */
       return send;



regards
klaus