fix framerate miscalculate fps for some codecs (#400)

src/avio.jl

@@ -117,7 +117,7 @@ convert(::Type{AVRational}, r::Rational) = AVRational(numerator(r), denominator(
 function pump(avin::AVInput)
     while avin.isopen && !avin.finished
         ret = disable_sigint() do
-            av_read_frame(avin.format_context, avin.packet)
+            return av_read_frame(avin.format_context, avin.packet)
         end
 
         if ret < 0
@@ -299,7 +299,7 @@ function VideoReader(
     # Open the decoder
     ret = disable_sigint() do
         lock(VIO_LOCK) do
-            avcodec_open2(codec_context, codec, C_NULL)
+            return avcodec_open2(codec_context, codec, C_NULL)
         end
     end
     ret < 0 && error("Could not open codec")
@@ -393,7 +393,22 @@ function aspect_ratio(f::VideoReader)
     return fixed_aspect
 end
 
-framerate(f::VideoReader) = f.codec_context.time_base.den // f.codec_context.time_base.num
+# From https://www.ffmpeg.org/doxygen/trunk/structAVCodecContext.html
+#=
+# time_base
+
+This is the fundamental unit of time (in seconds) in terms of which frame timestamps are represented.
+For fixed-fps content, timebase should be 1/framerate and timestamp increments should be identically 1. This often, but not always is the inverse of the frame rate or field rate for video. 1/time_base is not the average frame rate if the frame rate is not constant.
+Like containers, elementary streams also can store timestamps, 1/time_base is the unit in which these timestamps are specified. As example of such codec time base see ISO/IEC 14496-2:2001(E) vop_time_increment_resolution and fixed_vop_rate (fixed_vop_rate == 0 implies that it is different from the framerate)
+
+# ticks_per_frame
+
+For some codecs, the time base is closer to the field rate than the frame rate.
+Most notably, H.264 and MPEG-2 specify time_base as half of frame duration if no telecine is used ...
+Set to time_base ticks per frame. Default 1, e.g., H.264/MPEG-2 set it to 2. 
+=#
+framerate(f::VideoReader) =
+    f.codec_context.time_base.den // f.codec_context.time_base.num // f.codec_context.ticks_per_frame
 height(f::VideoReader) = f.codec_context.height
 width(f::VideoReader) = f.codec_context.width
 
@@ -965,15 +980,15 @@ function close(avin::AVInput)
             avformat_close_input(avin.format_context) # This is also done in the finalizer,
             # but closing the input here means users won't have to wait for the GC to run
             # before trying to remove the file
-            avin.format_context = AVFormatContextPtr(Ptr{AVFormatContext}(C_NULL))
+            return avin.format_context = AVFormatContextPtr(Ptr{AVFormatContext}(C_NULL))
         end
     end
 
     if check_ptr_valid(avin.avio_context, false)
         # Replace the existing object in avin with a null pointer. The finalizer
         # will close it and clean up its memory
         disable_sigint() do
-            avin.avio_context = AVIOContextPtr(Ptr{AVIOContext}(C_NULL))
+            return avin.avio_context = AVIOContextPtr(Ptr{AVIOContext}(C_NULL))
         end
     end
     return nothing

src/testvideos.jl

@@ -19,6 +19,35 @@ mutable struct VideoFile{compression}
     numframes::Int
     testframe::Int
     summarysize::Int
+
+    fps::Union{Nothing,Rational}
+
+    VideoFile{compression}(
+        name::AbstractString,
+        description::AbstractString,
+        license::AbstractString,
+        credit::AbstractString,
+        source::AbstractString,
+        download_url::AbstractString,
+        numframes::Int,
+        testframe::Int,
+        summarysize::Int,
+        fps::Rational,
+    ) where {compression} =
+        new(name, description, license, credit, source, download_url, numframes, testframe, summarysize, fps)
+
+    VideoFile{compression}(
+        name::AbstractString,
+        description::AbstractString,
+        license::AbstractString,
+        credit::AbstractString,
+        source::AbstractString,
+        download_url::AbstractString,
+        numframes::Int,
+        testframe::Int,
+        summarysize::Int,
+    ) where {compression} =
+        new(name, description, license, credit, source, download_url, numframes, testframe, summarysize, nothing)
 end
 
 show(io::IO, v::VideoFile) = print(
@@ -39,6 +68,9 @@ VideoFile:
 VideoFile(name, description, license, credit, source, download_url, numframes, testframe, summarysize) =
     VideoFile{:raw}(name, description, license, credit, source, download_url, numframes, testframe, summarysize)
 
+VideoFile(name, description, license, credit, source, download_url, numframes, testframe, summarysize, fps) =
+    VideoFile{:raw}(name, description, license, credit, source, download_url, numframes, testframe, summarysize, fps)
+
 # Standard test videos
 const videofiles = Dict(
     "ladybird.mp4" => VideoFile(
@@ -85,6 +117,19 @@ const videofiles = Dict(
         1,
         4816,
     ),
+    "Big_Buck_Bunny_360_10s_1MB.mp4" => VideoFile(
+        "Big_Buck_Bunny_360_10s_1MB.mp4",
+        "Big Buck Bunny",
+        "Creative Commons: By Attribution 3.0 Unported (http://creativecommons.org/licenses/by/3.0/deed)",
+        "Credit: Blender Foundation | www.blender.org",
+        "https://peach.blender.org/",
+        "https://test-videos.co.uk/vids/bigbuckbunny/mp4/h264/360/Big_Buck_Bunny_360_10s_1MB.mp4",
+        300,
+        2,
+        207376840,
+        # Can be also 30000/1001
+        30 // 1,
+    ),
 )
 
 """

test/reading.jl

@@ -1,13 +1,13 @@
 @testset "Reading of various example file formats" begin
-    swscale_options = (sws_flags = "accurate_rnd+full_chroma_inp+full_chroma_int",)
+    swscale_options = (sws_flags="accurate_rnd+full_chroma_inp+full_chroma_int",)
     for testvid in values(VideoIO.TestVideos.videofiles)
         name = testvid.name
         test_frameno = testvid.testframe
         @testset "Reading $(testvid.name)" begin
             testvid_path = joinpath(@__DIR__, "../videos", name)
             comparison_frame = make_comparison_frame_png(load, testvid_path, test_frameno)
             f = VideoIO.testvideo(testvid_path)
-            v = VideoIO.openvideo(f; swscale_options = swscale_options)
+            v = VideoIO.openvideo(f; swscale_options=swscale_options)
             try
                 time_seconds = VideoIO.gettime(v)
                 @test time_seconds == 0
@@ -21,8 +21,6 @@
                     trimmed_comparison_frame = comparison_frame
                 end
 
-                @test VideoIO.framerate(v) != 0
-
                 # Find the first non-trivial image
                 first_img = read(v)
                 first_time = VideoIO.gettime(v)
@@ -31,6 +29,16 @@
                 @test VideoIO.gettime(v) == first_time
                 @test img == first_img
                 @test size(img) == VideoIO.out_frame_size(v)[[2, 1]]
+
+
+                # First read(v) then framerate(v)
+                # https://github.com/JuliaIO/VideoIO.jl/issues/349
+                if !isnothing(testvid.fps)
+                    @test isapprox(VideoIO.framerate(v), testvid.fps, rtol=0.01)
+                else
+                    @test VideoIO.framerate(v) != 0
+                end
+
                 # no scaling currently
                 @test VideoIO.out_frame_size(v) == VideoIO.raw_frame_size(v)
                 @test VideoIO.raw_pixel_format(v) == 0 # true for current test videos
@@ -86,7 +94,7 @@
                 @test_throws(ArgumentError, VideoIO.read_raw!(v, similar(buff, size(buff) .- 1)))
                 @test_throws MethodError VideoIO.read_raw!(v, similar(buff, Int))
                 @test VideoIO.gettime(v) == last_time
-                notranscode_buff = VideoIO.openvideo(read, testvid_path, transcode = false)
+                notranscode_buff = VideoIO.openvideo(read, testvid_path, transcode=false)
                 @test notranscode_buff == buff_bak
 
                 # read first frames again, and compare
@@ -151,16 +159,16 @@ end
 @testset "Reading monochrome videos" begin
     testvid_path = joinpath(@__DIR__, "../videos", "annie_oakley.ogg")
     # Test that limited range YCbCr values are translated to "full range"
-    minp, maxp = VideoIO.openvideo(get_video_extrema, testvid_path, target_format = VideoIO.AV_PIX_FMT_GRAY8)
+    minp, maxp = VideoIO.openvideo(get_video_extrema, testvid_path, target_format=VideoIO.AV_PIX_FMT_GRAY8)
     @test typeof(minp) == Gray{N0f8}
     @test minp.val.i < 16
     @test maxp.val.i > 235
     # Disable automatic rescaling
     minp, maxp = VideoIO.openvideo(
         get_video_extrema,
         testvid_path,
-        target_format = VideoIO.AV_PIX_FMT_GRAY8,
-        target_colorspace_details = VideoIO.VioColorspaceDetails(),
+        target_format=VideoIO.AV_PIX_FMT_GRAY8,
+        target_colorspace_details=VideoIO.VioColorspaceDetails(),
     )
     @test minp.val.i >= 16
     @test maxp.val.i <= 235
@@ -171,7 +179,7 @@ end
 @testset "Reading RGB video as monochrome" begin
     @testset "Iterative" begin
         io = VideoIO.testvideo("ladybird")
-        VideoIO.openvideo(io, target_format = VideoIO.AV_PIX_FMT_GRAY8) do f
+        VideoIO.openvideo(io, target_format=VideoIO.AV_PIX_FMT_GRAY8) do f
             img = read(f)
             for i in 1:10
                 read!(f, img)
@@ -181,15 +189,15 @@ end
     end
     @testset "Full load" begin
         testvid_path = joinpath(@__DIR__, "../videos", "ladybird.mp4")
-        vid = VideoIO.load(testvid_path, target_format = VideoIO.AV_PIX_FMT_GRAY8)
+        vid = VideoIO.load(testvid_path, target_format=VideoIO.AV_PIX_FMT_GRAY8)
         @test eltype(first(vid)) == Gray{N0f8}
     end
     GC.gc()
 end
 @memory_profile
 
 @testset "IO reading of various example file formats" begin
-    swscale_options = (sws_flags = "accurate_rnd+full_chroma_inp+full_chroma_int",)
+    swscale_options = (sws_flags="accurate_rnd+full_chroma_inp+full_chroma_int",)
     for testvid in values(VideoIO.TestVideos.videofiles)
         name = testvid.name
         test_frameno = testvid.testframe
@@ -199,7 +207,7 @@ end
             testvid_path = joinpath(@__DIR__, "../videos", name)
             comparison_frame = make_comparison_frame_png(load, testvid_path, test_frameno)
             filename = joinpath(videodir, name)
-            VideoIO.openvideo(filename; swscale_options = swscale_options) do v
+            VideoIO.openvideo(filename; swscale_options=swscale_options) do v
                 width, height = VideoIO.out_frame_size(v)
                 if size(comparison_frame, 1) > height
                     trimmed_comparison_frame = comparison_frame[1+size(comparison_frame, 1)-height:end, :]