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

Update Clang wrapper generator and wrapper code #333

Merged
merged 7 commits into from
Nov 12, 2021

Conversation

melonedo
Copy link
Contributor

@melonedo melonedo commented Aug 26, 2021

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
Copy link

codecov bot commented Aug 26, 2021

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%) ⬆️
src/avdictionary.jl 8.82% <20.00%> (-0.56%) ⬇️
src/VideoIO.jl 54.34% <71.42%> (ø)
src/avptr.jl 69.56% <100.00%> (+2.89%) ⬆️
src/util.jl 89.47% <100.00%> (-5.27%) ⬇️
src/testvideos.jl 73.80% <0.00%> (-3.12%) ⬇️
src/avframe_transfer.jl 81.22% <0.00%> (-2.20%) ⬇️
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
Copy link
Contributor Author

melonedo commented Sep 3, 2021

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

@IanButterworth
Copy link
Member

IanButterworth commented Oct 11, 2021

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
Copy link
Contributor Author

melonedo commented Oct 13, 2021

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
Copy link

Gnimuc commented Oct 13, 2021

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
Copy link
Contributor Author

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

@Gnimuc
Copy link

Gnimuc commented Oct 13, 2021

Looks like GitHub CI for Windows is temporarily down.

@IanButterworth
Copy link
Member

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

@IanButterworth
Copy link
Member

@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
Copy link
Contributor Author

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

VideoIO.jl/lib/libffmpeg.jl

Lines 21397 to 21449 in bd4061a

const AVERROR_BSF_NOT_FOUND = FFERRTAG(0xf8, Cchar('B'), Cchar('S'), Cchar('F'))
const AVERROR_BUG = FFERRTAG(Cchar('B'), Cchar('U'), Cchar('G'), Cchar('!'))
const AVERROR_BUFFER_TOO_SMALL = FFERRTAG(Cchar('B'), Cchar('U'), Cchar('F'), Cchar('S'))
const AVERROR_DECODER_NOT_FOUND = FFERRTAG(0xf8, Cchar('D'), Cchar('E'), Cchar('C'))
const AVERROR_DEMUXER_NOT_FOUND = FFERRTAG(0xf8, Cchar('D'), Cchar('E'), Cchar('M'))
const AVERROR_ENCODER_NOT_FOUND = FFERRTAG(0xf8, Cchar('E'), Cchar('N'), Cchar('C'))
const AVERROR_EOF = FFERRTAG(Cchar('E'), Cchar('O'), Cchar('F'), Cchar(' '))
const AVERROR_EXIT = FFERRTAG(Cchar('E'), Cchar('X'), Cchar('I'), Cchar('T'))
const AVERROR_EXTERNAL = FFERRTAG(Cchar('E'), Cchar('X'), Cchar('T'), Cchar(' '))
const AVERROR_FILTER_NOT_FOUND = FFERRTAG(0xf8, Cchar('F'), Cchar('I'), Cchar('L'))
const AVERROR_INVALIDDATA = FFERRTAG(Cchar('I'), Cchar('N'), Cchar('D'), Cchar('A'))
const AVERROR_MUXER_NOT_FOUND = FFERRTAG(0xf8, Cchar('M'), Cchar('U'), Cchar('X'))
const AVERROR_OPTION_NOT_FOUND = FFERRTAG(0xf8, Cchar('O'), Cchar('P'), Cchar('T'))
const AVERROR_PATCHWELCOME = FFERRTAG(Cchar('P'), Cchar('A'), Cchar('W'), Cchar('E'))
const AVERROR_PROTOCOL_NOT_FOUND = FFERRTAG(0xf8, Cchar('P'), Cchar('R'), Cchar('O'))
const AVERROR_STREAM_NOT_FOUND = FFERRTAG(0xf8, Cchar('S'), Cchar('T'), Cchar('R'))
const AVERROR_BUG2 = FFERRTAG(Cchar('B'), Cchar('U'), Cchar('G'), Cchar(' '))
const AVERROR_UNKNOWN = FFERRTAG(Cchar('U'), Cchar('N'), Cchar('K'), Cchar('N'))
const AVERROR_EXPERIMENTAL = -0x2bb2afa8
const AVERROR_INPUT_CHANGED = -0x636e6701
const AVERROR_OUTPUT_CHANGED = -0x636e6702
const AVERROR_HTTP_BAD_REQUEST = FFERRTAG(0xf8, Cchar('4'), Cchar('0'), Cchar('0'))
const AVERROR_HTTP_UNAUTHORIZED = FFERRTAG(0xf8, Cchar('4'), Cchar('0'), Cchar('1'))
const AVERROR_HTTP_FORBIDDEN = FFERRTAG(0xf8, Cchar('4'), Cchar('0'), Cchar('3'))
const AVERROR_HTTP_NOT_FOUND = FFERRTAG(0xf8, Cchar('4'), Cchar('0'), Cchar('4'))
const AVERROR_HTTP_OTHER_4XX = FFERRTAG(0xf8, Cchar('4'), Cchar('X'), Cchar('X'))
const AVERROR_HTTP_SERVER_ERROR = FFERRTAG(0xf8, Cchar('5'), Cchar('X'), Cchar('X'))
.

@IanButterworth
Copy link
Member

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 :)

@IanButterworth IanButterworth changed the title Update generator Update Clang wrapper generator and wrapper code Nov 12, 2021
@IanButterworth IanButterworth merged commit 0a710e8 into JuliaIO:master Nov 12, 2021
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

Successfully merging this pull request may close these issues.

Cannot generate FFMPEG bindings
3 participants