On Mon, Feb 10, 2020 at 6:23 AM Hans Verkuil <hverkuil-cisco@xxxxxxxxx> 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@xxxxxxxxxxxx>
> > ---
> > 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
