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

[Klaus Darilion]

Sergio Garcia wrote:
> 
> 
> ---------- Original Message ---------------------------------- From:
> Klaus Darilion <klaus.mailinglists at pernau.at> Reply-To: Development
> discussion of video media support in
> Asterisk<asterisk-video at lists.digium.com> Date:  Wed, 25 Jul 2007
> 16:10:32 +0200
> 
>> 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;
> 
> Ups you-'re rigth

Same problem in app_mp4.c static: mp4_rtp_read(struct mp4rtp *p)



-  memset(f, 0, sizeof(struct ast_frame) + 1500);
+  memset(f, 0, sizeof(struct ast_frame) + AST_FRIENDLY_OFFSET + 1500);
         /* Let mp4 lib allocate memory */
-   f->data = (void*)f + AST_FRIENDLY_OFFSET;
+   f->data = (void*)f + sizeof(struct ast_frame) + AST_FRIENDLY_OFFSET;
+   f->offset = AST_FRIENDLY_OFFSET;
     f->datalen = 1500;


btw: setting datalen to 1500 is no safe method, as datalen is not
checked in mpeg4ip library:

> ... The calling application is responsible for ensuring that the
> buffer is large enough to hold the packet. This can be done by using
> MP4GetRtpPayload() to retrieve the maximum packet payload size and
> hence how large the receiving buffer must be. ...

But I could not find the MP4GetRtpPayload() function. :-(

>> 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?
> 
> 
> What you receive in the framedata is the amr if2 frame, you have to
> add a 2 byte header for rtp transmision. The problem is that I don't
> have any softphone client with amr support to test it.. :(
> 
> See http://www.ietf.org/rfc/rfc3267.txt Chapter 4.4.1. The Payload
> Header

I have just read RFC 4867 and things are much more clear now.

Thus - what is the exact format how AMR should be encoded in 
ast_frame->data ? Like RFC 4867, octet-aligned, normal order?

> 
>> /* 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?
> 
> Ok.
> 
>> 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.
> 
> 
> I'll have to check that, probably it's wrong also.

No. I think it is fine. it was just a comment that in this special case 
(allocating the frame structure AND the data buffer with a single 
malloc) it is fine.

regards
klaus