[MPlayer-dev-eng] Faster libaf

[Alex Beregszaszi]

Hi,

> 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...

so just forget play_fixed

> > 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...

the float conversion has a job because some of the filters are float
only, and yet others are s16_ne only, so usually no signed/unsigned
conversion happens, because the most decoders output s16

> >      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...

they should, else there are too much spaces in the string and maybe you
could get in trouble while looking at it (yes, it's a cosmetics one)

> > +    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 ;-)

false, the af_fmt2str adds the trailing space

> >        bps=4;
> > +
> > +    format |= (bps-1)<<3; // hack
> 
> As you already have a comment here enhance it to say what this is
> supposed to do..

sets the appropriate AF_FORMAT_BITS

> > +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 ;-)

That's an another story, still fewer conversions..

> > +
> > +  c->audio = l->audio;
> > +
> > +  return c;
> > +}
> 
> You probably must set c->format, too...

> > +  c->audio = l->audio;
> > +  c->len = len*2;
> > +  c->bps = 2;
> > +
> > +  return c;
> > +}
> 
> Same here...

It doesn't looks like

-- 
Alex Beregszaszi 			e-mail: alex at fsn.hu
Free Software Network			cell: +36 70 3144424