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

Test new version FFMPEG #418

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ jobs:
with:
version: ${{ matrix.julia-version }}
- uses: julia-actions/cache@v2
- name: Configure new FFMPEG version environment
shell: julia --project="." --color=yes {0}
run: |
using Pkg
Pkg.add(PackageSpec(url="https://github.com/theogf/FFMPEG.jl", rev="tgf/update-FFMPEG"))
Pkg.instantiate()
- uses: julia-actions/julia-runtest@v1
- uses: julia-actions/julia-processcoverage@v1
- uses: codecov/codecov-action@v5
Expand Down
4 changes: 2 additions & 2 deletions Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FFMPEG = "0.3, 0.4, 1"
FFMPEG = "0.4.3"

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

FFMPEG_jll = "6.1"
Comment on lines -20 to +21
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to directly depend on both?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FileIO = "1.6"
Glob = "1.2"
ImageCore = "0.8, 0.9, 0.10"
Expand Down
5 changes: 3 additions & 2 deletions gen/Project.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
[deps]
Clang = "40e3b903-d033-50b4-a0cc-940c62c95e31"
FFMPEG = "c87230d0-a227-11e9-1b43-d7ebe4e7570a"
FFMPEG_jll = "b22a6f82-2f65-5046-a5b2-351ab43fb4e5"
Match = "7eb4fadd-790c-5f42-8a69-bfa0b872bfbf"
Vulkan_Headers_jll = "8d446b21-f3ad-5576-a034-752265b9b6f9"

[compat]
Clang = "0.14.0"
Clang = "0.14, 0.15, 0.16, 0.17"
theogf marked this conversation as resolved.
Show resolved Hide resolved
FFMPEG_jll = "0.6.1"
5 changes: 2 additions & 3 deletions gen/generate.jl
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Clang.Generators
using FFMPEG
using FFMPEG.FFMPEG_jll
using FFMPEG_jll
using Vulkan_Headers_jll
using LibGit2
include("rewriter.jl")
Expand All @@ -9,7 +8,7 @@ cd(@__DIR__)

# Ideally I could have loaded all headers, but it turns out to be too hard

include_dir = joinpath(FFMPEG.FFMPEG_jll.artifact_dir, "include") |> normpath
include_dir = joinpath(FFMPEG_jll.artifact_dir, "include") |> normpath

vulkan_dir = joinpath(Vulkan_Headers_jll.artifact_dir, "include") |> normpath

Expand Down
3 changes: 2 additions & 1 deletion gen/generate.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ printer_blacklist = [
"AVPROBE_SCORE_MAX",
"Picture",
"AV_BPRINT_SIZE_UNLIMITED",
"FF_CEIL_RSHIFT"
"FF_CEIL_RSHIFT",
"AV_CHANNEL_LAYOUT_.*"
]
extract_c_comment_style = "doxygen"
[codegen]
Expand Down
12,746 changes: 6,849 additions & 5,897 deletions lib/libffmpeg.jl

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion src/VideoIO.jl
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ function __init__()
loglevel!(libffmpeg.AV_LOG_FATAL)
# @info "VideoIO: Low-level FFMPEG reporting set to minimal (AV_LOG_FATAL). See `? VideoIO.loglevel!` for options"
read_packet[] = @cfunction(_read_packet, Cint, (Ptr{AVInput}, Ptr{UInt8}, Cint))
av_register_all()
libffmpeg.avdevice_register_all()
end

Expand Down
8 changes: 4 additions & 4 deletions src/avio.jl
Original file line number Diff line number Diff line change
Expand Up @@ -832,10 +832,10 @@
function get_frame_period_timebase(r::VideoReader)
# This is probably not valid for many codecs, frame period in timebase
# units
stream = get_stream(s)
stream = get_stream(r)

Check warning on line 835 in src/avio.jl

View check run for this annotation

Codecov / codecov/patch

src/avio.jl#L835

Added line #L835 was not covered by tests
theogf marked this conversation as resolved.
Show resolved Hide resolved
time_base = convert(Rational, stream.time_base)
time_base == 0 && return nothing
frame_rate = convert(Rational, av_stream_get_r_frame_rate(stream))
frame_rate = convert(Rational, stream.r_frame_rate)

Check warning on line 838 in src/avio.jl

View check run for this annotation

Codecov / codecov/patch

src/avio.jl#L838

Added line #L838 was not covered by tests
frame_period_timebase = round(Int64, 1 / (frame_rate * time_base))
return frame_period_timebase
end
Expand All @@ -858,7 +858,7 @@
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)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a change to the full precision

frame_period_timebase = round(Int64, 1 / (frame_rate * time_base))
gotframe = pump_until_frame(r, false)
# If advancing another frame would still leave us before the target
Expand Down Expand Up @@ -1005,7 +1005,7 @@
time_base == 0 && return nothing
nqueued = n_queued_frames(r)
nqueued == 0 && return last_pts * time_base
frame_rate = convert(Rational, av_stream_get_r_frame_rate(stream))
frame_rate = convert(Rational, stream.r_frame_rate)
last_returned_pts = last_pts - round(Int, nqueued / (frame_rate * time_base))
return last_returned_pts * time_base
end
Expand Down
8 changes: 4 additions & 4 deletions test/writing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ end
@testset "Encoding video with rational frame rates" begin
n = 100
fr = 59 // 2 # 29.5
target_dur = 3.39
target_dur = 3.389831
@testset "Encoding with frame rate $(float(fr))" begin
imgstack = map(x -> rand(UInt8, 100, 100), 1:n)
encoder_options = (color_range = 2, crf = 0, preset = "medium")
Expand All @@ -193,14 +193,14 @@ end
command = VideoIO.FFMPEG.ffprobe,
collect = true,
)
@test parse(Float64, measured_dur_str[1]) == target_dur
@test parse(Float64, measured_dur_str[1]) target_dur
end
end

@testset "Encoding video with float frame rates" begin
n = 100
fr = 29.5 # 59 // 2
target_dur = 3.39
target_dur = 3.389831
@testset "Encoding with frame rate $(float(fr))" begin
imgstack = map(x -> rand(UInt8, 100, 100), 1:n)
encoder_options = (color_range = 2, crf = 0, preset = "medium")
Expand All @@ -211,6 +211,6 @@ end
command = VideoIO.FFMPEG.ffprobe,
collect = true,
)
@test parse(Float64, measured_dur_str[1]) == target_dur
@test parse(Float64, measured_dur_str[1]) target_dur
end
end
Loading