JuliaIO / VideoIO.jl

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

Update to the new Images #92

Closed timholy closed 7 years ago

timholy commented 7 years ago

This performs a couple of cleanup operations, and then updates for recent changes in the FixedPointNumbers/Images landscape. Most significantly, it decouples Video from Images itself. In this version I have it relying on ImageCore (a much smaller package), but even that isn't really necessary: I think the only function I use from there is permuteddimsview, and that's basically a wrapper around Base.PermutedDimsArrays.PermutedDimsArray. (I use a little bit more functionality for the tests.) If you'd prefer to decouple entirely, that would be fine.

It does change the output type returned by read: rather than an Image that wraps an Array{UInt8}, it returns a (lazy-transposed, i.e., permuted) Array{RGB{N0f8}}. As a consequence, this is a breaking change.

This bumps the minimum julia version requirement, so if you like it this will need a minor-version bump when tagged.

timholy commented 7 years ago

The 0.5 stall is puzzling. Tests work for me locally, but since both instances failed it seems likely to be real. We could try running on Trusty, I suppose?

kmsquire commented 7 years ago

@timholy, thanks for this! I've been planning to try to decouple Images (and ImageView) for a long time, but it's taken the back burner.

I'll take a closer look at this tomorrow or Friday.

timholy commented 7 years ago

I suspect that if we make it separate from ImageView, too, then we can turn on precompilation.

For reference:

Neither of those is necessary for this, but in the future it can become a little cleaner.

tkelman commented 7 years ago

How's this coming?

It looks like Compat is used in quite a few more places that aren't all touched here, which may need to be if it's being removed from REQUIRE.

kmsquire commented 7 years ago

For the failing v0.5 tests, the video reading just seems incredibly slow. One way to get the tests to pass would be to print something every 100 frames or shorten the total number of frames being read.

timholy commented 7 years ago

Hmm. I developed this on 0.5, and for me they aren't slow:

$ time julia-0.5 runtests.jl 
VideoFile:
   name:         annie_oakley.ogg  (Downloaded)
   description:  The "Little Sure Shot" of the "Wild West," exhibition of rifle shooting at glass balls.
   license:      pubic_domain (US)
   credit:       
   source:       http://commons.wikimedia.org/wiki/File:Annie_Oakley_shooting_glass_balls,_1894.ogg
   download_url: https://upload.wikimedia.org/wikipedia/commons/8/87/Annie_Oakley_shooting_glass_balls%2C_1894.ogv

