GetJPGSize Funktion (wollen wir sie verbessern?)

[MicMic]

Hallo
folgender Code gibt's im Internet. Gerne würde ich sie verbessern und genauer verstehen. So manches ist mir nicht ganz klar. Außerdem habe ich eine Prüfung mehr reingemacht, da ich ein paar JPGs habe, die auf "$C2" (siehe Code) hören.

Hier mal der Code

Delphi-Quellcode:

procedure GetJPGSize(const sFile: string; var wWidth, wHeight: word);
const
  ValidSig : array[0..1] of byte = ($FF, $D8);
  Parameterless = [$01, $D0, $D1, $D2, $D3, $D4, $D5, $D6, $D7];
var
  Sig: array[0..1] of byte;
  f: TFileStream;
  x: integer;
  Seg: byte;
  Dummy: array[0..15] of byte;
  Len: word;
  ReadLen: LongInt;
begin
  FillChar(Sig, SizeOf(Sig), #0);
  f := TFileStream.Create(sFile, fmOpenRead);
  try
    ReadLen := f.Read(Sig[0], SizeOf(Sig));
    for x := Low(Sig) to High(Sig) do
      if Sig[x] <> ValidSig[x] then
        ReadLen := 0;
      if ReadLen > 0 then
      begin
        ReadLen := f.Read(Seg, 1);
        while (Seg = $FF) and (ReadLen > 0) do
        begin
          ReadLen := f.Read(Seg, 1);
          if Seg <> $FF then
          begin
            if (Seg = $C0) or (Seg = $C1) or (Seg = $C2) then  // $C2 von mir dazugemacht.
            begin
              ReadLen := f.Read(Dummy[0], 3);  { don't need these bytes }
              wHeight := ReadMWord(f);
              wWidth := ReadMWord(f);
            end
            else
            begin
              if not (Seg in Parameterless) then
              begin
                Len := ReadMWord(f);
                f.Seek(Len - 2, 1);
                f.Read(Seg, 1);
              end
              else
                Seg := $FF;  { Fake it to keep looping. }
            end;
          end;
        end;
      end;
    finally
    f.Free;
  end;
end;

Mein "grübeln" nun.
Nach dem "Begin" wird FillChar genutzt. Wäre das hier nicht überflüssig?

Nach dem "try" ist folgende Zeile angegeben:

Code:

ReadLen := f.Read(Sig[0], SizeOf(Sig));

Da ich hier die Größe nicht ändere, würde ich kein SizeOf nutzen und auch die Variable selbst ganz normal angeben. Also ohne "[0]". Also ganz normal "...Read(Sig,2);". Die Ergebnisse sind gleich. Warum macht man hier die Angabe "[0]"? Weiter im Code findet man z.B. "f.Read(Dummy[0], 3);". Das verwirrt mich jetzt ganz. Was wird denn gelesen? 3 Bytes? Warum hat hier dann "Dummy" [0..15] Of Byte" ?. Dort im Kommentar steht außerdem, dass diese Werte nicht benötigt werden. Wieso geht man dann nicht mit TFileStream.Position oder mit Seek zu der neuen stelle im Header?
Auch würde ich (da sich dies ja wohl nicht mehr ändert) die "ValidSig" und "Parameterless" Werte direkt angeben.

Was würdet ihr denn so verbessern?

[jaenicke]

Nein, FillChar braucht man da nicht, weil Sig danach ohnehin beschrieben wird.

Mir fallen aber noch z.B. diese Punkte auf:

• Ein normaler TFileStream ist nicht optimal, besser wäre eine Pufferung z.B. durch eine MMF
• Sig als Array mit zwei Byte-Einträgen ist nicht gerade schnell. Besser wäre ein einfacher Word-Wert, der beide Bytes umfasst.
• Es sollten out- statt var-Parameter sein
• Wenn keine Größe gefunden wird, werden wWidth und wHeight nicht beschrieben, es gibt aber auch keine Fehlerbehandlung wie einen Rückgabewert als Boolean.

[Luckie]

Zitat:

Wenn keine Größe gefunden wird, werden wWidth und wHeight nicht beschrieben, es gibt aber auch keine Fehlerbehandlung wie einen Rückgabewert als Boolean.

Man könnte in diesem Fall jeweils 0 oder -1 eintragen, dann weiß man, dass keine Größe ermittelt werden konnte, da 0 oder -1 wohl unsinnige Werte sind.

[MicMic]

@jaenicke … das mit MMF ist mir noch ganz unbekannt. Las eben nur, dass es hier Sinn macht, wenn man große Daten liest.

Ich vergas noch die ReadMWord Funktion, die dabei war.

Delphi-Quellcode:

function ReadMWord(f: TFileStream): Word;
type
  TMotorolaWord = record
    case Byte of
      0: (Value: Word);
      1: (Byte1, Byte2: Byte);
  end;
var
  MW: TMotorolaWord;
begin
  { It would probably be better to just read these two bytes in normally }
  { and then do a small ASM routine to swap them.  But we aren't talking }
  { about reading entire files, so I doubt the performance gain would be }
  { worth the trouble. }
  f.read(MW.Byte2, SizeOf(Byte));
  f.read(MW.Byte1, SizeOf(Byte));
  Result := MW.Value;
end;

Eine extra Funktion wollte ich nicht und habe mir was zusammengebastelt. Was nun schneller oder langsamer ist, weiß ich jetzt nicht.
Mir ist die JPG Header Struktur sowieso noch nicht ganz klar. Es gibt da einige unterschiede.
Meine derzeitige Prozedur (siehe unten) scheint gut zu funktionieren. Mache mir nur noch Gedanken um den "While" Block. Nicht das hier (z.B. bei defekten JPG Dateien) einfach zu viel (wäre langsam) in der Datei gesucht wird und die Bildgröße gar nicht ermittelt (wäre dann nicht so schlimm) werden kann.

Delphi-Quellcode:

Procedure GetJPGSize(sFile: String; Out wWidth, wHeight: Word);
Var
  FS: TFileStream;
  BD: Byte;
  WD : Word;
  RL: LongInt;
  HW : Array[0..3] Of Byte;
  LE : Array[0..1] Of Byte;
Begin
  sFile := '\\?\'+SFile;
  wWidth := 0;
  wHeight := 0;
  FS := TFileStream.Create(sFile, fmShareDenyNone);
  try
    RL := FS.Read(WD, 2);
    If (Lo(WD) <> $FF) And (Hi(WD) <> $D8) Then RL := 0;
    If RL > 0 Then
    Begin
      RL := FS.Read(BD, 1);
      While (BD = $FF) and (RL > 0) Do
      Begin
        RL := FS.Read(BD, 1);
        If BD <> $FF Then
        Begin
          If BD In [$C0,$C1,$C2] Then
          Begin
            FS.Seek(3,1);
            FS.Read(HW,4);
            wHeight := HW[0] Shl 8 + HW[1];
            wWidth := HW[2] Shl 8 + HW[3];
          End Else
          Begin
            If Not (BD In [$01, $D0, $D1, $D2, $D3, $D4, $D5, $D6, $D7]) Then
            Begin
              FS.Read(Le,2);
              WD := Le[0] Shl 8 + Le[1];
              FS.Seek(WD - 2, 1);
              FS.Read(BD, 1);
            End Else BD := $FF;
          End;
        End;
      End;
    End;
  Finally
    FS.Free;
  End;
End;

[freimatz]

Zitat von MicMic:

Was würdet ihr denn so verbessern?

Den Code lesbarer machen
Nur ein Delphi-Profi kann den Code lesen, versteht jedoch m.E. immer noch nicht warum da was passiert.

[Luckie]

Vorallem die Magic Bytes sollte man kommentieren, damit man weiß, was da wie und warum geprüft wird.

[KodeZwerg]

Bei dem was ich auf die schnelle recherchiert habe kommt ein fast gleiches Snippet bei raus.
Wie Luckie bereits schrieb, den Magic Bytes jede Menge beachtung schenken, weswegen dieses Snippet noch ausbaufähig ist.
Geschwindigkeit scheint mir mit beiden Versuchen auf gleicher Höhe zu liegen.

Delphi-Quellcode:

function GetJpegSize(const FileName: string): TPoint;
var
  fs: TFileStream;
  SegmentPos: Integer;
  SOIcount: Integer;
  x, y: word;
  b: byte;
begin
  fs := TFileStream.Create(FileName, fmOpenRead or fmShareDenyNone);
  try
    fs.Position := 0;
    fs.Read(x, 2);
    if x <> $D8FF then
      raise Exception.Create('Not a Jpeg file');
    SOIcount := 0;
    fs.Position := 0;
    while fs.Position + 7 < fs.Size do
    begin
      fs.Read(b, 1);
      if b = $FF then
      begin
        fs.Read(b, 1);
        if b = $D8 then
          Inc(SOIcount);
        if b = $DA then
          Break;
      end;
    end;
    if b <> $DA then
      raise Exception.Create('Corrupt Jpeg file');
    SegmentPos := -1;
    fs.Position := 0;
    while fs.Position + 7 < fs.Size do
    begin
      fs.Read(b, 1);
      if b = $FF then
      begin
        fs.Read(b, 1);
        if b in [$C0, $C1, $C2] then
        begin
          SegmentPos := fs.Position;
          Dec(SOIcount);
          if SOIcount = 0 then
            Break;
        end;
      end;
    end;
    if SegmentPos = -1 then
      raise Exception.Create('Corrupt Jpeg file');
    if fs.Position + 7 > fs.Size then
      raise Exception.Create('Corrupt Jpeg file');
    fs.Position := SegmentPos + 3;
    fs.Read(y, 2);
    fs.Read(x, 2);
    Result := Point(Swap(x), Swap(y));
  finally
    fs.Free;
  end;
end;

Quelle: get-width-and-height-of-jpg-image.html

[MicMic]

Ich habe sie mal getestet.
Genau geschaut habe ich noch nicht aber ich habe jetzt eine JPG Datei mit falschen Abmessungen (256x171 anstatt 7360x4912) und eine JPG, da stimmt die Größe zwar aber gefühlte 10 Sekunden versucht er da rumzumachen. Fehler selbst "Corrupt Jpeg file" kommt nicht. Nur mal so zur Info. Zeit habe ich noch nicht gefunden, mir ein wenig mehr von der JPG Header Struktur durchzulesen.

[Luckie]

Einen TPoint als Ergebnis einer Breiten- und Höhenermittlung zurück zu geben, ist aber auch befremdlich. Ich würde in den x- und y-Koordinaten jetzt erstmal keine Dimensionen vermuten, sondern, wie der Datentyp schon impliziert, einen Punkt. Ich würde eine Funktion machen mit Höhe und Breite als var-Parameter und true oder false, im Falle eines Erfolges oder Misserfolges, zurückgeben.

[himitsu]

TSize

TSmallSize passt von den Bits her, ist aber mit Vorzeichen und Unsigned gibt es hier nicht vordefiniert.