search

JuliaIO / VideoIO.jl

Reading and writing of video files in Julia via ffmpeg
https://juliaio.github.io/VideoIO.jl/stable
Other
128 stars 53 forks source link

Update Clang wrapper generator and wrapper code #333

Closed melonedo closed 2 years ago

melonedo commented 3 years ago

The generator uses the master branch of Clang.jl (add https://github.com/JuliaInterop/Clang.jl). Most troubles come from avptr, but hopefully I can add a compatibility layer for this. Currently it is impossible to split different modules based on header structures because there might be problems about where to put type definitions, so I collected all of the wrappers at libffmpeg.jl.

May fix #287. cc @gnimuc

codecov[bot] commented 3 years ago

Codecov Report

Merging #333 (bd4061a) into master (bda9d46) will increase coverage by 0.39%. The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #333      +/-   ##
==========================================
+ Coverage   81.60%   82.00%   +0.39%     
==========================================
  Files          17       10       -7     
  Lines        1185     1239      +54     
==========================================
+ Hits          967     1016      +49     
- Misses        218      223       +5
Impacted Files Coverage Δ
src/encoding.jl 93.33% <ø> (+0.09%) :arrow_up:
src/avdictionary.jl 8.82% <20.00%> (-0.56%) :arrow_down:
src/VideoIO.jl 54.34% <71.42%> (ø)
src/avptr.jl 69.56% <100.00%> (+2.89%) :arrow_up:
src/util.jl 89.47% <100.00%> (-5.27%) :arrow_down:
src/testvideos.jl 73.80% <0.00%> (-3.12%) :arrow_down:
src/avframe_transfer.jl 81.22% <0.00%> (-2.20%) :arrow_down:
src/info.jl 100.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bda9d46...bd4061a. Read the comment docs.

melonedo commented 3 years ago

Oops the tests failed on windows because I did not import ffmpeg, now that is fixed. @IanButterworth cc @gnimuc

IanButterworth commented 2 years ago

Sorry, somehow I missed this! This is much appreciated work!!

One question is whether the move to put all functions under libffmpeg rather than AVUtil etc. as they were before loses any information/clarity?

Also I believe I recall @galenlynch suggesting the idea of separating the ffmpeg wrapper out into its own package. If that was the case, what are the motivations to do so, as perhaps this could be the time to?

melonedo commented 2 years ago

One question is whether the move to put all functions under libffmpeg rather than AVUtil etc. as they were before loses any information/clarity?

Separating each header file was a feature of older Clang versions. Now as we resolve dependency by pulling out all of a header's dependency, types and constants in system headers would be duplicated in each wrapper if we split them.

Gnimuc commented 2 years ago

It's not hard to manually filter out those duplicated decls, see https://github.com/maleadt/LLVM.jl/blob/790a3d4b231f1fd41848710a37dd190d44e45740/res/wrap_llvmextra.jl#L37.

melonedo commented 2 years ago

I do not know why CI on windows failed with message "This check failed" with no additional log.

Gnimuc commented 2 years ago

Looks like GitHub CI for Windows is temporarily down.

IanButterworth commented 2 years ago

@galenlynch I wanted to check if you have a view on this? IMO we should merge

IanButterworth commented 2 years ago

@melonedo @Gnimuc One question.. in #340 I'm adding error codes to julia errors. Is there a way to automate converting those codes back to the ffmpeg code symbols, i.e. ENOMEM?

This change doesn't expose those symbol values, AFAICT

melonedo commented 2 years ago

These error codes are not all defined by FFMPEG. Instead, some of them are just "standard" error codes defined in errorno.h, e.g. ENOMEM. I suppose these constants are already exposed somewhere. Simple searching found an old version of julia had them: https://github.com/Wilfred/julia/blob/c258b4c3aca5c17e7c7069f28776fe62e0ec5e73/base/errno.jl. Some of them are defined in FFMPEG, these are already exported at https://github.com/JuliaIO/VideoIO.jl/blob/bd4061a0bb670b40875324bf0ac3cc1abc55ee52/lib/libffmpeg.jl#L21397-L21449.

IanButterworth commented 2 years ago

Of course. I picked a bad example :)

That's great. Following this PR we can write an error code -> symbol converter for the error messages.

@galenlynch I'll merge tomorrow if I don't hear otherwise :)