JuliaIO / VideoIO.jl

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

Test new version FFMPEG #418

Open theogf opened 4 months ago

theogf commented 4 months ago

As suggested in https://github.com/JuliaIO/FFMPEG.jl/pull/59#discussion_r1492764393, this should test if VideoIO runs correctly with the new version o FFMPEG

theogf commented 4 months ago

This segfault does not look good :sweat_smile:

giordano commented 4 months ago

For the benefit of future (and present) readers, the segmentation fault @theogf refers to is

[6222] signal (11.2): Segmentation fault: 11
in expression starting at /Users/runner/work/VideoIO.jl/VideoIO.jl/src/VideoIO.jl:203
av_freep at /Users/runner/.julia/artifacts/112745bebf2bab1be3313a35fda52d857c5b4ab4/lib/libavutil.58.29.100.dylib (unknown line)
avcodec_parameters_from_context at /Users/runner/.julia/artifacts/112745bebf2bab1be3313a35fda52d857c5b4ab4/lib/libavcodec.60.31.102.dylib (unknown line)

In particular, it's happening to https://github.com/JuliaIO/VideoIO.jl/blob/0398aa6be81a1e637132a39136ebad94afd2f196/src/VideoIO.jl#L208 My loose understanding is that VideoIO use only CLI ffmpeg, so it shouldn't be too complicated to craft a reproducible example to report upstream for someone motivated to push for FFMPEG v6, otherwise we'd have to yank that version, if it's broken (and will also be stuck forever to v4 until someone else does that anyway)

giordano commented 4 months ago

Actually, I was looking for ccall in src/, but I just realised that there are thousands of ccalls in https://github.com/JuliaIO/VideoIO.jl/blob/0398aa6be81a1e637132a39136ebad94afd2f196/lib/libffmpeg.jl, I guess you'd need to update the generated wrappers, too. Side note, this means that VideoIO pretty much needs to specify FFMPEG_jll explicitly.

giordano commented 4 months ago

@theogf bump

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.34%. Comparing base (a2bc9a9) to head (ef6db25).

:exclamation: Current head ef6db25 differs from pull request most recent head 3bedf4b. Consider uploading reports for the commit 3bedf4b to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #418 +/- ## ========================================== - Coverage 79.01% 78.34% -0.67% ========================================== Files 10 10 Lines 1282 1284 +2 ========================================== - Hits 1013 1006 -7 - Misses 269 278 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

theogf commented 4 months ago

Actually, I was looking for ccall in src/, but I just realised that there are thousands of ccalls in https://github.com/JuliaIO/VideoIO.jl/blob/0398aa6be81a1e637132a39136ebad94afd2f196/lib/libffmpeg.jl, I guess you'd need to update the generated wrappers, too. Side note, this means that VideoIO pretty much needs to specify FFMPEG_jll explicitly.

I am not sure what you mean here. One wrapper is for example

function avcodec_decode_subtitle2(avctx, sub, got_sub_ptr, avpkt)
    ccall((:avcodec_decode_subtitle2, libavcodec), Cint, (Ptr{AVCodecContext}, Ptr{AVSubtitle}, Ptr{Cint}, Ptr{AVPacket}), avctx, sub, got_sub_ptr, avpkt)
end

Is the idea to just make sure that libavcodec directly comes from FFMPEG_jll? Or by update you mean to make sure that all wrappers are compatible with 6.1?

giordano commented 4 months ago

Or by update you mean to make sure that all wrappers are compatible with 6.1?

Yes, but you don't do that manually, but run the script in https://github.com/JuliaIO/VideoIO.jl/tree/42b25a21c0ec524d75bfead8209a0bfd61ca91b5/gen

theogf commented 4 months ago

So I generated libffmpeg.jl again, (also restricting the dependency to FFMPEG_jll). However I get the error

Failed to precompile VideoIO [d6d074c3-1acf-5d4c-9a43-ef38773959a2] to "/home/theo/.julia/compiled/v1.10/VideoIO/jl_nKnqkE".
ERROR: LoadError: UndefVarError: `AV_CHANNEL_LAYOUT_MASK` not defined

I located the corresponding C code here https://github.com/FFmpeg/FFmpeg/blob/84e669167de9394e0de1bee0244f48ffac6f3826/libavutil/channel_layout.h#L378

But I don't know how to translate this to Julia? (and add it to prologue.jl)

giordano commented 4 months ago

That's a macro, it doesn't really have a correspondence in julia since it's a source-code transformation, not part of the ABI of the library. I don't know what's going on there.

theogf commented 4 months ago

That's a macro, it doesn't really have a correspondence in julia since it's a source-code transformation, not part of the ABI of the library. I don't know what's going on there.

Ah I think it's just some constructor for AVChannelLayout which is defined just above.

giordano commented 4 months ago

@Gnimuc do you have a clue how to deal with this? Clang.jl correctly defined the macros https://github.com/JuliaIO/VideoIO.jl/blob/a9384d99cf0b659339f3380516b62a530ae18bd2/lib/libffmpeg.jl#L22098-L22104 etc., but the macro AV_CHANNEL_LAYOUT_MASK has not been expanded.

Gnimuc commented 4 months ago
AV_CHANNEL_LAYOUT_MASK(nb, m) \
    { /* .order */ AV_CHANNEL_ORDER_NATIVE, \
      /* .nb_channels */  (nb), \
      /* .u.mask */ { m }, \
      /* .opaque */ NULL }

unfortunately, Clang.jl doesn't support such "convoluted" macros. Honestly, I don't even know how to manually translate this function-like macro into Julia...

if there is no direct use, it's safe to ignore all of the AV_CHANNEL* macro identifiers via the .toml file.

giordano commented 4 months ago

if there is no direct use, it's safe to ignore all of the AV_CHANNEL* macro identifiers via the .toml file.

Good point, it looks like all the AV_CHANNEL_LAYOUT_* macros are only defined in the resulting file, but not used. Thanks!

theogf commented 4 months ago

Just a small update, adding the right regex worked, but now there are some deprecated functions I need to find replacement for, namely av_stream_get_r_frame_rate

theogf commented 4 months ago

Hopefully it should work now :sweat_smile: