Peak-Hold Algorithmus

[Lord Z. (lordziu)]

Hallo Gemeinde.

Ich taste auf einem mega644P ein Audiosignal ab und schicke es durch die 
ElmChan-FFT. Ausgewählte Frequenzen zeige ich über LEDs an. Nun möchte 
ich ein Peak-Hold realisieren, damit die Anzeige nicht so "zuckt".

Ich habe dazu folgenden Algorithmus geschrieben, der als Parameter den 
aktuell berechneten Frequenzwert bekommt. Alle alten Werte werden um 3dB 
abgesenkt (halbiert). Den größten Wert in meinem Buffer gebe ich dann 
zurück und zeige ihn an.

#define PEAKHOLD_N  5
uint16_t peaks[PEAKHOLD_N];

/*
  Wird alle 10ms mit einem neuen FFT-Wert für
  die entsprechende Frequenz aufgerufen.
  
  Return-Wert wird über LEDs angezeigt
*/
uint16_t getNextPeak(uint16_t new_value)
{
  // shift old samples and fade them away
  for(uint8_t n=PEAKHOLD_N-1; n>0; n--)
  {
    peaks[n] = (peaks[n-1] >> 2);
  }
  
  // insert the new value
  peaks[0] = new_value;
  
  // search for max to display
  uint16_t max = 0;
  for(uint8_t n=PEAKHOLD_N-1; n>=0; n--)
  {
    if(peaks[n] > max)
      max = peaks[n];
  }

  return max;
}

Mir geht's darum, ob der Algorithmus so funktionieren kann oder ob ich 
völlig auf dem Holzweg bin?
Syntaxfehler im Code interessieren mich jetzt erstmal nicht.

Grüße

[Karl H. (kbuchegg)]

Daniel R. schrieb:

> uint16_t getNextPeak(uint16_t new_value)
> {
>   // shift old samples and fade them away
>   for(uint8_t n=PEAKHOLD_N-1; n>0; n--)
>   {
>     peaks[n] = (peaks[n-1] >> 2);

Das ist eine Division durch 4 und nicht durch 2
Wenn du durch 2 dividieren willst, dann schreib auch Division durch 2! 
Dein Compiler weiß schon, wann er das mit Schieben erledigen kann und 
wann er tatsächlich dividieren muss.


Ich würds so machen:
Wenn der neue Werte größer ist als der alte, dann übernimm ihn direkt. 
Ist er kleiner, dann wird der alte Wert einfach nur um 1 verringert. 
Wenn das je nach Wertebereich zu langsam wieder runter fällt, dann eben 
2 abziehen (oder halbieren oder ...)

static uint16_t oldPeak;

uint16_t getNextPeak(uint16_t new_value)
{
  if( new_value > oldPeak )
    oldPeak = new_value;

  else if( oldPeak > 0 )
    oldPeak--;

  return oldPeak;
}

Ziel: ansteigende Werte werden sofort übernommen. Fällt der Wert, dann 
tut er das langsam.

[Karl H. (kbuchegg)]

Daniel R. schrieb:

>   // search for max to display
>   uint16_t max = 0;
>   for(uint8_t n=PEAKHOLD_N-1; n>=0; n--)
>   {
>     if(peaks[n] > max)
>       max = peaks[n];
>   }
>
>   return max;
> }
> [/c]
>
> Mir geht's darum, ob der Algorithmus so funktionieren kann oder ob ich
> völlig auf dem Holzweg bin?

Probiers aus.
Schreib ein Programm, welches eine repräsentatives Samplemenge durch die 
Funktion durchjagt und sieh dir die Ergebnisse an. Letztendlich hängt es 
vom Effekt ab, den du erzielen willst, ob der Code das macht, was du dir 
vorstellst.

[Lord Z. (lordziu)]

Karl heinz Buchegger schrieb:
> Das ist eine Division durch 4 und nicht durch 2
>
> Wenn du durch 2 dividieren willst, dann schreib auch Division durch 2!

Ich meinte eine Division durch 2, also >>1.

>
> Ich würds so machen:
> Wenn der neue Werte größer ist als der alte, dann übernimm ihn direkt.
> Ist er kleiner, dann wird der alte Wert einfch nur um 1 verringert. Wenn
> das je nach Wertebereich zu langsam wieder runter fällt, dann eben 2
> abziehen

Das ist natürlich die einfachere Methode, da hab ich wieder zu 
kompliziert gedacht ^^.

Ich kanns leider erst morgen testen, weil ich die HW nicht hier hab. 
Hatte mir das nur theoretisch überlegt und wollte mal Meinungen dazu 
hören.

Danke

[Karl H. (kbuchegg)]

Daniel R. schrieb:
> Karl heinz Buchegger schrieb:
>> Das ist eine Division durch 4 und nicht durch 2
>>
>> Wenn du durch 2 dividieren willst, dann schreib auch Division durch 2!
>
> Ich meinte eine Division durch 2, also >>1.

Dann schreibs auch als Division durch 2

    peaks[n] = peaks[n-1] / 2;

und alle sind glücklich. Da gibt es kein Vertun und man sieht was Sache 
ist.

[Karl H. (kbuchegg)]

Daniel R. schrieb:

> Das ist natürlich die einfachere Methode, da hab ich wieder zu
> kompliziert gedacht ^^.

Muss nicht sein.

Das ergibt einen anderen Effekt. Vielleicht willst du den ja gar nicht 
haben.

Hier gibt es kein richtig und kein falsch. Was funktioniert und was dir 
optisch gefällt ist per Definition richtig :-)

[Besserwisser (Gast)]

Dieses Shiften im Quellcode kannst du dir sparen, ob du >> 2 oder / 4 
schreibst ist das gleiche. Vertraue deinem Compiler, das er intelligent 
genug ist, das effektivste daraus zu machen.

[Lord Z. (lordziu)]

Besserwisser schrieb:
> Dieses Shiften im Quellcode kannst du dir sparen, ob du >> 2 oder / 4
> schreibst ist das gleiche. Vertraue deinem Compiler, das er intelligent
> genug ist, das effektivste daraus zu machen.

Wahrscheinlich werden gleich alle über mich herfallen, aber ich hatte 
das vor ein paar Wochen mit der neuesten Kombination WinAVR / AVR-Studio 
getestet. Und da wurde meines Erachtens nicht vom Compiler optimiert.

Als ich da ne Division durch z.B. 64 durch shiften ersetzt hatte, habe 
ich deutlich schnelleren Code erhalten. Steht ja auch so in diversen 
Optimierungs-Guides. Ich war auch der Meinung, dass der Compiler sowas 
machen müsste. Vielleicht liegts ja auch an Einstellungen, ich 
compiliere immer mit -Os.

[Karl H. (kbuchegg)]

Daniel R. schrieb:

> Als ich da ne Division durch z.B. 64 durch shiften ersetzt hatte, habe
> ich deutlich schnelleren Code erhalten. Steht ja auch so in diversen
> Optimierungs-Guides. Ich war auch der Meinung, dass der Compiler sowas
> machen müsste. Vielleicht liegts ja auch an Einstellungen, ich
> compiliere immer mit -Os.

Das kann dann nur ein Fehler im Optimizer des Compilers gewesen sein.
Diese Optimierung ist eine der leichtesten Übungen für einen Compiler 
die sie seit mehr als 40 Jahren anstandslos beherrschen.

Wenn dem tatsächlich so war, dann ist das ein Fall für den Bug Report.