On Tue, Apr 04, 2017 at 06:13:30PM -0400, Ronald S. Bultje wrote: > Hi, > > On Tue, Apr 4, 2017 at 5:52 PM, Michael Niedermayer <mich...@niedermayer.cc> > wrote: > > > On Tue, Apr 04, 2017 at 12:48:17PM -0400, Ronald S. Bultje wrote: > > > These use the mmx IDCT, but sse2 put/add_pixels_clamped implementations. > > > This way we don't need to use the ff_put/add_pixels_clamped function > > > pointers. > > > --- > > > libavcodec/x86/idctdsp_init.c | 10 ++++++++++ > > > libavcodec/x86/simple_idct.c | 15 +++++++++++++-- > > > libavcodec/x86/simple_idct.h | 3 +++ > > > 3 files changed, 26 insertions(+), 2 deletions(-) > > > > > > diff --git a/libavcodec/x86/idctdsp_init.c > > b/libavcodec/x86/idctdsp_init.c > > > index bcf7e5b..579c5e7 100644 > > > --- a/libavcodec/x86/idctdsp_init.c > > > +++ b/libavcodec/x86/idctdsp_init.c > > > @@ -87,6 +87,16 @@ av_cold void ff_idctdsp_init_x86(IDCTDSPContext *c, > > AVCodecContext *avctx, > > > } > > > > > > if (ARCH_X86_64 && avctx->lowres == 0) { > > > + if (!high_bit_depth && > > > > > + avctx->lowres == 0 && > > > > this looks redundant > > > > otherwise should be ok > > > > thx > > > Thanks. > > While looking, this patch may actually have a slightly theoretical issue. > Imagine that you're on a configuration where external asm is disabled but > inline asm is enabled. In our current system, > put/add_pixels_clamped_mmx/sse2 are yasm functions, but idct() is still > inline. So in this configuration, idct() is available, but idct_add/put are > not. >
> What do we set idct_permutation to? idct_permutation must match the idcts, the idcts must match each other permutation wise for internal / extern, we could sidestep this by using a internal clamped with internal idcts > (I know that in practice this configuration is basically a bug and so the > performance implications aren't that important, but I do believe it should > not crash; should we just move the assignment of idct under > HAVE_EXTERNAL_MMX - which is where idct_put/add would go - even though it > strictly speaking doesn't use external asm?) > if the idct depends on external asm then it would need to be under HAVE_EXTERNAL_*, you are correct. Also that is ugly [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Avoid a single point of failure, be that a person or equipment.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel