Hi, On Wed, Sep 26, 2012 at 11:00 AM, Diego Biurrun <di...@biurrun.de> wrote: > On Tue, Sep 11, 2012 at 10:29:08AM +0200, Diego Biurrun wrote: >> On Thu, Sep 06, 2012 at 11:42:45AM +0100, Måns Rullgård wrote: >> > "Ronald S. Bultje" <rsbul...@gmail.com> writes: >> > > On Wed, Sep 5, 2012 at 9:12 PM, Diego Biurrun <di...@biurrun.de> wrote: >> > >> ff_get_cpu_flags_x86() requires cpuid(), which is conditionally defined >> > >> elsewhere in the file. Surrounding the function body with ifdefs allows >> > >> building even when cpuid is not defined. An empty cpuflags mask is >> > >> returned in this case. >> > >> --- >> > >> libavutil/x86/cpu.c | 4 ++++ >> > >> 1 files changed, 4 insertions(+), 0 deletions(-) >> > >> > What configuration does this affect? >> >> --disable-inline-asm, see my FATE instance with that flag. >> >> > > I don't like this. I remember someone not testing his cpumask patch a >> > > while ago, causing all of fate to run (silently, and undetected for a >> > > good hour) in pure C on all x86 machines. >> > >> > That mistake had nothing to do with this patch. >> > >> > > Running in pure C should not go undetected and especially not silent. >> > > I prefer if this fails to compile, so we can _fix_ it instead of >> > > pretending to support a machine that we don't. For such machines, >> > > --disable-mmx or similar is good enough as a hack, right? I'm fine >> > > with some kind of a very loud warning also. >> > >> > There is currently only a problem if building with a compiler that >> > supports neither inline asm nor cpuid intrinsics. I don't know of such >> > a compiler. >> >> Ronald, do you still object to this patch? I believe Mans summed up >> that there should not be the risk you fear and it will fix a FATE >> instance. > > ping
I asked that a warning be added to either configure output or to runtime (similar to the warning if you don't specifically disable asm, but yasm wasn't found) that you'll get useless performance. Maybe link it so that an explicit --disable-asm or --disable-inline-asm is required, and it's not possible for HAVE_YASM as well as HAVE_INLINE_ASM to both be disabled without specifically being told to do so on the configure commandline. Ronald _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel
