-
Notifications
You must be signed in to change notification settings - Fork 46
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
Fix compiling and add sample decoder #84
Conversation
This looks OK. The sample decoders are technically build artifacts, I'm not necessarily opposed to including them in-tree but it would be good for them to be reproducible. At minimum I'd like a script and the exact libaom versions you used to generate them. |
I added a readme within sample decoder to add instructions on how to generate the artifacts. https://github.com/xiph/aomanalyzer/blob/ca6c82ac127e638d815c2223817f42b9f707b7e7/sample_decoder/readme.md I also added this to tsconfig.json |
OK. I'm still not super hot on the idea of the .wasm/.js build artifacts being checked into git - I think they are nice thing to have but I don't really like committing build artifacts. If you split those commits into a separate PR so we can discuss it further, we can land the other changes in this PR. |
Okay I'll remove the artifacts for this PR. I still think we need to have a way for the user to have access to inspect.js/inspect.wasm as there aren't instructions on how to get them and the emscripten build for aom is currently a bit messy. I'll create a draft PR so that we can discuss this further |
Issues with Compiling
I was running into issues when trying to build on my local and realized that there were some issues.
typescript was complaining about type issues in node_modules/**/.d.ts files. To fix that, I upgraded to typescript 4.2.4
There were some
this.document.<attr>
that weren't referenced as(this.document as any).<attr>
so typescript complained that document does not have'attr'
. I addedas any
to fix that issue.Finally, tsconfig had some redundant config values so I just removed them.
Added Sample Decoder
I also added a sample_decoder to include
inspect.js
andinspect.wasm
files