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

Jsapi v0.2 #2446

Merged
merged 6 commits into from
Aug 6, 2020
Merged

Jsapi v0.2 #2446

merged 6 commits into from
Aug 6, 2020

Conversation

Urhengulas
Copy link
Contributor

@Urhengulas Urhengulas commented Jul 14, 2020

Goals

Feed custom data into encoder for:

@Urhengulas Urhengulas self-assigned this Jul 14, 2020
@coveralls
Copy link
Collaborator

coveralls commented Jul 14, 2020

Coverage Status

Coverage decreased (-1.5%) to 80.736% when pulling e4e4dea on Urhengulas:jsapi-v0.2 into f6ac395 on xiph:master.

rav1e_js/src/lib.rs Outdated Show resolved Hide resolved
rav1e_js/Cargo.toml Outdated Show resolved Hide resolved
@Urhengulas Urhengulas force-pushed the jsapi-v0.2 branch 2 times, most recently from 50d34f5 to a027b49 Compare July 29, 2020 00:52
@Urhengulas Urhengulas force-pushed the jsapi-v0.2 branch 4 times, most recently from 172fe64 to ce4c992 Compare August 2, 2020 17:04
@Urhengulas Urhengulas marked this pull request as ready for review August 2, 2020 17:05
@vibhoothi
Copy link
Collaborator

Is 0.1.1 valid if this exists if so that should be landed first imo, if not better close it

@Urhengulas
Copy link
Contributor Author

Is 0.1.1 valid if this exists if so that should be landed first imo, if not better close it

@vibhoothiiaanand: 0.1.1 is only a draft and not complete. I will probably do I next and land it as 0.2.1.

Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

It looks already fairly good and an interesting demo. I have only a nit and it is good to go for me.

rav1e_js/src/frame.rs Show resolved Hide resolved
Copy link
Collaborator

@vibhoothi vibhoothi left a comment

Choose a reason for hiding this comment

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

I was seeing around locally, seems good overall but have some concerns before we land.

rav1e_js/www/public/index.html Outdated Show resolved Hide resolved
rav1e_js/www/src/App.tsx Outdated Show resolved Hide resolved
rav1e_js/rebuild.sh Outdated Show resolved Hide resolved
rav1e_js/www/src/App.tsx Outdated Show resolved Hide resolved
rav1e_js/www/src/App.tsx Outdated Show resolved Hide resolved
rav1e_js/www/src/App.tsx Show resolved Hide resolved
Copy link
Collaborator

@vibhoothi vibhoothi left a comment

Choose a reason for hiding this comment

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

Right now, it does not work in Safari

Logs from console:

[Error] invalid width 0 (expected >= 16, <= 32767): Invalid EncoderConfig
	__wbindgen_rethrow (2.chunk.js:1786)
	__wbindgen_rethrow (bundle.js:908:95)
	wasm-stub
	<?>.wasm-function[wasm_bindgen::throw_val::hf03b4efa9e9f245f]
	<?>.wasm-function[wasm_bindgen::convert::impls::<impl wasm_bindgen::convert::traits::ReturnWasmAbi for core::result::Result<T,wasm_bindgen::JsValue>>::return_abi::h952487efcbd465ce]
	<?>.wasm-function[videoencoder_fromEncoderConfig]
	wasm-stub
	videoencoder_fromEncoderConfig
	VideoEncoder (2.chunk.js:1441:96)
	(anonymous function) (2.chunk.js:1873)
	commitHookEffectListMount (1.chunk.js:19850)
	commitPassiveHookEffects (1.chunk.js:19887)
	callCallback (1.chunk.js:432)
	dispatchEvent
	invokeGuardedCallbackDev (1.chunk.js:481)
	invokeGuardedCallback (1.chunk.js:534)
	flushPassiveEffectsImpl (1.chunk.js:22951)
	unstable_runWithPriority (1.chunk.js:28033)
	(anonymous function) (1.chunk.js:22798)
	workLoop (1.chunk.js:27977)
	flushWork (1.chunk.js:27933)
	performWorkUntilDeadline (1.chunk.js:27537)
[Error] The above error occurred in the <App> component:
    in App (at main.tsx:16)
    in StrictMode (at main.tsx:15)

