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.

Sign up for GitHub

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

Jump to bottom

render: Handle PNG and GIF data in DefineBitsJPEG tags (fix #530) #532

Merged
merged 2 commits into from
May 13, 2020
Merged

render: Handle PNG and GIF data in DefineBitsJPEG tags (fix #530) #532

merged 2 commits into from
May 13, 2020

Conversation

Herschel
Copy link
Member

According to the specs, DefineBitsJPEG2/3 tags can sometimes contain GIF and PNG data. This PR adds the ability to decode PNG data out of these tags.

I'm still not sure how to generate these tags using the Flash IDE, but there are examples in #530. I'm guessing it's some incantation of [Embed], for example, [Embed(source="test.gif")]. This may depend on the version of the authoring software (this metadata was first added in Flex Builder and didn't make it over to Flash until ~CS5).

Added determine_jpeg_tag_format and decode_define_bits_jpeg helper methods to handle the SWF tags. decode_define_bits_jpeg will verify the format of the image data and decode it appropriately.

GIF is currently unimplemented (we haven't seen a case of this in the wild yet), but a better error will now be returned.

@Herschel
Copy link
Member Author

@Dinnerbone could you try running this through scanner?

@Herschel Herschel force-pushed the jpeg-png branch 3 times, most recently from 9b86fc8 to 6482dff Compare April 29, 2020 12:02
@Dinnerbone
Copy link
Contributor

Scanning now, so far it's successfully parsing some PNGs and there's no errors. Should be finished in a few hours, but I'll know of any errors before then.

@Dinnerbone
Copy link
Contributor

Dinnerbone commented Apr 29, 2020 •
edited
Loading

I hit a panic early in the scan whilst parsing https://www.newgrounds.com/portal/view/524451/format/flash?emulate=flash:

thread 'main' panicked at 'called Result::unwrap() on an Err value: Format("TEM marker found where not allowed")', core\src\backend\render.rs:242:5

Before that, there was one JPEG that was broken: https://www.newgrounds.com/portal/view/586108/format/flash?emulate=flash

4 components without Adobe APP14 metadata to tell color space

Edit: Same panic as before with https://www.newgrounds.com/portal/view/615438/format/flash?emulate=flash and https://www.newgrounds.com/portal/view/515778/format/flash?emulate=flash

@Dinnerbone
Copy link
Contributor

Dinnerbone commented Apr 29, 2020 •
edited
Loading

DefineBitsJPEG2 scan results

Data source is Newgrounds dump via Flashpoint

Detected JPEG but couldn't parse

Detected PNG but couldn't parse

Detected GIF (and we don't have a parser)

Other types

There were no Unknown types detected, everything was JPEG, PNG or GIF.

@Dinnerbone
Copy link
Contributor

Dinnerbone commented Apr 29, 2020 •
edited
Loading

DefineBitsJPEG3 scan results

Data source is Newgrounds dump via Flashpoint.

Detected JPEG but couldn't parse

Other types

Every single tag contained a JPEG, there were 0 PNGs, GIFs or Unknowns. The warning about alpha + non-JPEG was never triggered.

@Herschel
Copy link
Member Author

Added GIF support (this also fixes #27).

Interestingly these all seem to be generated by OpenFL, so maybe you can't generate these using Flash IDE directly after all.

@Herschel Herschel changed the title render: Handle PNG data in DefineBitsJPEG tags (fix #530) render: Handle PNG and GIF data in DefineBitsJPEG tags (fix #530) Apr 30, 2020
@Dinnerbone
Copy link
Contributor

All gifs successfully parse 👍

@Herschel Herschel merged commit b59140e into ruffle-rs:master May 13, 2020
@Herschel Herschel mentioned this pull request May 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Herschel @Dinnerbone