Hi Shashank,
There is no cover letter for this series, so I'll just reply to the
first patch, but my comments are high-level and not specific to this
patch.
To be honest, I am not at all convinced that using edid-decode as a
parser library is the right thing to do. It was never written with that
in mind. The two purposes of edid-decode are to:
1) Convert the EDID to a human readable text, and
2) Verify if the EDID conforms to the various standards and is internally
consistent.
As a result the state information that edid-decode stores is just the
state that it needs to check conformity across Extension Blocks and/or
Data Blocks. Most of the parsed data is just printed to stdout and checked
and then forgotten.
I have considered if it would make sense to make a library to parse and
store the EDID data and have edid-decode sit on top of that, but that will
make the conformity tests much harder. It's kind of interwoven with the
parsing and a parser library is really not interested in that anyway.
I think edid-decode can function very well as a reference source for
a real EDID parser since edid-decode is very complete, but not as a
EDID parser library.
BTW, if anyone has detailed info for the AMD Freesync Data Block, then
I'd love to have that. The cta_amd() function is based on reverse engineering
and not everything could be decoded.
Regards,
Hans
On 3/4/22 13:49, Shashank Sharma wrote:
> From: Shashank Sharma <shashank.sharma@xxxxxxx>
>
> This patch does some small changes to make the core logic of
> edid-decode tool available to a shared library wrapper. With
> these changes, the EDID's 'state' variable will be avialble
> to another process via some library API calls.
>
> Cc: Pekka Paalanen <ppaalanen@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
>
> Signed-off-by: Shashank Sharma <contactshashanksharma@xxxxxxxxx>
> ---
> Makefile | 22 +++++++++++++++++++++-
> edid-decode.cpp | 15 ++++++++++++++-
> 2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1843700..ebf3370 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,14 +1,20 @@
> ifeq ($(OS),Windows_NT)
> bindir ?= /usr/bin
> mandir ?= /usr/share/man
> + libdir ?= /usr/lib
> + includedir ?= /usr/include
> else
> UNAME_S := $(shell uname -s)
> ifeq ($(UNAME_S),Darwin)
> bindir ?= /usr/local/sbin
> mandir ?= /usr/local/share/man
> + libdir ?= /usr/local/lib
> + includedir ?= /usr/include
> else
> bindir ?= /usr/bin
> mandir ?= /usr/share/man
> + libdir ?= /usr/lib
> + includedir ?= /usr/include
> endif
> endif
>
> @@ -19,6 +25,11 @@ SOURCES = edid-decode.cpp parse-base-block.cpp parse-cta-block.cpp \
> parse-di-ext-block.cpp parse-vtb-ext-block.cpp calc-gtf-cvt.cpp
> WARN_FLAGS = -Wall -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Wimplicit-fallthrough
>
> +LIB_NAME = libedid-decode.so
> +LIB_FLAGS = -fPIC
> +LIBLINK_FLAGS = -shared
> +LIB_SOURCES = libedid-decode-api.cpp
> +
> all: edid-decode
>
> sha = -DSHA=$(shell if test -d .git ; then git rev-parse --short=12 HEAD ; fi)
> @@ -30,11 +41,20 @@ edid-decode: $(SOURCES) edid-decode.h oui.h Makefile
> edid-decode.js: $(SOURCES) edid-decode.h oui.h Makefile
> $(EMXX) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $(WARN_FLAGS) $(sha) $(date) -s EXPORTED_FUNCTIONS='["_parse_edid"]' -s EXTRA_EXPORTED_RUNTIME_METHODS='["ccall", "cwrap"]' -o $@ $(SOURCES) -lm
>
> +libedid-decode: $(SOURCES) edid-decode.h oui.h Makefile
> + $(CXX) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $(WARN_FLAGS) -g $(LIB_FLAGS) $(sha) $(date) $(LIBLINK_FLAGS) -o $(LIB_NAME) $(LIB_SOURCES) $(SOURCES) -lm
> +
> clean:
> - rm -f edid-decode
> + rm -f edid-decode libedid-decode.so
>
> install:
> mkdir -p $(DESTDIR)$(bindir)
> install -m 0755 edid-decode $(DESTDIR)$(bindir)
> mkdir -p $(DESTDIR)$(mandir)/man1
> install -m 0644 edid-decode.1 $(DESTDIR)$(mandir)/man1
> +
> +install-lib:
> + mkdir -p $(DESTDIR)$(libdir)
> + mkdir -p $(DESTDIR)$(includedir)
> + install -m 0755 libedid-decode.so $(DESTDIR)$(libdir)
> + install -m 0644 libedid-decode-api.h $(DESTDIR)$(includedir)
> diff --git a/edid-decode.cpp b/edid-decode.cpp
> index 4a90aba..babff4a 100644
> --- a/edid-decode.cpp
> +++ b/edid-decode.cpp
> @@ -21,7 +21,7 @@
> #define STR(x) #x
> #define STRING(x) STR(x)
>
> -static edid_state state;
> +edid_state state;
>
> static unsigned char edid[EDID_PAGE_SIZE * EDID_MAX_BLOCKS];
> static bool odd_hex_digits;
> @@ -1012,6 +1012,19 @@ static bool extract_edid(int fd, FILE *error)
> state.edid_size = edid_data.size();
> return true;
> }
> +struct edid_state *extract_edid_state(int fd, FILE *error)
> +{
> + bool ret;
> +
> + ret = extract_edid(fd, error);
> + if (ret) {
> + /* update the number of blocks */
> + state.num_blocks = state.edid_size / EDID_PAGE_SIZE;
> + return &state;
> + }
> +
> + return NULL;
> +}
>
> static unsigned char crc_calc(const unsigned char *b)
> {
