[MPlayer-dev-eng] [PATCH] revert CPU speed detection

Diego Biurrun diego at biurrun.de
Thu Oct 7 11:31:47 CEST 2004


D Richard Felker III writes:
> On Thu, Oct 07, 2004 at 04:34:50AM +0200, Diego Biurrun wrote:
> > 
> > I've had a look at a way to make the CPU speed detection run only in
> > verbose mode but then reconsidered.  Speed detection has never worked
> > reliably and slows down startup noticeably.  Thus I propose the
> > following patch that rips out CPU speed detection by reverting the
> > commit from Atmos that added it:
> > 
> > http://mplayerhq.hu/pipermail/mplayer-cvslog/2003-September/016619.html
> > 
> > What do you think?
> 
> i'm against applying this patch as-is. it's just a reversed version of
> the original patches. granted those seem to have had some cosmetics,
> but reverting that now is just more cosmetics.

Not reverting the cosmetics defeats the purpose of forbidding
cosmetics in the first place.  No cosmetics lead to cleaner CVS diffs,
if you keep the cosmetics now, the diffs remain unreadable, so what
are we supposed to gain by not removing the cosmetics?

> also there's some useful stuff, like the hasTSC flag, which
> shouldn't be reverted, and it's especially bad to change this back
> in cpudetect.h which is otherwise untouched by this patch.

It's used nowhere else according to grep, neither are the other
cpuspeed detection functions.

I'm not against cherry picking the good parts out of the speed
detection code.  IMO this should be done in a separate patch, though.
That patch will be clean and easy to verify just as this one.  This
would also separate each logical step into one commit as it should be
done.  One cleanly backs out speed detection, one small patch adds TSC
detection.

> i vote for just removing the speed detection code itself, and none of
> the other reversals. that makes the new patch much cleaner and easy to
> tell what it does (and clear that there are no bugs).

The patch is clean and easy to verify.  How can I make such bold
claims?  I did not create the patch myself, CVS did it for me.

Just look at the "cvs diff" headers:

The first hunk was created by

  cvs diff -r 1.31 -r 1.29 cpudetect.c

Index: cpudetect.c
===================================================================
RCS file: /cvsroot/mplayer/main/cpudetect.c,v
retrieving revision 1.31
retrieving revision 1.29
diff -u -r1.31 -r1.29
--- cpudetect.c	28 Sep 2003 01:45:54 -0000	1.31
+++ cpudetect.c	6 Sep 2003 12:06:59 -0000	1.29

The second hunk was created by

  cvs diff -r 1.10 -r 1.9 cpudetect.h

Index: cpudetect.h
===================================================================
RCS file: /cvsroot/mplayer/main/cpudetect.h,v
retrieving revision 1.10
retrieving revision 1.9
diff -u -r1.10 -r1.9
--- cpudetect.h	19 Sep 2003 23:52:41 -0000	1.10
+++ cpudetect.h	18 Jan 2003 19:29:45 -0000	1.9

So all you have to verify is that I used the right commands and that I
have really posted the output of those commands.

Am I overlooking something?

Diego




More information about the MPlayer-dev-eng mailing list