* [PATCH 0/2] edid-decode: add emscripten support
@ 2020-02-09 17:49 Ilia Mirkin
2020-02-09 17:49 ` [PATCH 1/2] edid-decode: add function to enable usage as a library Ilia Mirkin
2020-02-09 17:49 ` [PATCH 2/2] edid-decode: add build rule for emscripten output Ilia Mirkin
0 siblings, 2 replies; 7+ messages in thread
From: Ilia Mirkin @ 2020-02-09 17:49 UTC (permalink / raw)
To: linux-media, hverkuil-cisco; +Cc: Ilia Mirkin
Demo available here:
https://people.freedesktop.org/~imirkin/edid-decode/
This is enough to generate the edid-decode.{js,wasm} files, and the html
was hand-modified from the demo one to remove a bunch of unnecessary
bits and provide the required integration. Happy to move it into the
project too if desired.
I'm a complete emscripten newbie, so I'm not 100% sure I've set this up
correctly, as alluded in patch 2's comments. But it does seem to work.
Given that the patches aren't crazy, I think it's reasonable to carry
them inside of the project.
Ilia Mirkin (2):
edid-decode: add function to enable usage as a library
edid-decode: add build rule for emscripten output
Makefile | 5 +++++
edid-decode.cpp | 18 ++++++++++++++++++
2 files changed, 23 insertions(+)
--
2.24.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] edid-decode: add function to enable usage as a library
2020-02-09 17:49 [PATCH 0/2] edid-decode: add emscripten support Ilia Mirkin
@ 2020-02-09 17:49 ` Ilia Mirkin
2020-02-10 11:23 ` Hans Verkuil
2020-02-09 17:49 ` [PATCH 2/2] edid-decode: add build rule for emscripten output Ilia Mirkin
1 sibling, 1 reply; 7+ messages in thread
From: Ilia Mirkin @ 2020-02-09 17:49 UTC (permalink / raw)
To: linux-media, hverkuil-cisco; +Cc: Ilia Mirkin
This is helpful for an emscripten setup, as there's no great way to
reinitialize the object from scratch.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
edid-decode.cpp | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/edid-decode.cpp b/edid-decode.cpp
index ef50da2..fd17bc6 100644
--- a/edid-decode.cpp
+++ b/edid-decode.cpp
@@ -1005,3 +1005,21 @@ int main(int argc, char **argv)
return edid_from_file(argv[optind], argv[optind + 1], out_fmt);
return ret ? ret : state.parse_edid();
}
+
+#ifdef __EMSCRIPTEN__
+/*
+ * The surrounding JavaScript implementation will call this function
+ * each time it wants to decode an EDID. So this should reset all the
+ * state and start over.
+ */
+extern "C" int parse_edid(const char *input) {
+ for (unsigned i = 0; i < EDID_MAX_BLOCKS + 1; i++) {
+ s_msgs[i][0].clear();
+ s_msgs[i][1].clear();
+ }
+ options[OptCheck] = 1;
+ state = edid_state();
+ int ret = edid_from_file(input, NULL, OUT_FMT_DEFAULT);
+ return ret ? ret : state.parse_edid();
+}
+#endif
--
2.24.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] edid-decode: add function to enable usage as a library
2020-02-09 17:49 ` [PATCH 1/2] edid-decode: add function to enable usage as a library Ilia Mirkin
@ 2020-02-10 11:23 ` Hans Verkuil
2020-02-10 14:58 ` Ilia Mirkin
0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2020-02-10 11:23 UTC (permalink / raw)
To: Ilia Mirkin, linux-media
Hi Ilia,
I pushed my patch to initialize edid_state in the constructor to the
edid-decode git repo.
I've also split off the 'write to a file' part from edid_from_file into
a separate edid_to_file function, so you'll have to rebase your patch.
Two comments below:
On 2/9/20 6:49 PM, Ilia Mirkin wrote:
> This is helpful for an emscripten setup, as there's no great way to
> reinitialize the object from scratch.
>
> Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> ---
> edid-decode.cpp | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/edid-decode.cpp b/edid-decode.cpp
> index ef50da2..fd17bc6 100644
> --- a/edid-decode.cpp
> +++ b/edid-decode.cpp
> @@ -1005,3 +1005,21 @@ int main(int argc, char **argv)
> return edid_from_file(argv[optind], argv[optind + 1], out_fmt);
> return ret ? ret : state.parse_edid();
> }
> +
> +#ifdef __EMSCRIPTEN__
> +/*
> + * The surrounding JavaScript implementation will call this function
> + * each time it wants to decode an EDID. So this should reset all the
> + * state and start over.
> + */
> +extern "C" int parse_edid(const char *input) {
'{' should start on the next line. Let's keep the coding style consistent.
> + for (unsigned i = 0; i < EDID_MAX_BLOCKS + 1; i++) {
> + s_msgs[i][0].clear();
> + s_msgs[i][1].clear();
> + }
I'm pretty sure that this for loop is not necessary. Can you drop this
for loop and see if it works?
> + options[OptCheck] = 1;
> + state = edid_state();
> + int ret = edid_from_file(input, NULL, OUT_FMT_DEFAULT);
> + return ret ? ret : state.parse_edid();
> +}
> +#endif
>
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] edid-decode: add function to enable usage as a library
2020-02-10 11:23 ` Hans Verkuil
@ 2020-02-10 14:58 ` Ilia Mirkin
2020-02-13 0:48 ` Ilia Mirkin
0 siblings, 1 reply; 7+ messages in thread
From: Ilia Mirkin @ 2020-02-10 14:58 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media
On Mon, Feb 10, 2020 at 6:23 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>
> Hi Ilia,
>
> I pushed my patch to initialize edid_state in the constructor to the
> edid-decode git repo.
Hmm... I thought it should already work before that. Things should get
zero-initialized in C++ constructors. There are some subtleties with
PODs though, but I don't think they apply here. But it's been a while
since I've looked at those C++ details, and your update constructor
definitely doesn't hurt.
>
> I've also split off the 'write to a file' part from edid_from_file into
> a separate edid_to_file function, so you'll have to rebase your patch.
Will do.
>
> Two comments below:
>
> On 2/9/20 6:49 PM, Ilia Mirkin wrote:
> > This is helpful for an emscripten setup, as there's no great way to
> > reinitialize the object from scratch.
> >
> > Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
> > ---
> > edid-decode.cpp | 18 ++++++++++++++++++
> > 1 file changed, 18 insertions(+)
> >
> > diff --git a/edid-decode.cpp b/edid-decode.cpp
> > index ef50da2..fd17bc6 100644
> > --- a/edid-decode.cpp
> > +++ b/edid-decode.cpp
> > @@ -1005,3 +1005,21 @@ int main(int argc, char **argv)
> > return edid_from_file(argv[optind], argv[optind + 1], out_fmt);
> > return ret ? ret : state.parse_edid();
> > }
> > +
> > +#ifdef __EMSCRIPTEN__
> > +/*
> > + * The surrounding JavaScript implementation will call this function
> > + * each time it wants to decode an EDID. So this should reset all the
> > + * state and start over.
> > + */
> > +extern "C" int parse_edid(const char *input) {
>
> '{' should start on the next line. Let's keep the coding style consistent.
Right, my bad.
>
> > + for (unsigned i = 0; i < EDID_MAX_BLOCKS + 1; i++) {
> > + s_msgs[i][0].clear();
> > + s_msgs[i][1].clear();
> > + }
>
> I'm pretty sure that this for loop is not necessary. Can you drop this
> for loop and see if it works?
The current code has:
static void show_msgs(bool is_warn)
{
printf("\n%s:\n\n", is_warn ? "Warnings" : "Failures");
for (unsigned i = 0; i < state.num_blocks; i++) {
if (s_msgs[i][is_warn].empty())
continue;
print-the-error
What would prevent an error from a previous run to appear without an
explicit clearing of s_msgs?
>
> > + options[OptCheck] = 1;
> > + state = edid_state();
> > + int ret = edid_from_file(input, NULL, OUT_FMT_DEFAULT);
> > + return ret ? ret : state.parse_edid();
> > +}
> > +#endif
> >
>
> Regards,
>
> Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] edid-decode: add function to enable usage as a library
2020-02-10 14:58 ` Ilia Mirkin
@ 2020-02-13 0:48 ` Ilia Mirkin
2020-02-13 7:16 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Ilia Mirkin @ 2020-02-13 0:48 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media
On Mon, Feb 10, 2020 at 9:58 AM Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Mon, Feb 10, 2020 at 6:23 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
> > > + for (unsigned i = 0; i < EDID_MAX_BLOCKS + 1; i++) {
> > > + s_msgs[i][0].clear();
> > > + s_msgs[i][1].clear();
> > > + }
> >
> > I'm pretty sure that this for loop is not necessary. Can you drop this
> > for loop and see if it works?
>
> The current code has:
>
> static void show_msgs(bool is_warn)
> {
> printf("\n%s:\n\n", is_warn ? "Warnings" : "Failures");
> for (unsigned i = 0; i < state.num_blocks; i++) {
> if (s_msgs[i][is_warn].empty())
> continue;
> print-the-error
>
> What would prevent an error from a previous run to appear without an
> explicit clearing of s_msgs?
Hi Hans,
Do you agree with my assessment above?
-ilia
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] edid-decode: add function to enable usage as a library
2020-02-13 0:48 ` Ilia Mirkin
@ 2020-02-13 7:16 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2020-02-13 7:16 UTC (permalink / raw)
To: Ilia Mirkin; +Cc: linux-media
On 2/13/20 1:48 AM, Ilia Mirkin wrote:
> On Mon, Feb 10, 2020 at 9:58 AM Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> On Mon, Feb 10, 2020 at 6:23 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:
>>>> + for (unsigned i = 0; i < EDID_MAX_BLOCKS + 1; i++) {
>>>> + s_msgs[i][0].clear();
>>>> + s_msgs[i][1].clear();
>>>> + }
>>>
>>> I'm pretty sure that this for loop is not necessary. Can you drop this
>>> for loop and see if it works?
>>
>> The current code has:
>>
>> static void show_msgs(bool is_warn)
>> {
>> printf("\n%s:\n\n", is_warn ? "Warnings" : "Failures");
>> for (unsigned i = 0; i < state.num_blocks; i++) {
>> if (s_msgs[i][is_warn].empty())
>> continue;
>> print-the-error
>>
>> What would prevent an error from a previous run to appear without an
>> explicit clearing of s_msgs?
>
> Hi Hans,
>
> Do you agree with my assessment above?
>
> -ilia
>
You are right, sorry. I should have checked the actual code instead of
relying on my obviously faulty memory.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] edid-decode: add build rule for emscripten output
2020-02-09 17:49 [PATCH 0/2] edid-decode: add emscripten support Ilia Mirkin
2020-02-09 17:49 ` [PATCH 1/2] edid-decode: add function to enable usage as a library Ilia Mirkin
@ 2020-02-09 17:49 ` Ilia Mirkin
1 sibling, 0 replies; 7+ messages in thread
From: Ilia Mirkin @ 2020-02-09 17:49 UTC (permalink / raw)
To: linux-media, hverkuil-cisco; +Cc: Ilia Mirkin
All I can really say is that this works in my current setup. This
current setup was not the first setup I tried, with lots of random
flailing around emscripten and binaryen versions to be compatible with
my LLVM version. Perhaps later emscripten needs slightly different flags
to export functions, who knows.
Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu>
---
Makefile | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Makefile b/Makefile
index 6db6386..49ad39b 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,8 @@
bindir ?= /usr/bin
mandir ?= /usr/share/man
+EMXX ?= em++
+
SOURCES = edid-decode.cpp parse-base-block.cpp parse-cta-block.cpp \
parse-displayid-block.cpp parse-ls-ext-block.cpp \
parse-di-ext-block.cpp parse-vtb-ext-block.cpp
@@ -13,6 +15,9 @@ sha = -DSHA=$(shell if [ -d .git ]; then git rev-parse HEAD ; else printf '"not
edid-decode: $(SOURCES) edid-decode.h Makefile
$(CXX) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $(WARN_FLAGS) -g $(sha) -o $@ $(SOURCES) -lm
+edid-decode.js: $(SOURCES) edid-decode.h Makefile
+ $(EMXX) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $(WARN_FLAGS) $(sha) -s EXPORTED_FUNCTIONS='["_parse_edid"]' -s EXTRA_EXPORTED_RUNTIME_METHODS='["ccall", "cwrap"]' -o $@ $(SOURCES) -lm
+
clean:
rm -f edid-decode
--
2.24.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-13 7:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-09 17:49 [PATCH 0/2] edid-decode: add emscripten support Ilia Mirkin
2020-02-09 17:49 ` [PATCH 1/2] edid-decode: add function to enable usage as a library Ilia Mirkin
2020-02-10 11:23 ` Hans Verkuil
2020-02-10 14:58 ` Ilia Mirkin
2020-02-13 0:48 ` Ilia Mirkin
2020-02-13 7:16 ` Hans Verkuil
2020-02-09 17:49 ` [PATCH 2/2] edid-decode: add build rule for emscripten output Ilia Mirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).