video: pick up VP6-with-alpha decoder fix#23743
Conversation
|
Oh, were you able to access the content in the linked issue somehow? |
torokati44
added
A-video
T-fix
doctorpangloss
force-pushed
the
fix/vp6-alpha-split
branch
from
de17346 to
1a96a20
Compare
no, but the error message is just saying that swf contained vp6 with alpha, which is same as my content sources: https://github.com/doctorpangloss/multics
afaik, they are solving different issues. if you provide a guide for establishing baselines, i will fix all of TextLine too, because multics also needs that. |
Ahh, so that's where the typewriter came from...
Indeed, and both are valid!
I can only point you at: https://github.com/ruffle-rs/ruffle/blob/master/CONTRIBUTING.md And as also noted therein, under "Test Guidelines", I think the added test should be an SWF-based one too, like the one added in #23665. BTW #23215 also touches |
There was a problem hiding this comment.
Looks like you did a full cargo update. That will break a bunch of things - I suggest to just replace the git hashes in Cargo.toml, then a simple cargo check will update this file accordingly, without changing anything else.
| nihav_duck = { git = "https://github.com/ruffle-rs/nihav-vp6", rev = "e3fec79dda5120caa46a127eaafc18eb36b32408", optional = true } | ||
|
|
||
| [dev-dependencies] | ||
| flv-rs = { path = "../../flv" } |
| use ruffle_video_software::decoder::VideoDecoder; | ||
| use swf::VideoCodec; | ||
|
|
||
| const FIXTURE: &[u8] = include_bytes!("fixtures/vp6_alpha_one_frame.flv"); |
There was a problem hiding this comment.
Could you make an SWF-based test instead of this? you can see tests/tests/swfs/visual/video/colorconversion/vp6 (IIRC) for an example.
Two interacting VP6-with-alpha (VP6A) fixes, rolled together: - Pick up the nihav-vp6 decoder fix (bump the crate git ref): corrects VP6A decoding when the Y and alpha plane sizes differ. - Premultiply the alpha in Bitmap::to_rgba for Yuva420p. VP6A carries straight alpha while the renderer and blend state expect premultiplied alpha; the previous min(c, a) clamp left every pixel with c <= a un-premultiplied, rendering semi-transparent video too bright. Use a real premultiply (c * a / 255). Add a vp6_alpha decoder unit test, and re-baseline the colorconversion/vp6a visual test to the corrected premultiplied render. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
doctorpangloss
force-pushed
the
fix/vp6-alpha-split
branch
from
f9a39ba to
7b1a5d9
Compare
|
I think the two fixes would be nicer in separate commits. |
|
About premultiplied alpha: The vp6a test looks exactly identical in FP and current Ruffle (without this PR) on my machine: 
Maybe this is dependent on the platform or FP version? 😵 EDIT: What Flash Player, on what platform, did you use to get the new (less bright) expected image? |
|
Also, from the SWF spec (https://open-flash.github.io/mirrors/swf-spec-19.pdf) 
EDIT: Not that these specs are infallible, or never lie... |
|
I hope it's not that this is done differently depending on whether the video is embedded in a SWF, or comes from an external FLV file... (I doubt that, but you can never know!) EDIT: Well, but then the existing test wouldn't change, as that doesn't use a FLV file... |
|
It also looks exactly as bright to me on Windows as on Linux in Flash Player: 
|
torokati44
added
the
waiting-on-author
2 participants
Bumps nihav-vp6 to ruffle-rs/nihav-vp6#3, which fixes a 3-byte off-by-one
in the VP6-alpha color sub-stream length. Adds a regression test that
decodes a real VP6-alpha keyframe end-to-end.
Fixes #23662.