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

Add librsvg for SVG rasterization support #444

Closed
khiemtong opened this issue May 16, 2024 · 12 comments
Closed

Add librsvg for SVG rasterization support #444

khiemtong opened this issue May 16, 2024 · 12 comments

Comments

@khiemtong
Copy link

Feature request to add librsvg for SVG support

@wader
Copy link
Owner

wader commented May 16, 2024

Hey! yes that would be nice. I did try it some years ago master...librsvg but lost motivation when there was some rust issues etc, don't remember the details. I think the situation is probably much better today but i think we still probably have to figure out how to build static libglib, pango and maybe something more.

@wader
Copy link
Owner

wader commented May 18, 2024

Have a WIP PR now #445 it's a bit of a mess to get it working but should be doable and i did managed to build a static ffmpeg binary with just librsvg and it's dependencies. Now have to make it all work together :)

If the multi-arch test build succeeds there should be a image tag called librsvg-test that you could test.

@khiemtong
Copy link
Author

Amazing! 🤩

@wader
Copy link
Owner

wader commented May 19, 2024

Ran into rust-lang/rust#104707 static-ffmpeg links with librav1e and now librsvg which both are written in rust which seems to cause symbol collision. Hope there is some workaround, will have a look later today

@wader
Copy link
Owner

wader commented May 19, 2024

Think found a workaround using -Wl,--allow-multiple-definition which is not great as i suspect it might hide real problem, but it seems like ppl are working actively to resolve this in rust in a more proper way. So maybe it's ok for now if we don't notice anything weird.

If things go well there should be a librsvg-test tag that you can try in 30-40min. It would be good to test things like font rendering and if something looks strange it would be great to try with a different "known good" ffmpeg buitl with rsvg support if possible.

@wader
Copy link
Owner

wader commented May 19, 2024

@khiemtong could you have a look at the new section in the readme https://github.com/wader/static-ffmpeg/blob/316f67840ab1b9fdf91b31071c471fd30d4b320f/README.md#fonts-usage-with-svg-or-draw-text-filters-etc about fonts and see if it works (use the librsvg-test tag) for you use case?

@khiemtong
Copy link
Author

@wader Works as expected 🎊 🏅

  • I was able to pull down the librsvg-test tag
  • Installed Inter Google font with fontconfig on my Docker image
  • Converted a custom SVG with Inter font in use and got the expected results

I'll do more testing on other files but looks like we're cooking, thanks for all the work on this!

@wader
Copy link
Owner

wader commented May 20, 2024

An alternative to -Wl,--allow-multiple-definition seem to be running ld -r on the rust produced objects files, see rust-lang/rust#104707 (comment), maybe worth a try. It has happen before with static-ffmpeg that some c libraries happens to have colliding symbol name that were not compatible, that probably would have caused crashes or unknown behavior if not detected.

@wader
Copy link
Owner

wader commented May 21, 2024

@khiemtong things seem to work?

I'm still a bit reluctant about -Wl,--allow-multiple-definition but i'm thinking maybe we can do some temp hack to at least have some safety about colliding symbols until it gets fixed in rust or whatnot. One way is to do the symbol check ourself which is messy and not perfect and there are some oddities with it that i don't understand yet (libav* seems to have collisions but ld is ok with it, ex ff_icc_context_init in both libavfilter and libcodec hmm)

If this command returns a line with count 2 we probably have a collision

for i in $(rm -f ffmpeg* ; make V=1 LD="gcc -Wl,-t" | sort | grep '.a$' | uniq | grep -Ev '(librav1e|librsvg|libtheora|libav)'); do nm --extern-only $i | awk '/^\d+ [TDB]/ {pr
int $3}' | sort | uniq ; done | sort | uniq -c | sort -n

@khiemtong
Copy link
Author

khiemtong commented May 21, 2024

@wader Yes, for our use cases things work so defer to you how to proceed. Frankly the compilation and symbols are out of my depth but I'm trying to follow as best I can 😄

I've been thinking about how to ensure stability based on your comments. Only idea I've had is to just compile a separate tags that includes librsvg (and other options) so it's opt in

i.e.

If you want main path

COPY --from=mwader/static-ffmpeg:7.0-1 /ffmpeg /usr/local/bin/
COPY --from=mwader/static-ffmpeg:7.0-1 /ffprobe /usr/local/bin/

If you want everything

COPY --from=mwader/static-ffmpeg:7.0-1-unstable /ffmpeg /usr/local/bin/
COPY --from=mwader/static-ffmpeg:7.0-1-unstable /ffprobe /usr/local/bin/

Then as things get more tested, we can move more options into the stable path or forever leave librsvg to the unstable tag. A bit more maintenance but mitigates the risk?

Another option is if there's just a Dockerfile arg to build with librsvg that would work too since whoever needs it could opt-in that way and build it themselves

@wader
Copy link
Owner

wader commented May 21, 2024

Great! Ideally i would want librsvg to be included by default and no args etc to keep things simple. The problem we encountered now is not really an issue with librsvg, it just happen to be the second rust-based library to link with, so this was probably inevitable to happen as more and more libraries use rust.

So i think i'm lening towards using -Wl,--allow-multiple-definition and a ugly hack to fail build if a dup symbol is found, this is only to have some safety when updating versions or adding something new. And then hope thing will get sorted in the rust toolchain in a year or two (that it hides symbols that are not public/exported).

@wader wader mentioned this issue May 22, 2024
@wader wader closed this as completed May 22, 2024
@wader
Copy link
Owner

wader commented May 22, 2024

Now merged 🥳 and i've also tagged 7.0-2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants