[asterisk-dev] app_mp3.c, timed_read, short frames

[Paul Albrecht]

On Jul 12, 2013, at 11:25 AM, Matthew Jordan <mjordan at digium.com> wrote:

> 
> On Fri, Jul 12, 2013 at 9:04 AM, Paul Albrecht <palbrecht at glccom.com> wrote:
> Hi,
> 
> Think I found the reason the mp3 application sends short frames. Here's the relevant code:
> 
> static int timed_read(int fd, void *data, int datalen, int timeout)
> {
>         int res;
>         struct pollfd fds[1];
>         fds[0].fd = fd;
>         fds[0].events = POLLIN;
>         res = ast_poll(fds, 1, timeout);
>         if (res < 1) {
>                 ast_log(LOG_NOTICE, "Poll timed out/errored out with %d\n", res);
>                 return -1;
>         }
>         return read(fd, data, datalen);
> 
> }
> 
> 
> I'm not sure I would fix it there. The loop in mp3_exec is what is calling timed_read and constructs the audio frame from the read data - it should probably check the number of bytes read and, if it isn't enough for a full frame, buffer the data and call read again.
> 

It makes more sense to fix timed_read because the loop provides frame timing … something like this:

static int timed_read(int fd, void *data, int datalen, int timeout)
{
	int res;
	struct pollfd fds[1];
	fds[0].fd = fd;
	fds[0].events = POLLIN;
	res = ast_poll(fds, 1, timeout);
	if (res < 1) {
		ast_log(LOG_NOTICE, "Poll timed out/errored out with %d\n", res);
		return -1;
	}

	int tot = 0;
	while ((res = read(fd, data+tot, datalen-tot)) > 0)
	{
		if ((tot+=res) == 320) break;
	}
	return tot;

	//return read(fd, data, datalen);
	
}


> So yes, "bug".
> 
> On the other hand, app_mp3 is an extended support module, so any development effort for it comes from the Asterisk developer community. A patch fixing the behavior would be appreciated.
> 
> Matt
> 
> -- 
> Matthew Jordan
> Digium, Inc. | Engineering Manager
> 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
> Check us out at: http://digium.com & http://asterisk.org
> --
> _____________________________________________________________________
> -- Bandwidth and Colocation Provided by http://www.api-digital.com --
> 
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>   http://lists.digium.com/mailman/listinfo/asterisk-dev