On Wed, Aug 13, 2014 at 02:30:20PM +0200, James Darnley wrote:
> On 2014-08-13 14:05, Michael Niedermayer wrote:
> > On Wed, Aug 13, 2014 at 10:46:45AM +0200, James Darnley wrote:
> >> On 2014-08-13 04:50, Michael Niedermayer wrote:
> >>> On Tue, Aug 12, 2014 at 11:22:07PM +0200, James Darnley wrote:
> >>>> ---
> >>>> libavcodec/flacdsp.h | 7 +++++++
> >>>> 1 files changed, 7 insertions(+), 0 deletions(-)
> >>>>
> >>>> diff --git a/libavcodec/flacdsp.h b/libavcodec/flacdsp.h
> >>>> index 272cf2a..36cd904 100644
> >>>> --- a/libavcodec/flacdsp.h
> >>>> +++ b/libavcodec/flacdsp.h
> >>>> @@ -27,6 +27,13 @@ typedef struct FLACDSPContext {
> >>>> int len, int shift);
> >>>> void (*lpc)(int32_t *samples, const int coeffs[32], int order,
> >>>> int qlevel, int len);
> >>>> + /**
> >>>> + * This function has some limitations with various configurations:
> >>>> + * - when CONFIG_SMALL is 0 there is an unrolled loop which assumes
> >>>> the
> >>>> + * maximum value of order is 32.
> >>>> + * - when SSE4 (or newer) is available on x86 there is an unrolled
> >>>> copy
> >>>> + * which also assumes the maximum value of order is 0.
> >>>> + */
> >>>
> >>> sounds like
> >>>
> >>> printf()
> >>> on fridays with SSE4 this is limited to 27 characters
> >>>
> >>> a function either should have a limit or not have one, it should
> >>> not depend on other factors
> >>>
> >>> People using this function must be able to tell in what cases they
> >>> can use it
> >>>
> >>> and People optimizing the function need to know which cases their
> >>> optimized code must support
> >>>
> >>> the API defines both
> >>
> >> I don't get it. FLAC only allows upto 32-order LPC. This was,
> >> apprarently, an acceptable assumption to make for the unrolled C code
> >> yet somehow I can't make the same assumption for assembly.
> >
> > theres some kind of misunderatanding here
> >
> > its perfectly fine to say
> >
> > /**
> > * This function supports upto a order of 32
> > */
> >
> > what i think is a really bad idea is to make the API conditional
> > on cpu features and build flags.
> > What if someone wants to add a SSE2 optimization that works just up
> > to order 32 ? he cannot do it without changing the API and checking
> > that all uses of the API are safe with the new limitation
>
> Okay I understand that.
>
> I thought that if someone wanted to re-use the function in some other
> encoder which might allow 64 order (for example), I should document
> where the limitations are.
> > I can change the patch to state that it supports up to 32 but should I > also still mention where that is assumed? Its ok to mention but the difference between API = interface specification and interface implementation should be clear > > What about Christophe's suggestion of changing the function definition > by using "int coeffs[32]"? thats a good idea too [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Complexity theory is the science of finding the exact solution to an approximation. Benchmarking OTOH is finding an approximation of the exact
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
