Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libdisplay-info fixes #457

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

libdisplay-info fixes #457

wants to merge 5 commits into from

Conversation

emersion
Copy link

@emersion emersion commented Nov 23, 2024

A bunch of fixes to make owi work with libdisplay-info.

Test with:

owi c -I include/ *.c di-edid-decode/*.c

It reached the owi_assert(0) in the patch below after 27 minutes of execution time.

libdisplay-info patch
diff --git a/di-edid-decode/main.c b/di-edid-decode/main.c
index bb3d9e7e0999..78d3265548a4 100644
--- a/di-edid-decode/main.c
+++ b/di-edid-decode/main.c
@@ -3,20 +3,22 @@
 #include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <getopt.h>
+//#include <getopt.h>
 
 #include <libdisplay-info/displayid.h>
 #include <libdisplay-info/edid.h>
 #include <libdisplay-info/info.h>
 
+#include <owi.h>
+
 #include "di-edid-decode.h"
 
 struct uncommon_features uncommon_features = {0};
 
-static const struct option long_options[] = {
+/*static const struct option long_options[] = {
 	{ "help", no_argument, 0, 'h' },
 	{ 0, 0, 0, 0 }
-};
+};*/
 
 static void usage(void)
 {
@@ -95,7 +97,8 @@ main(int argc, char *argv[])
 	in = stdin;
 	while (1) {
 		int option_index = 0;
-		opt = getopt_long(argc, argv, "h", long_options, &option_index);
+		//opt = getopt_long(argc, argv, "h", long_options, &option_index);
+		opt = getopt(argc, argv, "h");
 		if (opt == -1)
 			break;
 
@@ -117,7 +120,7 @@ main(int argc, char *argv[])
 		}
 	}
 
-	while (!feof(in)) {
+	/*while (!feof(in)) {
 		size += fread(&raw[size], 1, sizeof(raw) - size, in);
 		if (ferror(in)) {
 			perror("fread failed");
@@ -128,7 +131,12 @@ main(int argc, char *argv[])
 		}
 	}
 
-	fclose(in);
+	fclose(in);*/
+
+	size = 128;
+	for (size_t i = 0; i < size; i++) {
+		raw[i] = owi_char();
+	}
 
 	info = di_info_parse_edid(raw, size);
 	if (!info) {
diff --git a/edid.c b/edid.c
index 2383fa91e9f2..716d46f1ff41 100644
--- a/edid.c
+++ b/edid.c
@@ -4,6 +4,8 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include <owi.h>
+
 #include "bits.h"
 #include "dmt.h"
 #include "edid.h"
@@ -337,9 +339,9 @@ parse_standard_timing(struct di_edid *edid,
 		return true;
 	}
 	if (data[0] == 0x00) {
-		add_failure_until(edid, 4,
+		/*add_failure_until(edid, 4,
 				  "Use 0x0101 as the invalid Standard Timings code, not 0x%02x%02x.",
-				  data[0], data[1]);
+				  data[0], data[1]);*/
 		return true;
 	}
 
@@ -1275,6 +1277,8 @@ _di_edid_parse(const void *data, size_t size, FILE *failure_msg_file)
 		}
 	}
 
+	owi_assert(0);
+
 	for (i = 0; i < EDID_BYTE_DESCRIPTOR_COUNT; i++) {
 		byte_desc_data = (const uint8_t *) data
 			       + 0x36 + i * EDID_BYTE_DESCRIPTOR_SIZE;

I also had to add #define __ASSERT_FUNCTION NULL otherwise it would fail compilation because it's undefined.

Comment on lines +56 to +57
FILE memstream = {0};
FILE *open_memstream(char **ptr, size_t *sizeloc) {
return &memstream;
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally I tried to write this as:

FILE *open_memstream(char **ptr, size_t *sizeloc) {
  static FILE f = {0};
  return &f;
}

but this caused some runtime memory owi errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @filipeom, this is where we got a "cannot fetch base pointer" error.

double sqrt(double x) { return x; }
float roundf(float x) { return x; }
float log2f(float x) { return x; }
float sqrtf(float x) { return x; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filipeom I believe we could map many of those directly to corresponding Wasm instructions, right? I'm thinking of stuff like {f32,f64}.{floor,ceil,sqrt}.

@zapashcanon
Copy link
Member

Thanks for the various patches!

It reached the owi_assert(0) in the patch below after 27 minutes of execution time.

Great! This is a little bit long. I'm going to see if I can make this faster. Have you tried to push it further ?

@zapashcanon
Copy link
Member

+	size = 128;

I have a question regarding the initial size. 128 is the maximum allowed size, right? What is the minimum one? It could help a lot to use a smaller one for my investigation in order to reduce execution time.

@emersion
Copy link
Author

128 is the minimum valid size for an EDID blob. To expand on this a bit: an EDID blob is made up of 1 or more blocks, and each block takes 128 bytes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants