Bug 127468

Comment 1 Edwin Groothuis 2008-09-18 12:50:10 UTC

Responsible Changed
From-To: freebsd-ports-bugs->freebsd-multimedia

Over to maintainer (via the GNATS Auto Assign Tool)

Comment 2 Christian Weisgerber 2009-01-12 16:15:32 UTC

State Changed
From-To: open->closed

Independent update committed.  Thanks anyway.

I have been waiting for some time for
a response to this PR, after having gone
to the trouble of submitting it, and now
I find that the port has been modified,
and no consideration seems to have been
given to my suggested changes.  In
particular, the option to use MMX on some
architectures could speed up encoding,
and this is still missing from the port.

b.

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

--- On Mon, 1/12/09, Christian Weisgerber <naddy@mips.inka.de> wrote:

> From: Christian Weisgerber <naddy@mips.inka.de>
> Subject: Re: ports/127468: [PATCH]audio/wavpack: update to 4.50.1
> To: "bf" <bf2006a@yahoo.com>
> Cc: multimedia@FreeBSD.org
> Date: Monday, January 12, 2009, 1:58 PM
> bf:
> 
> > I have been waiting for some time for
> > a response to this PR, after having gone
> > to the trouble of submitting it, and now
> > I find that the port has been modified,
> > and no consideration seems to have been
> > given to my suggested changes.
> 
> What happened was that I independently updated
> audio/wavpack and
> only afterwards accidentally noticed that there had been
> PRs open
> about it.

That's understandable, I've done it 
myself.  But I think that you should have
explained why you thought my changes
weren't worthwhile, or, better yet, asked
me about them, before closing the PR.


> 
> > In particular, the option to use MMX on some
> > architectures could speed up encoding,
> > and this is still missing from the port.
> 
> Hmm.  I've just tried this and there is *no* measurable
> speed
> difference with and without MMX, both for default encoding
> and -hh
> -x6.
> 
> hw.machine: amd64
> hw.model: AMD Athlon(tm) 64 X2 Dual Core Processor 5200+

Well, you haven't given me enough
information to explain your results.
The performance also depends on your
compiler, compiler flags, and the data
encoded.  But I'm guessing that you used
gcc > 4, which by default on amd64
exploits SIMD parallelism even when you
don't use SIMD intrinsics explicitly in
code -- and in this case you have some
effective SSE extensions in your
hardware.  Recall that the default
compiler on FreeBSD 6.x is 3.4.6, which
doesn't do this nearly as well, and 
also that not everyone using this port
has such a nice computer.

And you may not have used high-resolution
data.  As explained in wavpack's
src/pack.c, the MMX option offers an
advantage primarily for encoding of
> 16-bit audio.

My own tests on a uniprocessor 900MHz
Athlon using 24-bit WAVs at 44100kHz
and 96000kHz show that the MMX option
cuts my encoding time by 10-30%.
Therefore I think that this build option
should be exposed in the port Makefile,
whether in an OPTION or just a simple
knob, off by default. 

Regards,
          b.


> 
> -- 
> Christian "naddy" Weisgerber                     
>     naddy@mips.inka.de

 
> My own tests on a uniprocessor 900MHz
> Athlon using 24-bit WAVs at 44100kHz
> and 96000kHz show that the MMX option
> cuts my encoding time by 10-30%

I meant 44.1/96kHz.  Those would be ungodly speeds.

bf:

> Well, you haven't given me enough
> information to explain your results.
> The performance also depends on your
> compiler, compiler flags, and the data
> encoded.  But I'm guessing that you used
> gcc > 4, which by default on amd64
> exploits SIMD parallelism even when you
> don't use SIMD intrinsics explicitly in
> code -- and in this case you have some
> effective SSE extensions in your
> hardware.  Recall that the default

Yes.

> compiler on FreeBSD 6.x is 3.4.6, which
> doesn't do this nearly as well, and 

I tried building wavpack with MMX support on 6.4/i386, but cc aborts
with an internal compiler error there.

> Therefore I think that this build option
> should be exposed in the port Makefile,
> whether in an OPTION or just a simple
> knob, off by default. 

Well, we'll have to figure out where it _can_ be enabled.

        6.x     7.x     8.x
i386    no      yes     ?
amd64   ?       ?       yes

Care to check the rest of the matrix?

(This is why I'm not happy with such knobs.  The effort of supporting
them snowballs beyond the gain they offer.)

-- 
Christian "naddy" Weisgerber                          naddy@mips.inka.de

--- On Wed, 1/14/09, Christian Weisgerber <naddy@mips.inka.de> wrote:

... 

> 
> I tried building wavpack with MMX support on 6.4/i386, but
> cc aborts
> with an internal compiler error there.
> 

That's too bad.  Do you have a build log?  If so, I'd be interested to see
it.  Did you send it to the upstream maintainers?


> > Therefore I think that this build option
> > should be exposed in the port Makefile,
> > whether in an OPTION or just a simple
> > knob, off by default. 
> 
> Well, we'll have to figure out where it _can_ be
> enabled.
> 
>         6.x     7.x     8.x
> i386    no      yes     ?
> amd64   ?       ?       yes
> 
> Care to check the rest of the matrix?
> 
> (This is why I'm not happy with such knobs.  The effort
> of supporting
> them snowballs beyond the gain they offer.)
> 

I admire your thoroughness, but I think you're making it too hard for
yourself: I don't see the need to map all of this out now.  With a
non-default knob, I think we can expect (and probably will receive) help
from users trying it on their systems, and it won't affect the package
building cluster.  

I can tell you it definitely works on 8.x i386, because that's what
I'm using.  If you want to err on the side of caution, you could use
something like:

.ifdef(WITH_MMX)
.if ( ${ARCH} == "i386" && ${OSVERSION} >= 700000 )
CONFIGURE_ARGS+=	--enable-mmx

post-patch:
	@${ECHO_MSG} ""
	@${ECHO_MSG} " You have chosen to enable the use of MMX intrinsics to "
	@${ECHO_MSG} " accelerate the encoding of high resolution ( > 16 bits- "
	@${ECHO_MSG} " per-sample ) data.  Support for this non-default build "
	@${ECHO_MSG} " option will be at the discretion of individual committers. "
	@${ECHO_MSG} " Please submit bug reports to the upstream maintainers. "
	@${ECHO_MSG} ""
.else
IGNORE =	option WITH_MMX is for i386 on FreeBSD >= 7 only
.endif #ARCH, etc.
.else
CONFIGURE_ARGS+=	--disable-mmx
.endif #WITH_MMX

Otherwise, you could contact someone with a tinderbox, or turn it on
temporarily and throw it to the build cluster to get error logs.

Regards,
           b.

Comment 8 dfilter service 2009-01-19 23:31:56 UTC

naddy       2009-01-19 23:31:41 UTC

  FreeBSD ports repository

  Modified files:
    audio/wavpack        Makefile 
  Log:
  Add an OPTION to enable MMX optimizations on x86.
  Not available with GCC3, which suffers an internal compiler error.
  
  PR:             127468
  Submitted by:   bf2006a@yahoo.com
  
  Revision  Changes    Path
  1.14      +13 -1     ports/audio/wavpack/Makefile
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"