Test new version FFMPEG #418

theogf · 2024-02-17T12:27:18Z

As suggested in JuliaIO/FFMPEG.jl#59 (comment), this should test if VideoIO runs correctly with the new version o FFMPEG

.github/workflows/CI.yml

Co-authored-by: Mosè Giordano <[email protected]>

theogf · 2024-02-17T13:05:38Z

This segfault does not look good 😅

giordano · 2024-02-17T13:49:36Z

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

VideoIO.jl/src/VideoIO.jl

Line 208 in 0398aa6

VideoIO.save(fname, imgstack)

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 · 2024-02-17T13:53:32Z

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 · 2024-02-25T19:33:25Z

@theogf bump

codecov · 2024-02-26T12:33:40Z

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.49%. Comparing base (54ece49) to head (2913c85).

Files with missing lines	Patch %	Lines
src/avio.jl	50.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #418      +/-   ##
==========================================
- Coverage   78.50%   78.49%   -0.02%     
==========================================
  Files          10       10              
  Lines        1247     1246       -1     
==========================================
- Hits          979      978       -1     
  Misses        268      268

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Project.toml

theogf · 2024-02-26T13:02:28Z

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 · 2024-02-26T15:20:21Z

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 · 2024-02-26T16:10:14Z

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 · 2024-02-26T16:26:53Z

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 · 2024-02-26T16:31:50Z

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 · 2024-02-26T16:44:26Z

@Gnimuc do you have a clue how to deal with this? Clang.jl correctly defined the macros

VideoIO.jl/lib/libffmpeg.jl

Lines 22098 to 22104 in a9384d9

    
           const AV_CHANNEL_LAYOUT_MONO = AV_CHANNEL_LAYOUT_MASK(1, AV_CH_LAYOUT_MONO) 
        
           const AV_CHANNEL_LAYOUT_STEREO = AV_CHANNEL_LAYOUT_MASK(2, AV_CH_LAYOUT_STEREO) 
        
           const AV_CHANNEL_LAYOUT_2POINT1 = AV_CHANNEL_LAYOUT_MASK(3, AV_CH_LAYOUT_2POINT1) 
        
           const AV_CHANNEL_LAYOUT_2_1 = AV_CHANNEL_LAYOUT_MASK(3, AV_CH_LAYOUT_2_1)

etc., but the macro AV_CHANNEL_LAYOUT_MASK has not been expanded.

Gnimuc · 2024-02-27T12:53:45Z

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 · 2024-02-27T13:13:45Z

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 · 2024-02-29T21:54:31Z

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 · 2024-02-29T22:08:06Z

Hopefully it should work now 😅

Project.toml

gen/Project.toml

src/avio.jl

Co-authored-by: Ian Butterworth <[email protected]>

theogf · 2024-02-29T22:55:16Z

src/avio.jl

@@ -858,7 +858,7 @@ function seek_trim(r::VideoReader, seconds::Number)
    time_base = convert(Rational, stream.time_base)
    time_base == 0 && error("No time base")
    target_pts = seconds_to_timestamp(seconds, time_base)
-    frame_rate = convert(Rational, av_stream_get_r_frame_rate(stream))
+    frame_rate = convert(Rational, stream.r_frame_rate)


Tests are failing so I can only assume that this is not the solution?

This seems like the correct change based on https://patchwork.ffmpeg.org/project/ffmpeg/patch/[email protected]/

And the test failures could be rounding assumptions. Not looked deeper into it

Encoding with frame rate 29.5: Test Failed at /home/runner/work/VideoIO.jl/VideoIO.jl/test/writing.jl:196 Expression: parse(Float64, measured_dur_str[1]) == target_dur Evaluated: 3.389831 == 3.39

Encoding with frame rate 29.5: Test Failed at /home/runner/work/VideoIO.jl/VideoIO.jl/test/writing.jl:214 Expression: parse(Float64, measured_dur_str[1]) == target_dur Evaluated: 3.389831 == 3.39

We're a bit far from rounding error? Maybe it's a difference of 1 frame?

I think this is just correct

julia> 100 / 29.5 3.389830508474576 julia> 99 / 29.5 3.3559322033898304 julia> 101 / 29.5 3.4237288135593222

Perhaps ffmpeg -v error -show_entries format=duration -of default=noprint_wrappers=1:nokey=1 just returns more precision on newer versions.

I've pushed a change to the full precision

IanButterworth · 2024-12-23T12:47:40Z

It'd be great to get this figured out

IanButterworth · 2024-12-24T18:07:10Z

Project.toml

-FFMPEG = "0.3, 0.4"
-FFMPEG_jll = "4.1"
+FFMPEG = "0.3, 0.4, 1"
+FFMPEG_jll = "6.1"


Do we still need to directly depend on both?

#418 (comment)

IanButterworth · 2024-12-24T19:14:27Z

Project.toml

@@ -17,8 +17,8 @@ Scratch = "6c6a2e73-6563-6170-7368-637461726353"
 [compat]
 ColorTypes = "0.9, 0.10, 0.11, 0.12"
 Downloads = "1.3"