Consider adding an error boundary to your tree to customize error handling behavior.
Visit https://fb.me/react-error-boundaries to learn more about error boundaries.
	logCapturedError (1.chunk.js:19651)
	logError (1.chunk.js:19687)
	(anonymous function) (1.chunk.js:20803)
	callCallback (1.chunk.js:12773)
	commitUpdateQueue (1.chunk.js:12796)
	commitLifeCycles (1.chunk.js:19997)
	commitLayoutEffects (1.chunk.js:22902)
	callCallback (1.chunk.js:432)
	dispatchEvent
	invokeGuardedCallbackDev (1.chunk.js:481)
	invokeGuardedCallback (1.chunk.js:534)
	commitRootImpl (1.chunk.js:22644)
	commitRootImpl
	unstable_runWithPriority (1.chunk.js:28033)
	commitRoot (1.chunk.js:22486)
	finishSyncRender (1.chunk.js:21903)
	performSyncWorkOnRoot (1.chunk.js:21889)
	performSyncWorkOnRoot
	(anonymous function) (1.chunk.js:11373)
	unstable_runWithPriority (1.chunk.js:28033)
	flushSyncCallbackQueueImpl (1.chunk.js:11368)
	flushSyncCallbackQueue (1.chunk.js:11356)
	flushPassiveEffectsImpl (1.chunk.js:22978)
	unstable_runWithPriority (1.chunk.js:28033)
	(anonymous function) (1.chunk.js:22798)
	workLoop (1.chunk.js:27977)
	flushWork (1.chunk.js:27933)
	performWorkUntilDeadline (1.chunk.js:27537)
[Error] invalid width 0 (expected >= 16, <= 32767): Invalid EncoderConfig
	performWorkUntilDeadline (1.chunk.js:27551)

Image:
Screenshot 2020-08-04 at 3 26 27 PM

@vibhoothi
Copy link
Collaborator

Screenshot 2020-08-04 at 3 30 03 PM

On firefox, it does autostart properly but after doing 10 frames, the browser becomes super slow and freezes and I had to stop it manually and then only I can do anything to make browser to normalcy(like normal right click).

@vibhoothi
Copy link
Collaborator

