[MPlayer-dev-eng] Faster libaf

[Reimar Döffinger]

Hi,

On Wed, Dec 29, 2004 at 12:16:45PM +0100, Alex Beregszaszi wrote:
> > I don't quite get it... How can the format change inbetween? I don't
> > think it can without a reinit of the filter chain?
> > Also most filters don't have huge if-then-else blocks in the play
> > functions (they support only one format), can you give an example?
> 
> Mostly af_format
> 
> > 
> > > What do you think? I know it looks like crap (and it is) but still
> > > better than the current situation.
> > 
> > Allowing to change value of af->play during the REINIT control should
> > be enough to get what you want unless I misunderstood you...
> 
> Here's an updated patch attached.
> 
> I decided to take this way of doing it because I didn't wanted to cause
> more 10l bugs and recieve more cartloads of coke. But if you accept that
> filters shouldn't change the parameters without REINIT than just ignore
> the play_fixed stuff in my patch and just change the play in af_format
> to what i proposed.

As far as I can see it this is just impossible otherwise, as without
REINIT how can you find out if the filter supports that format?? So
whenever it is changed there is no other way than doing REINIT, and
maybe even inserting (or removing) some additional filters.
Thus I'm against that play_fixed stuff...

> An explanation of the af_format changes:
> play_swapendian is just a byteswapping one, which is the most common
> case. Other common case is converting from float to s16. These are
> implemented, however, s16 to float is still missing.

Ideally the float conversion shouldn't be common at all. Signed <->
unsigned conversion might happen quite often, too...
Another idea would of course be to split af_format in multiple
filters, but probably not that good an idea...

>    if(AF_FORMAT_LE == (format & AF_FORMAT_END_MASK))
> -    i+=snprintf(str,size-i,"little endian ");
> +    i+=snprintf(str,size-i,"little-endian ");
>    else
> -    i+=snprintf(str,size-i,"big endian ");
> +    i+=snprintf(str,size-i,"big-endian ");
>    
>    if(format & AF_FORMAT_SPECIAL_MASK){
>      switch(format & AF_FORMAT_SPECIAL_MASK){
>      case(AF_FORMAT_MU_LAW): 
> -      i+=snprintf(&str[i],size-i,"mu law "); break;
> +      i+=snprintf(&str[i],size-i,"mu-law "); break;
>      case(AF_FORMAT_A_LAW): 
> -      i+=snprintf(&str[i],size-i,"A law "); break;
> +      i+=snprintf(&str[i],size-i,"A-law "); break;
>      case(AF_FORMAT_MPEG2): 
> -      i+=snprintf(&str[i],size-i,"MPEG 2 "); break;
> +      i+=snprintf(&str[i],size-i,"MPEG-2 "); break;
>      case(AF_FORMAT_AC3): 
>        i+=snprintf(&str[i],size-i,"AC3 "); break;
>      case(AF_FORMAT_IMA_ADPCM): 
> -      i+=snprintf(&str[i],size-i,"IMA ADPCM "); break;
> +      i+=snprintf(&str[i],size-i,"IMA-ADPCM "); break;
>      default:

that shouldn't be there I guess...

> +    if ((data->format == AF_FORMAT_FLOAT_NE) &&
> +	(af->data->format == AF_FORMAT_S16_NE))
> +    {
> +	af_msg(AF_MSG_VERBOSE,"[format] Accelerated %sto %sconversion\n",

missing space before to ;-)

>        bps=4;
> +
> +    format |= (bps-1)<<3; // hack

As you already have a comment here enhance it to say what this is
supposed to do..

> +static af_data_t* play_swapendian(struct af_instance_s* af, af_data_t* data)
> +{
> +  af_data_t*   l   = af->data;	// Local data
> +  af_data_t*   c   = data;	// Current working data
> +  int 	       len = c->len/c->bps; // Lenght in samples of current audio block
> +
> +  if(AF_OK != RESIZE_LOCAL_BUFFER(af,data))
> +    return NULL;
> +
> +  endian(c->audio,l->audio,len,c->bps);

endian still contains a big switch ;-)

> +
> +  c->audio = l->audio;
> +
> +  return c;
> +}

You probably must set c->format, too...

> +
> +static af_data_t* play_float2s16(struct af_instance_s* af, af_data_t* data)
> +{
> +  af_data_t*   l   = af->data;	// Local data
> +  af_data_t*   c   = data;	// Current working data
> +  int 	       len = c->len/4; // Lenght in samples of current audio block
> +
> +  if(AF_OK != RESIZE_LOCAL_BUFFER(af,data))
> +    return NULL;
> +
> +  // i think this is safe as reading in the buffer is always 2 bytes ahead
> +  float2int(c->audio, l->audio, len, 2);
> +
> +  c->audio = l->audio;
> +  c->len = len*2;
> +  c->bps = 2;
> +
> +  return c;
> +}

Same here...

Greetings,
Reimar Döffinger