savonet / ocaml-ffmpeg

OCaml bindings to the FFmpeg library.
http://www.liquidsoap.info/ocaml-ffmpeg/
GNU Lesser General Public License v2.1
48 stars 12 forks source link

Implement library exceptions, fix decoding logic. #31

Closed toots closed 5 years ago

toots commented 5 years ago

Okay, admittedly this got a little out of hands but eventually I believe that these are great changes. Here's the rundown:

I also changed the semantic a bit to follow the underlying library's. Typically, raise Eof instead of having a specific return type. This makes the C code much more simple and easy to follow..

Thanks to the awesome test suite, I believe that the PR is ready for review now. Will be testing it locally as well.

toots commented 5 years ago

@gndl, let me know how you feel about these changes. I think that it makes the binding better and leaner. I can try to clean them up more if you want.

gndl commented 5 years ago

The new exception format is a big improvement. I am less convinced by the immediate raising of exceptions (risk of memory leaks among others) but if you prefer this style, I do not object. I am therefore in favor of this changes.

toots commented 5 years ago

Great, thank you. I'll have another pass to double check about memory leaks, you are totally right about this.

toots commented 5 years ago

Just had another pass at it, this looks good to me now.

smimram commented 5 years ago

Please also adapt the code in Liquidsoap!

toots commented 5 years ago

Please also adapt the code in Liquidsoap!

It's coming!

toots commented 5 years ago

@gndl I'm gonna release this. There might still be bugs but I'll be ready to fix them asap and re-release. This will help smoothing the process for liquidsoap next release.