VideoFile:
   name:         crescent-moon.ogv  (Downloaded)
   description:  Moonset (time-lapse).
   license:      Creative Commons Attribution 2.0 Generic (http://creativecommons.org/licenses/by/2.0/deed)
   credit:       Photo : Thomas Bresson
   source:       http://commons.wikimedia.org/wiki/File:2010-10-10-Lune.ogv
   download_url: http://upload.wikimedia.org/wikipedia/commons/e/ef/2010-10-10-Lune.ogv

VideoFile:
   name:         ladybird.mp4  (Downloaded)
   description:  Ladybird opening wings (slow motion)
   license:      Creative Commons: By Attribution 3.0 Unported (http://creativecommons.org/licenses/by/3.0/deed)
   credit:       CC-BY NatureClip (http://youtube.com/natureclip)
   source:       http://downloadnatureclip.blogspot.com/p/download-links.html
   download_url: https://archive.org/download/LadybirdOpeningWingsCCBYNatureClip/Ladybird%20opening%20wings%20CC-BY%20NatureClip.mp4

VideoFile:
   name:         black_hole.webm  (Downloaded)
   description:  Artist’s impression of the black hole inside NGC 300 X-1 (ESO 1004c)
   license:      Creative Commons Attribution 3.0 Unported (http://creativecommons.org/licenses/by/3.0/deed)
   credit:       Credit: ESO/L. Calçada
   source:       http://www.eso.org/public/videos/eso1004a/
   download_url: http://upload.wikimedia.org/wikipedia/commons/1/13/Artist%E2%80%99s_impression_of_the_black_hole_inside_NGC_300_X-1_%28ESO_1004c%29.webm

Testing file reading...
   Testing annie_oakley.ogg...
   Testing crescent-moon.ogv...
   Testing ladybird.mp4...
   Testing black_hole.webm...
Testing IO reading...
   Testing annie_oakley.ogg...
   Testing crescent-moon.ogv...
   Testing black_hole.webm...

real    0m26.357s
user    0m37.076s
sys     0m0.720s

Can you profile it to see where the problem is?

I'm traveling soon so I won't be in frequent touch.

kmsquire commented 7 years ago

(Testing this idea now in kms/teh/newimages...)

kmsquire commented 7 years ago

Sorry @timholy, I just saw your last comment.

I think they might just be slow on travis machines. They're definitely not slow on a Mac either.

kmsquire commented 7 years ago

So far, tests just seem to hang when reading mp4 files. At least, none of the print statements I added work. The correct libraries seem to be installed....

Anyway, I'm going to try to reproduce locally using the travis docker images.

kmsquire commented 7 years ago

Turns out that --bounds-check=yes is the culprit for the timeout on Travis. If I have time, I'll explore further, but for now, I'm just going to turn it off.
It's a bit unfortunate that we won't be checking bounds, but if the tests won't run with it...

kmsquire commented 7 years ago

And... that didn't work. :-( At a little bit of a loss, but will try to investigate more as I have time.

kmsquire commented 7 years ago

A little more exploration: it turns out the slowdown (on Travis machines) is in notblank().

I attempted to rewrite the maps of anonymous functions using loops, but that didn't help. :frowning_face:

@timholy, any thoughts? I'll attempt to look more into this when I have time, but not sure when.

timholy commented 7 years ago

I'm having trouble understanding how big of a slowdown you're talking about. One possible way to diagnose would be to disable one or the other of several tests, e.g., test only the green color channel or something.

tkelman commented 7 years ago

if it's just hitting the 10 minutes with no output thing on travis, there is a travis_wait command you can use - https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received

timholy commented 7 years ago

I was able to replicate the problem locally with Pkg.test("VideoIO", coverage=true). Turning off coverage fixes the problem (locally at least). Weird. I don't have time to dig deeper, so disabling coverage seems like the expedient solution.

timholy commented 7 years ago

Looks like that worked. There's an error on 0.6,

UndefVarError: __va_list_tag not defined

which comes from one of the libav log.jl files. I wasn't able to find any place where this variable was defined, so I'm not quite sure how to go about fixing it.

But at least everything works on 0.5.

kmsquire commented 7 years ago

I'm having trouble understanding how big of a slowdown you're talking about. One possible way to diagnose would be to disable one or the other of several tests, e.g., test only the green color channel or something.

It was taking over 2 minutes to test whether a frame was blank! I also had thought about testing in the way you said, but I honestly just didn't have time.

if it's just hitting the 10 minutes with no output thing on travis, there is a travis_wait command you can use - https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received

While debugging, I was able to output enough debug statements that this wasn't a problem. Instead, it was running over the total time allotted.

Turning off coverage fixes the problem (locally at least). Weird. I don't have time to dig deeper, so disabling coverage seems like the expedient solution.

Looks like that worked. There's an error on 0.6,

Thanks! The v0.6 error shouldn't be that hard to fix (it's caused by trying to let some C vararg functions be wrapped... but really, there's no real reason to be wrapping the libav log.jl file anyway.)

kmsquire commented 7 years ago

...but really, there's no real reason to be wrapping the libav log.jl file anyway

Except if you want to set the log level to reduce the number of warning messages libav/ffmpeg write.

Anyway, I have some easy fixes for this and a few other cleanups that I'll push in a few minutes.