-FFMPEG = "0.3, 0.4"
-FFMPEG_jll = "4.1"
+FFMPEG = "0.3, 0.4, 1"


Suggested change

FFMPEG = "0.3, 0.4, 1"

FFMPEG = "0.4.3"

https://github.com/JuliaIO/FFMPEG.jl/pull/59/files#r1896927588

Test FFMPEG 6.1.1

56ab35d

giordano reviewed Feb 17, 2024

View reviewed changes

.github/workflows/CI.yml Outdated Show resolved Hide resolved

theogf and others added 2 commits February 17, 2024 13:41

Update .github/workflows/CI.yml

ae3d9a6

Co-authored-by: Mosè Giordano <[email protected]>

fix pkg spec

95ff370

IanButterworth mentioned this pull request Feb 18, 2024

add FFMPEG_jll as direct dep to control compat #419

Merged

theogf added 2 commits February 26, 2024 13:17

Merge branch 'master' into tgf/test-new-FFMPEG

9f4b868

Update Project.toml

242ee87

giordano reviewed Feb 26, 2024

View reviewed changes

Project.toml Outdated Show resolved Hide resolved

theogf added 2 commits February 26, 2024 16:45

update project

27115f8

update generate libffmpeg

a694d31

theogf added 2 commits February 26, 2024 17:28

Merge branch 'master' into tgf/test-new-FFMPEG

ef6db25

Merge remote-tracking branch 'refs/remotes/origin/tgf/test-new-FFMPEG'

a9384d9

fix generation

61d6c45

theogf added 2 commits February 29, 2024 23:05

replace accessor with getproperty

c31052d

remove av_register_all

6ba3993

IanButterworth reviewed Feb 29, 2024

View reviewed changes

Project.toml Outdated Show resolved Hide resolved

giordano reviewed Feb 29, 2024

View reviewed changes

gen/Project.toml Show resolved Hide resolved

giordano reviewed Feb 29, 2024

View reviewed changes

src/avio.jl Show resolved Hide resolved

theogf and others added 2 commits February 29, 2024 23:53

Update Project.toml

262509b

Co-authored-by: Ian Butterworth <[email protected]>

add gen Project FFMPEG_jll compat

19f38b7

theogf commented Feb 29, 2024

View reviewed changes

Merge branch 'master' into tgf/test-new-FFMPEG

3bedf4b

giordano mentioned this pull request Oct 19, 2024

Update FFMPEG to use 6.1.1 JuliaIO/FFMPEG.jl#59

Open

Merge branch 'master' into tgf/test-new-FFMPEG

30bb016

IanButterworth reviewed Dec 24, 2024

View reviewed changes

use greater full float precision and isapprox for test

f7f18a8

IanButterworth force-pushed the tgf/test-new-FFMPEG branch from a93e069 to f7f18a8 Compare December 24, 2024 18:08

Merge branch 'master' into tgf/test-new-FFMPEG

2913c85

IanButterworth reviewed Dec 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test new version FFMPEG #418

Test new version FFMPEG #418

theogf commented Feb 17, 2024

theogf commented Feb 17, 2024

giordano commented Feb 17, 2024

giordano commented Feb 17, 2024

giordano commented Feb 25, 2024

codecov bot commented Feb 26, 2024 •

edited

Loading

theogf commented Feb 26, 2024

giordano commented Feb 26, 2024

theogf commented Feb 26, 2024

giordano commented Feb 26, 2024

theogf commented Feb 26, 2024

giordano commented Feb 26, 2024

Gnimuc commented Feb 27, 2024

giordano commented Feb 27, 2024

theogf commented Feb 29, 2024 •

edited

Loading

theogf commented Feb 29, 2024

theogf Feb 29, 2024

IanButterworth Mar 2, 2024

theogf Mar 29, 2024

IanButterworth Dec 24, 2024

IanButterworth Dec 24, 2024

IanButterworth commented Dec 23, 2024

IanButterworth Dec 24, 2024

giordano Dec 24, 2024

IanButterworth Dec 24, 2024

Test new version FFMPEG #418

Are you sure you want to change the base?

Test new version FFMPEG #418

Conversation

theogf commented Feb 17, 2024

theogf commented Feb 17, 2024

giordano commented Feb 17, 2024

giordano commented Feb 17, 2024

giordano commented Feb 25, 2024

codecov bot commented Feb 26, 2024 • edited Loading

Codecov Report

theogf commented Feb 26, 2024

giordano commented Feb 26, 2024

theogf commented Feb 26, 2024

giordano commented Feb 26, 2024

theogf commented Feb 26, 2024

giordano commented Feb 26, 2024

Gnimuc commented Feb 27, 2024

giordano commented Feb 27, 2024

theogf commented Feb 29, 2024 • edited Loading

theogf commented Feb 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IanButterworth commented Dec 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 26, 2024 •

edited

Loading

theogf commented Feb 29, 2024 •

edited

Loading