techyian / MMALSharp

C# wrapper to Broadcom's MMAL with an API to the Raspberry Pi camera.
MIT License
195 stars 33 forks source link

Logging debug output with Serilog #155

Closed MV10 closed 4 years ago

MV10 commented 4 years ago

Here's a note for your Logging topic in your wiki about how to set up Serilog. It doesn't normally work with LoggerFactory the way older loggers do (I used to be a frequent Serilog contributor), so it's kind of hard to track down how this is done because one of the packages has been mostly superseded by builder-helpers in other packages. The dependencies are Microsoft.Extensions.Logging, Serilog.Extensions.Logging, and Serilog.Sinks.File.

There are many other possible approaches, but at a minimum this gets the job done:

if (useDebug)
{
    File.Delete("debug.log"); // log output will append
    MMALCameraConfig.Debug = true;
    MMALLog.LoggerFactory = new LoggerFactory()
        .AddSerilog(new LoggerConfiguration()
            .MinimumLevel.Verbose()
            .WriteTo.File("debug.log")
            .CreateLogger());
}
techyian commented 4 years ago

Hi Jon,

Thanks for the info on setting up Serilog, I'll get that added to the Wiki. I've been trying to keep up with the other issues you've raised and happy to review any changes you feel would be appropriate when piping to StdOut/Err. I've not done much research into using the new Channel APIs, but the problem I have with using async code in that section of the library is that currently you're unable to await anything from the native callback methods as they're unsafe due to accepting native pointers as parameters; this could potentially be changed and accept IntPtr types instead, but I'd need to be clear on the performance benefits before making such changes as it'll get messy quick with casts all over the place and possible unsafe blocks. If you want to send a PR in with your proposed changes I'll review them and we can take it from there.

Thanks, Ian

MV10 commented 4 years ago

Yeah I was going in about three directions at once in that other topic, sorry about that. The MMALCamera.Cleanup exception I mentioned over there was because I was using the new one-liner using statement (no block in braces) which meant nothing was disposed until the method exits -- and I was calling Cleanup before that. So no problems there but my own.

Since you already had ProcessAsync running the show, the additional async changes didn't take over the world after all. Before I PR anything maybe have a peek at what ExternalProcessCaptureHandler turned into (my repo). It's considerably improved over what I started with the other day. If you like it, I'll PR it into this project. In addition to buffering the output, I also added the ability to specify one or more signals to cleanly terminate signal-aware apps. While that doesn't help with ffmpeg due to its quirks I mentioned in the other thread, it does allow clean termination of VLC, which I'm using for preview-quality MJPEG streaming direct to browsers.

From a package-consumer standpoint there are just two changes:

That extra task manages buffering and shutdown:

Apart from that it's basically the ffmpeg handler.

techyian commented 4 years ago

Notes added to wiki.