[MPlayer-dev-eng] Too big buffer in "libmpcodecs/ad_pcm.c"?

[Reimar Döffinger]

Hello,
On Mon, Aug 14, 2006 at 08:17:30PM +0300, Uoti Urpala wrote:
> On Mon, 2006-08-14 at 13:02 +0200, Reimar Döffinger wrote:
> > Are you really sure that audio_out->get_space() can't be larger than
> > sh_audio->a_out_buffer_size?
> 
> Nothing prevents aos from returning huge values (though it could cause
> some problems). However such an assumption isn't implied by what I said
> above. It's enough to assume that maxlen >= minlen + audio_out_minsize,
> and that should be guaranteed by the calling functions.

That's one of the biggest problems I have with MPlayer. You say this
should be guaranteed by the calling functions, but it is nowhere
documented that this should be so and why.

> >  This really seems like a non-trivial
> > assumption, and I don't really agree that simplifying
> > maxlen = maxlen - maxlen % align;
> > to
> > return -1;
> > is worth making such an assumption.
> 
> I think allowing return of partial data with some channels missing is
> more of a bug than a feature. It'll just make the problems triggered in
> filters and aos harder to diagnose.

Huh? My suggestion wouldn't cause that to happen, it would still round
correctly. And it would also same in the same way when the problem is
that the channel count is simply too large.
But it would continue to work e.g. in the case that minlen > maxlen.
Well, all those cases would be a bug I guess, but I prefer more
resilient code if it's not too complicated.

> There already are decoders which rely on that assumption, and I think
> they shouldn't all need extra checks. They're all called through
> dec_audio.c, having the check there is better. In fact there already is
> a check but it doesn't quite match what is actually required, I'll fix
> it.

I don't disagree, but I'd like to point out that this is the usual case:
If checks and the code assuming the checks are far apart, nobody in the
end cares to check if the checks and the assumptions actually match...
I feel very uneasy around (esp. undocumented) assumptions in code.

Greetings,
Reimar Döffinger