acowley / ffmpeg-light

Minimal Haskell bindings to the FFmpeg library
BSD 3-Clause "New" or "Revised" License
67 stars 29 forks source link

Use av_strerror for FFmpeg error messages #43

Closed owickstrom closed 6 years ago

owickstrom commented 6 years ago

This should resolve #42. I've used the new stringError function for openFile, and for two other cases I found that seemed to make sense. There might be other places in other modules. Do you think I should go through those, or can that be done later (as someone works with those modules?) I couldn't find any other show r-like cases.

I didn't find any test suites to extend or run, but I've verified in my own project that the error message appears correctly (managed to trigger two different messages). The demos also seems to work as before.

Happy for any feedback!

acowley commented 6 years ago

This looks great, and incrementally adding support as people touch other parts of the API sounds reasonable to me.

I'd love to figure out a way to test things. I typically run the the demos, but it's manual, clunky, and not part of CI. The package is designed to be a thin layer over the FFI bindings, so there's not much logic to verify. What I could imagine is some smoke test type things where we get the number of frames from a video file, but I've been reluctant to do so since having proper video codecs available seems like a perennial problem. I suppose it's worth a shot... does everybody (for a useful approximation of "everybody") build with libx264 support?

This error message code you've added seems like something we definitely could test.

Let's go ahead and merge this, and when anybody gets a chance we can put together a minimal test module for an openFile error message. Once we have that, we can try adding in some other file queries to spot check that we're successfully binding to the underlying package.

owickstrom commented 6 years ago

Perfect, thank you!

Yes, I agree. I'll think a bit on how to test this and send a PR if I come up with something useful.