etemesi254 / zune-image

A fast and memory efficient image library in Rust
Other
332 stars 30 forks source link

Incorrect decoding / artifacts when opening APNG #185

Closed woelper closed 7 months ago

woelper commented 7 months ago

Hi, and thanks for your great library!

I am using code similar to the example in https://docs.rs/zune-png/latest/zune_png/fn.post_process_image.html to open APNG files. They seem to have artifacts that look like they could come from a wrong offset or background. I have made a minimum example here:

https://github.com/woelper/apng_test

19 2

33

etemesi254 commented 7 months ago

It may be me who didn't understand the spec, so here is what is happening

The spec at https://wiki.mozilla.org/APNG_Specification says when the value of dispose_op is none no disposal is done, so contents of output buffer are left as is.

In our case, output buffer always contains the previous frame or zeroed if it's the first frame

And it happens that the current frame may be less than the image size. The frame 2 and 19 are smaller than image size with the output buffer being the previous frame (frame 1 and 18) so the implementation only copies the area it was told to.

I think ffmpeg zeroes out the buffer even if dispose_op is none, so I'll just copy whatever it does.

etemesi254 commented 7 months ago

Done in https://github.com/etemesi254/zune-image/commit/d51341e32f0914a044fc1e290e4b96d68ca36ad6

woelper commented 7 months ago

Wow, that was incredibly fast. Thank you so much for looking into this! You rock!

etemesi254 commented 7 months ago

Pushed 0.5.0-rc0 which contains this and a lot of other changes, please confirm everything is working (it's a breaking change so it may need some things to be rewritten but has some cool features like directly reading from files and not requiring the whole image into memory)

woelper commented 7 months ago

Thanks! There were indeed some changes, but nothing dramatic. The bouncing ball animation looks correct now. The clock however just seems to draw the first frame. I'll try to update my example with your latest release soon.

woelper commented 7 months ago

Thanks so much!

I tested and updated the example repo in https://github.com/woelper/apng_test.

I noticed the "Bouncing Ball" animation is now correct and displays without artifacts. As for the "Clock" example, only the first frame is visible, all other frames are now fully transparent. FrameInfo is as follows:

Frame info: FrameInfo { seq_number: 0, width: 150, height: 150, x_offset: 0, y_offset: 0, delay_num: 4, delay_denom: 100, dispose_op: None, blend_op: Source, is_part_of_seq: true }
Frame info: FrameInfo { seq_number: 1, width: 70, height: 58, x_offset: 48, y_offset: 31, delay_num: 4, delay_denom: 100, dispose_op: None, blend_op: Over, is_part_of_seq: true }
Frame info: FrameInfo { seq_number: 3, width: 72, height: 57, x_offset: 46, y_offset: 32, delay_num: 4, delay_denom: 100, dispose_op: None, blend_op: Over, is_part_of_seq: true }
...

(The following frames have similar output)

I don't know if the clock animation is fully up to spec, but it at least renders correctly in the browser.

etemesi254 commented 7 months ago

0.5.0-rc1 has a fix for this.

Sorry for the delay

woelper commented 6 months ago

Sorry for the long delay, but I have verified your fix works - thank you again!