Update #2 to julia v1

[yakir12]

added some required packages for the tests, tests now pass in julia 1.

[perrutquist]

Thanks!

[yakir12]

I'm trying to add some key word arguments so that we'd be able to control things like start time, duration, frame rate, and dimensions (width x height). These are controlled with the ffmpeg flags: ss, t, r, and s. But it doesn't seem to work. Is the frame size set?

[perrutquist]

The command line flags are missing. You should pass "-r $r" etc. Also, -r will have no effect when opening a video for reading, and it is already present when writing. As for the dimensions, those will be given by the first image written and do not have to be passed explicitly.

[yakir12]

Yea, I caught that shortly after pushing...
This is mainly for reading off of a video, with the intention of changing it from its original format. This is most useful when reading an overly heavy video file with high frame rates, high resolution, and dead time in the beginning and end.

[perrutquist]

I see. Maybe it would make sense to have input_options and output_options arguments. They could each take NamedTuples, so that not all possible ffmpeg flags have to be hard-coded in the wrapper.

[yakir12]

Hmf.. Having a hard time with interpolating pairs from NamedTuples into a command without it turning into strings:

nt = (ss = 1, t = 2, r = 25, s = "100x200")
opt = [`-$k $v ` for (k,v) in pairs(nt)]
cmd = `ffmpeg -i file.mp4 $opt`

results in ffmpeg -i file.mp4 '\`-ss 1\`' '\`-t 2\`' '\`-r 25\`' '\`-s 100x200\`' instead of simply ffmpeg -i file.mp4 -ss 1 -t 2 -r 25 -s 100x200

Any suggestions for this pretty basic hurdle...?

[perrutquist]

How about:

opt = Iterators.flatten([("-$k", "$v") for (k,v) in pairs(nt)])

[yakir12]

OK, this package kind of works for me now. Any plans on merging this, or should I add some more stuff (like the equivalent settings to the writing function)?

[perrutquist]

I'm a bit short on time right now, but I'll try to merge this next week. I'd still like to have the ffmpeg flags as two separate arguments (input_options and output_options) so that the user can decide which flag applies to which stream, instead of hard-coding that in the package.

[yakir12]

I'll look into it more carefully, but I think that's how it is now (you might have missed my last push). In any case, I thought that options is enough because the differentiation between input and output happens already before that (with the "r" and "w"). I'll take a look.

[perrutquist]

"w" and "r" control whether the Julia process is reading or writing. The ffmpeg process is always doing both. (Either reading from file and writing to Julia, or reading from julia and writing to file.) What I'm saying is the befores list should be removed, and the separation done by the user. Some options definitely make sense in both places. For example: one can get every third frame by setting the framerate on the input stream to 3 and the output stream to 1.

[yakir12]

Now I get you. Sorry, my bad. I mixed the IO of the julia Cmd with ffmpeg's in video and out video. Cool. I might have time in the next couple of days. Thanks for the explanation!

[yakir12]

@perrutquist, You already have a few key word arguments pre-defined in your openvideo command, how about we bake there into the same input output options? No real reason to separate them, or are there any specific reasons why you define these specific ones:

r=24, q=3, vcodec="h264", loglevel="fatal"

?

[yakir12]

Sorry for being so dense here, but do you mean that if a user wants to specify some input option on top of say specifying the output rate, they'll be able to do that with:

openvideo(filename, :r, r = 24, input_options = (ss = 1,))

?

[perrutquist]

I think we could get rid of the "r" and similar keyword arguments, and make input/output options positional arguments, so one would write:

openvideo(filename, :r, (ss = 1,), (r = 24,))

Maybe there needs to be a third category for arguments that don't apply to either stream, like loglevel, or the user can just put those in either of the two named tuples.

Maybe we later we add an interface with kwargs that get automatically sorted into those two named tuples if they en ind _in or _out. Then one could write:

openvideo(filename, :r, ss_in = 1, r_out = 24)

[yakir12]

Great idea. I'll try that out tomorrow.

[yakir12]

OK, so I found out that the easiest thing is to just build in this _in and _out mechanism into the kwargs signature you had going. No named tuples "needed". All arguments default to output ones except the ones that are explicitly _ined (or _outed). This works pretty nicely I think. Give it a try and let me know what you think.

[yakir12]

Also, side comment, don't you want to add ffmpeg as a build or a requirement? Either through the CI (I think I know how to do that) or bindeps, (I don't know how to do that...)?

[perrutquist]

Yes, it would definitely be good to include ffmpeg. I have no idea how to do this, which is the main reason why this is not yet a registered package. (I wrote this once because I needed to make a movie out of a series of plots, and after I was done with that I didn't have the time to make a proper package.)
I think an easy way to get ffmpeg in there would be to copy https://github.com/kmsquire/VideoIO.jl/blob/master/deps/build.jl by @kmsquire.
But maybe an even better way forward would be to move the functionality from this package into VideoIO.jl and not have two packages doing basically the same thing...

[yakir12]

Yes, it would definitely be good to include ffmpeg. I have no idea how to do this, which is the main reason why this is not yet a registered package.

I've now added all the ffmpeg related stuff needed for CI testing. So now the build and coverage badges will at least be green.

I think an easy way to get ffmpeg in there would be to copy https://github.com/kmsquire/VideoIO.jl/blob/master/deps/build.jl by @kmsquire.

I don't think that would work. I think VideoIO is not really ready for v1.

But maybe an even better way forward would be to move the functionality from this package into VideoIO.jl and not have two packages doing basically the same thing...

Finally, I don't think there is anything your package does that (a functioning) VideoIO can't do and do better (speed-wise at least). However, by shelling-out to ffmpeg and with the new key word interface you do have more flexibility than VideoIO.

So to sum it up, if the badges go green then the only thing left to do is to tell the user that a prerequisite for this package is ffmpeg and you're all good to go! Let me know what else you'd need to merge this fork and then you can publish it :)

[yakir12]

Ok, I'm done I think. It currently errors on windows (the part with reading off a file and writing that into a second one), not sure why. I also tagged a release on my fork.

[perrutquist]

Merged. Thanks!
What's left to do now is registering the package, and updating the documentation.
@yakir12 Would you like to take ownership of this project? (By which I mean register your branch with METADATA.jl) In that case I'd update the README on my branch to point to yours, and you'd be responsible for upkeep in the future. (Should be minimal work now that Julia 1.0 is out.)

[yakir12]

Sure, I'll re-read up on publishing in v1.. Thanks!

[yakir12]

done!