Fragen zu Windows Bitmap einlesen #2
-
Puh, also wieder ne große Pause ...
Danke nochmals für die Beiträge und den Beispielcode von hustbaer (den ich einfach ganz frech kopiert habe ;)). Da im Artikel steht
BMP verwendet die Little-Endian-Konvention.
dachte ich mir, darum muss ich mich nicht kümmern.
Jetzt habe ich aber das "Problem", das es gar keine negativen Werte gibt, auch bei dem Bild, wo ich vorher von ausgegangen bin. Vielleicht habe ich aber beim Schreiben des Code auch gepfuscht? Vielleicht habe ich auch was falsch verstanden?
Oder kann mir jemand ein Beispiel für einen Beispielvector in der Form des
bmp_data-vector mit negativen Werten geben, um dies zu überprüfen?struct Head //Kopf { static const auto Size = 4; std::array <std::pair <std::string, int>, Size> bType { { { "bfType", 1 }, //uint16_t { "bfSize", 2 }, //uint32_t { "bfReserved", 2 }, //uint32_t { "bfOffbits", 2 } //uint32_t } }; std::vector <int> data; } head; struct Info //Eigenschaften { static const auto Size = 11; std::array <std::pair <std::string, int>, Size> bType { { { "biSize", 2 }, //uint32_t { "biWidth", 3 }, //int32_t { "biHeight", 3 }, //int32_t { "biPlanes", 1 }, //uint16_t { "biBitCount", 1 }, //uint16_t { "biCompression", 2 }, //uint32_t { "biSizeImage", 2 }, //uint32_t { "biXPelsPerMeter", 3 }, //int32_t { "biYPelsPerMeter", 3 }, //int32_t { "biClrUsed", 2 }, //uint32_t { "biClrImportant", 2 } //uint32_t } }; std::vector <int> data; } info; /* struct ColTable //Farbtabelle { bool is_ = true; //existiert Farbtabelle std::vector <std::tuple <uint8_t, uint8_t, uint8_t, uint8_t>> tblEntry; } colTable; */ void read_bmpData (std::string file_, std::vector <unsigned char> &bmp_data) { std::string file_name = file_ + ".bmp"; char s_char; std::ifstream file (file_name.c_str(), std::ios::binary); if (!file) { std::string err_str = "!Fehler bei " + file_name; //throw err_str; std::cout << err_str; //wird noch behandelt } while (file.get(s_char)) bmp_data.push_back(s_char); std::cout << "_File: " << file_name << '\n' << '\n'; } uint16_t make_uint16 (std::vector <unsigned char> bmp_data, std::size_t pos) { uint16_t u_val = bmp_data.at(pos + 0) + (static_cast <uint16_t> (bmp_data.at(pos + 1)) << 8); return u_val; } uint32_t make_uint32 (std::vector <unsigned char> bmp_data, std::size_t pos) { uint32_t u_val = bmp_data.at(pos + 0) + (static_cast <uint32_t> (bmp_data.at(pos + 1)) << 8) + (static_cast <uint32_t> (bmp_data.at(pos + 2)) << 16) + (static_cast <uint32_t> (bmp_data.at(pos + 3)) << 24); return u_val; } int32_t make_int32 (std::vector <unsigned char> bmp_data, std::size_t pos) { uint32_t u_val = bmp_data.at(pos + 0) + (static_cast <uint32_t> (bmp_data.at(pos + 1)) << 8) + (static_cast <uint32_t> (bmp_data.at(pos + 2)) << 16) + (static_cast <uint32_t> (bmp_data.at(pos + 3)) << 24); int32_t s_val; if ((u_val >> 31) & 1) { s_val = - static_cast <int32_t> (~u_val) - 1; } else { s_val = u_val; } return s_val; } int main() { //std::string file_ = "test_me"; std::string file_ = "WOLKEN"; //std::string file_ = "046"; //std::string file_ = "Cubis"; std::vector <unsigned char> bmp_data; read_bmpData (file_, bmp_data); std::size_t offset = 0; uint16_t data_u16; uint32_t data_u32; int32_t data_32; for (auto i=0; i<head.Size; i++) { switch (head.bType.at(i).second) { case 1: data_u16 = make_uint16 (bmp_data, offset); head.data.push_back(data_u16); offset += 2; break; case 2: data_u32 = make_uint32 (bmp_data, offset); head.data.push_back(data_u32); offset += 4; break; case 3: data_32 = make_int32 (bmp_data, offset); head.data.push_back(data_32); offset += 4; break; } } for (auto i=0; i<info.Size; i++) { switch (info.bType.at(i).second) { case 1: data_u16 = make_uint16 (bmp_data, offset); info.data.push_back(data_u16); offset += 2; break; case 2: data_u32 = make_uint32 (bmp_data, offset); info.data.push_back(data_u32); offset += 4; break; case 3: data_32 = make_int32 (bmp_data, offset); info.data.push_back(data_32); offset += 4; break; } } for (std::size_t i=0; i<head.Size; i++) std::cout << head.bType.at(i).first << ": " << head.data.at(i) << '\n'; std::cout << '\n'; for (std::size_t i=0; i<info.Size; i++) std::cout << info.bType.at(i).first << ": " << info.data.at(i) << '\n'; }edit: einige Korrekturen im Code
-
lemon03 schrieb:
... int32_t s_val; if ((u_val >> 31) & 1) { s_val = -u_val; // <-------------------- } else { s_val = u_val; } ...Hast du in die markierte Zeile mal nen Breakpoint gemacht?
Und: Wieso hast du den Code in genau dieser Zeile geändert? Das hat schon nen Grund dass ich das so geschrieben habe wie ich es eben geschrieben habe.
-
Einen Breakpoint nicht (ich kann leider noch nicht in CodeBlocks den Debugger benutzen, weil ich mich noch nicht ganz mit
ERROR: You need to specify a debugger program in the debuggers's settings.auseinander gesetzt habe), aber ich habe eine cout-Ausgabe dort eingeführt, ob diese Wahl überhaupt getroffen wurde. Sie wurde nie aufgerufen.Die Zeilen habe ich geändert, weil ich davon ausgegangen bin, das die dortige Abfrage
if ((u_val >> 31) & 1)nur zur Entscheidung negativ oder positiv dient. Die weitere Verarbeitung wäre dann nur für eventuell andere Systeme. Dachte ich jedenfalls.Ich werd das mal korrigieren.
EDIT: Aber auch umgeschrieben, scheint es keine negativen Werte zu geben
uint32_t u_val = bmp_data.at(pos + 0) + (static_cast <uint32_t> (bmp_data.at(pos + 1)) << 8) + (static_cast <uint32_t> (bmp_data.at(pos + 2)) << 16) + (static_cast <uint32_t> (bmp_data.at(pos + 3)) << 24); int32_t s_val; if ((u_val >> 31) & 1) { s_val = - static_cast <int32_t> (~u_val) - 1; } else { s_val = u_val; } return s_val;Aber vielleicht ist das ja korrekt? Ich bräuchte zum Testen ein Beispiel, wo ganz bestimmt negative Werte enthalten sind?
-
EDIT: Korrektur ist im oberen Beitrag.
-
OK wenn das cout nie anschlägt dann wird es wohl keine negativen Werte gegeben haben in deinen Bitmaps.
lemon03 schrieb:
Aber vielleicht ist das ja korrekt? Ich bräuchte zum Testen ein Beispiel, wo ganz bestimmt negative Werte enthalten sind?
Ja korrekt ist es schon. Es gibt halt top-down und bottom-up BMPs. Standard ist bottom-up und bei denen ist die Höhe positiv.
D.h. du musst dir irgendwoher top-down BMPs zum Testen besorgen.
-
Jo danke dann
Und gute Idee, mal schauen ob man nach top-down BMPs goglen kannn, bzw was findet.
-
Super, hatte mehrere top-down Beispiele gefunden und bei allen wurde der korrekte Wert negativ angezeigt. Danke nochmals.
-
Na super
Nochwas...
Ich würde folgende Änderungen vorschlagen:int32_t make_int32 (std::vector <unsigned char> bmp_data, std::size_t pos) { uint32_t u_val = bmp_data.at(pos + 0) + (static_cast <uint32_t> (bmp_data.at(pos + 1)) << 8) + (static_cast <uint32_t> (bmp_data.at(pos + 2)) << 16) + (static_cast <uint32_t> (bmp_data.at(pos + 3)) << 24); int32_t s_val; if ((u_val >> 31) & 1) { s_val = - static_cast <int32_t> (~u_val) - 1; } else { s_val = u_val; } return s_val; } // => int32_t better_make_int32 (std::vector <unsigned char> const& /* 1 */ bmp_data, std::size_t pos) { uint32_t const /* 2 */ u_val = /* 3 */ make_uint32(bmp_data, pos); if ((u_val >> 31) & 1) { return - static_cast <int32_t> (~u_val) - 1; /* 4 */ } else { return u_val; /* 4 */ } }1: Du solltest nicht den ganzen std::vector kopieren. Das ist sogar im günstigsten Fall (wenig Daten) schon viel langsamer als nötig (dynamische Speicheranforderung!). Und wenn mal viel Daten drinnen wird's richtig langsam.
2: Lokale Variablen
constmachen wenn man sie (einfach, ohne grosse Umwege)constmachen kann. Damit man gleich sieht dass der Wert der Variable sich nach der Initialisierung nie mehr ändert.3: Code-Duplizierung vermeiden.
4: Mutable-State vermeiden, uninitialisierte Variablen vermeiden.
Uninitialisierte Variablen vermeiden allerdings nur, wenn man ihnen gleich bei der Initialisierung einen sinnvollen Wert geben kann. Wenn wirklich nur die Wahl besteht zwischen "uninitialisiert" oder "mit Dummy-Wert initialisiert", dann eher uninitialisiert. Meist kann man aber gleich mit dem passenden Wert initialisieren.Einige dieser Dinge (z.B. (1)) betreffen auch andere Funktionen, dort natürlich ebenso nachziehen.
Und wie bei allen Richtlinien gibt es natürlich Ausnahmen. Viel Aufwand zu betreiben um eine dieser Regeln zu befolgen ist nicht immer sinnvoll.
----
Und ich würde vorschlagen die Leerzeichen vor öffnenden Klammern und nach schliessenden Klammern weg zu lassen. Ausnahme: zwischen Keywords und runde Klammer kommt ein Leerzeichen
Also z.B.int x = MeineFunktion(param); // nach MeineFunktion nicht for (int i = 0; i < x; i++) // nach for schon { if (static_cast<int32_t>(i * i) < x) // nach if auch, nach static_cast aber nichtusw.
Das, also wo bei Klammern Leerzeichen gesetzt werden, ist nämlich eine der wenigen Regeln wo sich die meisten Programmierer einig sind. Zumindest ist das mein Eindruck anhand des Codes den ich so sehe.
EDIT: Copy/Paste/Edit Fehler korrigiert.
-
Und wenn wir schon dabei sind, gleich nochmal weiter:
int32_t int32_from_uint32(uint32_t u_val) { if ((u_val >> 31) & 1) return -static_cast<int32_t>(~u_val) - 1; else return u_val; } int32_t make_int32(std::vector <unsigned char> const& bmp_data, std::size_t pos) { return int32_from_uint32(make_uint32(bmp_data, pos)); }EDIT: Copy/Paste/Edit Fehler korrigiert.
-
Gleichheitszeichen zwischen
returnund der Variablen finde ich wiederum merkwürdig - mein Compiler auch...
-
war bestimmt nur ein Tippfehler ...
Ok, danke für die Hinweise. Wenn ich richtig verstanden habe, müssten die drei Funktionen dann so aussehen?:
uint16_t make_uint16(std::vector <unsigned char> const &bmp_data, std::size_t pos) { uint16_t const u_val = bmp_data.at(pos + 0) + (static_cast<uint16_t>(bmp_data.at(pos + 1)) << 8); return u_val; } uint32_t make_uint32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { uint32_t const u_val = bmp_data.at(pos + 0) + (static_cast<uint32_t>(bmp_data.at(pos + 1)) << 8) + (static_cast<uint32_t>(bmp_data.at(pos + 2)) << 16) + (static_cast<uint32_t>(bmp_data.at(pos + 3)) << 24); return u_val; } int32_t make_int32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { uint32_t const u_val = make_uint32 (bmp_data, pos); if ((u_val >> 31) & 1) { return -static_cast<int32_t>(~u_val) - 1; } else { return u_val; } }Nur das doppelt ausschreiben in Deinem letzten Beitrag habe ich nicht ganz verstanden?
---
Ich hoffe, das wird jetzt nicht zuviel, aber ich hätte noch ein anderes Anliegen. Und zwar bin ich mit meiner switch-Lösung nicht ganz zufrieden. Ich würde lieber einen Funktionszeiger nehmen, bekomme das aber nicht hin.
Vorgestellt habe ich mir, das hier der second-Wert das Argument für den Zeiger sein soll. Welcher Typ das sein muss, ist mir unklar.
struct Head //Kopf { static const auto Size = 4; std::array <std::pair <std::string, int>, Size> bType { { { "bfType", 1 }, //uint16_t { "bfSize", 2 }, //uint32_t { "bfReserved", 2 }, { "bfOffbits", 2 } } }; std::array <int, Size> data; } head; //int (*make_data)(std::vector <unsigned char>, std::size_t);Unklar ist mir auch, wie eine Funktion mit diesem Argument dann aufgerufen wird. Ich hatte bisher nur einmal mit Funktionszeigern zu tun, mir fehlt die Übung und ich kann den ersten Einsatz nicht für diesen Fall nachvollziehen.
-
wob schrieb:
Gleichheitszeichen zwischen
returnund der Variablen finde ich wiederum merkwürdig - mein Compiler auch...Danke, hab's korrigiert.
-
lemon03 schrieb:
(...)müssten die drei Funktionen dann so aussehen?:
uint16_t make_uint16(std::vector <unsigned char> const &bmp_data, std::size_t pos) { uint16_t const u_val = bmp_data.at(pos + 0) + (static_cast<uint16_t>(bmp_data.at(pos + 1)) << 8); return u_val; } uint32_t make_uint32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { uint32_t const u_val = bmp_data.at(pos + 0) + (static_cast<uint32_t>(bmp_data.at(pos + 1)) << 8) + (static_cast<uint32_t>(bmp_data.at(pos + 2)) << 16) + (static_cast<uint32_t>(bmp_data.at(pos + 3)) << 24); return u_val; } int32_t make_int32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { uint32_t const u_val = make_uint32 (bmp_data, pos); if ((u_val >> 31) & 1) { return -static_cast<int32_t>(~u_val) - 1; } else { return u_val; } }Sieht gut aus würde ich sagen.
lemon03 schrieb:
Nur das doppelt ausschreiben in Deinem letzten Beitrag habe ich nicht ganz verstanden?
Keine ahnung was du meinst. Steh vermutlich grad auf dem Schlauch
-
Danke. Meinte dies:
int32_t int32_from_uint32(uint32_t u_val) { if ((u_val >> 31) & 1) return -static_cast<int32_t>(~u_val) - 1; else return u_val; } int32_t make_int32(std::vector <unsigned char> const& bmp_data, std::size_t pos) { return int32_from_uint32(make_uint32(bmp_data, pos)); }Mir scheint das da zwei Funktionsteile genommen und daraus zwei Funktionen gemacht wurden?
-
Ja, genau. Und?
-
Dann würde aus
uint32_t make_uint32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { const uint32_t u_val = bmp_data.at(pos + 0) + (static_cast<uint32_t>(bmp_data.at(pos + 1)) << 8) + (static_cast<uint32_t>(bmp_data.at(pos + 2)) << 16) + (static_cast<uint32_t>(bmp_data.at(pos + 3)) << 24); return u_val; } int32_t make_int32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { const uint32_t u_val = make_uint32(bmp_data, pos); if ((u_val >> 31) & 1) { return -static_cast<int32_t>(~u_val) - 1; } else { return u_val; } }-->
uint32_t make_uint32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { const uint32_t u_val = bmp_data.at(pos + 0) + (static_cast<uint32_t>(bmp_data.at(pos + 1)) << 8) + (static_cast<uint32_t>(bmp_data.at(pos + 2)) << 16) + (static_cast<uint32_t>(bmp_data.at(pos + 3)) << 24); return u_val; } int32_t int32_from_uint32(uint32_t u_val) { if ((u_val >> 31) & 1) return -static_cast<int32_t>(~u_val) - 1; else return u_val; } int32_t make_int32(std::vector <unsigned char> const& bmp_data, std::size_t pos) { return int32_from_uint32(make_uint32(bmp_data, pos)); }Die Aufsplittung auf zwei Funktionen verstehe ich vom Nutzen her nicht. Da die einzelnen Teile nirgendwo als in dieser Funktion gebraucht werden, finde ich das irgendwie überflüssig?
-
lemon03 schrieb:
Die Aufsplittung auf zwei Funktionen verstehe ich vom Nutzen her nicht. Da die einzelnen Teile nirgendwo als in dieser Funktion gebraucht werden, finde ich das irgendwie überflüssig?
Der Code wird übersichtlicher und du kannst die einzelnen Funktionen einfacher testen. Eine Funktion sollte für genau eine Sache verantwortlich sein, diese darf gerne klein sein.
-
Das mag allgemein so stimmen, aber doch im obigen Fall nicht?
Der erst Block mit zwei Funktionen ist doch leichter auf einen Blick zu erfassen, als der untere mit drei verschachtelten Funktionen?
Und im Programm werden drei Werte durch drei ganz einfache Funktionen bestimmt, warum sollte man diese drei einfachen Funktionen auf vier aufsplitten?
-
wob schrieb:
und du kannst die einzelnen Funktionen einfacher testen.
Da die oberen Funktionen alle voneinander abhängig sind, kann man doch gar nicht die einzelnen Funktionen testen, ohne auch die anderen zu testen?
Also warum nicht nur eine Funktion testen, statt zwei?
-
Also sorry, wollte nicht aufmüpfig erscheinen. Bin ja froh, wenn man mir hilft und erklärt.
Ich würde gerne noch mal auf den Funktionszeiger zurück kommen, ob das hier möglich und sinnvoll ist. Vorgestellt habe ich mir das mit sozusagen Pseudocode etwa so:
int (*make_dec)(std::vector <unsigned char> const, std::size_t); //hier const richtig? uint16_t m_uint16(std::vector <unsigned char> const &bmp_data, std::size_t pos) { const uint16_t u_val = bmp_data.at(pos + 0) + (static_cast<uint16_t>(bmp_data.at(pos + 1)) << 8); return u_val; } uint32_t m_uint32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { const uint32_t u_val = bmp_data.at(pos + 0) + (static_cast<uint32_t>(bmp_data.at(pos + 1)) << 8) + (static_cast<uint32_t>(bmp_data.at(pos + 2)) << 16) + (static_cast<uint32_t>(bmp_data.at(pos + 3)) << 24); return u_val; } int32_t m_int32(std::vector <unsigned char> const &bmp_data, std::size_t pos) { const uint32_t u_val = make_uint32(bmp_data, pos); if ((u_val >> 31) & 1) { return -static_cast<int32_t>(~u_val) - 1; } else { return u_val; } } struct Head //Kopf { static const auto Size = 4; std::array <std::pair <std::string, |Typ?|>, Size> bType { { { "bfType", m_uint16 }, //uint16_t { "bfSize", m_uint32 }, //uint32_t { "bfReserved", m_uint32 }, { "bfOffbits", m_uint32 } } }; std::array <int, Size> data; } head; head.data.at(i) = Funktionsaufruf mit <head.bType.at(i).second>;Ich weiß leider nicht, wie ich das korrekt ausformuliere.