Module.__wbindgen_throw
/Volumes/buildBox/rav1e/rav1e-master/rav1e_js/pkg/rav1e_js_bg.js:1182
  1179 | };
  1180 | 
  1181 | export const __wbindgen_throw = function(arg0, arg1) {
> 1182 |     throw new Error(getStringFromWasm0(arg0, arg1));
  1183 | };
  1184 | 
  1185 | export const __wbindgen_rethrow = function(arg0) {
View compiled
__wbindgen_throw
/Volumes/buildBox/rav1e/rav1e-master/rav1e_js/www/webpack/bootstrap:904
  901 | 	return installedModules["../pkg/rav1e_js_bg.js"].exports["__wbindgen_debug_string"](p0i32,p1i32);
  902 | },
  903 | "__wbindgen_throw": function(p0i32,p1i32) {
> 904 | 	return installedModules["../pkg/rav1e_js_bg.js"].exports["__wbindgen_throw"](p0i32,p1i32);
      | ^  905 | },
  906 | "__wbindgen_rethrow": function(p0i32) {
  907 | 	return installedModules["../pkg/rav1e_js_bg.js"].exports["__wbindgen_rethrow"](p0i32);
View compiled
▶ 3 stack frames were collapsed.
VideoEncoder.flush
/Volumes/buildBox/rav1e/rav1e-master/rav1e_js/pkg/rav1e_js_bg.js:953
  950 | * Flushing signals the end of the video. After the encoder has been flushed, no additional frames are accepted.
  951 | */
  952 | flush() {
> 953 |     wasm.videoencoder_flush(this.ptr);
      | ^  954 | }
  955 | /**
  956 | * Encodes the next frame and returns the encoded data.
View compiled
HTMLVideoElement.<anonymous>
src/App.tsx:29
  26 | enc.sendVideo(video);
  27 | 
  28 | video.addEventListener("ended", (e) => {
> 29 | 	enc.flush()
     | ^  30 | 
  31 | 	// encode all frames
  32 | 	while (true) {
View compiled```


From console

rav1e_js_bg.js:1182 Uncaught Error: recursive use of an object detected which would lead to unsafe aliasing in rust
at Module.__wbindgen_throw (rav1e_js_bg.js:1182)
at __wbindgen_throw (bootstrap:904)
at f2899e32c0062e3868c2.module.wasm:wasm-function[1058]:0xa1c77
at f2899e32c0062e3868c2.module.wasm:wasm-function[1057]:0xa1c6c
at videoencoder_flush (f2899e32c0062e3868c2.module.wasm:wasm-function[916]:0xa0c3b)
at VideoEncoder.flush (rav1e_js_bg.js:953)
at HTMLVideoElement. (App.tsx:29)

@Urhengulas Urhengulas force-pushed the jsapi-v0.2 branch 2 times, most recently from f046a47 to 20983fa Compare August 5, 2020 10:49
Copy link
Collaborator

@vibhoothi vibhoothi left a comment

Choose a reason for hiding this comment

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

Now it is way better than old times with performance being ok(but too much memory+ ram usage at any case).

But Safari it is broken still stating same error, needs to be fixed
Screenshot 2020-08-05 at 4 45 58 PM
Seems before allocation of value the function is being called.

Once we start the encoding, then the browser goes brrrr with no control over things till it is done flushing and encoding, so in terms of Users, it is a not so nice UX, not sure how to address them.

Edit:

Now on Safari it works with latest head changes 35c70e6

@vibhoothi
Copy link
Collaborator

vibhoothi commented Aug 5, 2020

Also,
On Firefox after being encoded

panicked at 'called `Result::unwrap()` on an `Err` value: EnoughData', rav1e_js/src/encoder/video_encoder.rs:94:13

Stack:

__wbg_new_59cb74e423758ede@http://localhost:3000/static/js/2.chunk.js:1334:13
__wbg_new_59cb74e423758ede@http://localhost:3000/static/js/bundle.js:794:73
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[338]:0x807b5
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[1078]:0xa1d52
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[632]:0x98190
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[902]:0xa0956
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[939]:0xa10c9
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[768]:0x9dada
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[208]:0x6a68e
@http://localhost:3000/f2899e32c0062e3868c2.module.wasm:wasm-function[980]:0xa16e6
__wbg_adapter_18@http://localhost:3000/static/js/2.chunk.js:417:50
real@http://localhost:3000/static/js/2.chunk.js:402:14


index.js:1
    e index.js:1
    __wbg_error_4bb6c2a97407129a rav1e_js_bg.js:1006
    Webpack 11

Screenshot 2020-08-05 at 4 53 54 PM

Would be nice if it is addressed

Edit:
Easy way to reproduce,
encode one video -> wait for it to finish -> Tap Play button -> Panic.

Copy link
Collaborator

@vibhoothi vibhoothi left a comment

Choose a reason for hiding this comment

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

Suggestion: Disable autoplay in favour of better UX and browser handling since once it is started playing/encoding, we do not have fluid control over the browser.
So need to make sure console is open before starting it.

@Urhengulas Urhengulas force-pushed the jsapi-v0.2 branch 3 times, most recently from 49c03ae to 0e4edba Compare August 5, 2020 15:00
* access HtmlCanvasElements by id
* create new HtmlCanvasElement
* access CanvasRenderingContext2d
Changes:
* access underlying pixel-data (RGBA)
* convert to ARGB
* convert to I444
Copy link
Collaborator

@vibhoothi vibhoothi left a comment

Choose a reason for hiding this comment

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

Now the set looks good to me, more or less it is fine,

Tested locally with Chrome, Firefox, Safari,
Safari was a bit more optimal on initial testing.

The downside of 0.2 is, it is bit more memory hungry, we need to fix that by 0.3, and as it is an experimental thing, think it is ok to move forward.

Thank you for your work.

* create rav1e::api::Frame<u8> from web::Canvas
* switch to u8 (Encoder, Frame, Packer)
* encoder
    * `trait Encoder` implements basic encoder functionality
    * rename `struct Encoder` to `FrameEncoder`
* www: Adapt to renaming
@Urhengulas Urhengulas force-pushed the jsapi-v0.2 branch 5 times, most recently from 0e239d9 to 61e1b9a Compare August 6, 2020 13:57
Urhengulas and others added 2 commits August 6, 2020 15:58
* video.onplay
    * collect data
    * create frame from data
* video.onended
    * encode all data

Thanks to Alexander Fallenstedt for his blog post! (https://dev.to/fallenstedt/using-rust-and-webassembly-to-process-pixels-from-a-video-feed-4hhg)

Co-authored-by: Alexander Fallenstedt <a.fallenstedt@gmail.com>
* derive(Default) for EncoderConfig
* Add `tools/rebuild.sh`
* CI: also set node/npm version for 'build'
* Update README.md
* Clean Cargo.toml
    * remove patch-specification of dependency versions
    * configure wasm-opt arguments
* Bump version to 0.2.0
@lu-zero lu-zero merged commit 66dc853 into xiph:master Aug 6, 2020
@Urhengulas Urhengulas deleted the jsapi-v0.2 branch August 6, 2020 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants