Closed techyian closed 6 years ago
Add CancellationToken support to BeginProcessing
method so allow easy termination of potentially long running tasks (mainly for Video capture and timelapse images). Going to keep the DateTime timeout against the Video Encoder/Decoder components to keep the ability to terminate these components ad-hoc when attached to splitter component - I think it's a nice feature to be able to do so.
The Video/Timelapse helper methods also will be updated to reflect this.
This is a redundant parameter and is a bit confusing currently. It's only being used to check the macroblock rate limit when using H.264 encoding. We can use the VideoFramerate
property in MMALCameraConfig
instead here.
MMALPortBase
These will be replaced with a new Resolution
property. Use this if you wish to get the current Width / Height of a given port.
CancellationToken support
Good idea.
Remove framerate parameter against Video Encoder/Decoder
Good idea. I hate redundant parameters.
Width / Height properties in
MMALPortBase
Good idea as we already use Resolution
in many other classes.
MMALCamera.BeginProcessing()
The name of MMALCamera.BeginProcessing(MMALPortImpl)
is very confusing because you would expect an APM Pattern with this method returning an IAsyncResult
and a corresponding method MMALCamera.EndProcessing(IAsyncResult)
.
With the new Task-based Asynchronous Pattern it should look like this:
public async Task ProcessAsync(MMALPortImpl cameraPort, CancellationToken ct)
Ok yes - agreed. Thanks for the feedback :) will keep this issue open during the course of this release.
Custom exceptions for all interop related errors is definitely a good idea. Also the PiCameraError
is useful in situations where the reason for an unexpected behavior is completely unknown.
However in situations like parameter checking you would expect a .NET Library rather to throw an ArgumentNullException
or ArgumentOutOfRangeException
than a PiCameraError
.
I would use predefined exceptions wherever possible. What do you think?
Yes this makes sense, thanks for picking that up. I think when I originally started the project, I wanted all exceptions to be seen as PiCamera specific but I agree that doesn't make sense when exception types thrown are already part of .NET.
In my case of use I want to take multiple images in raw mode without waiting two seconds after each to configure the camera again. Additionally I want to render the output of the preview port manually on my UI and not as global overlay. An option but bad style in my opinion would be making public
static void MMALPortExtensions.SetRawCapture(this MMALPortImpl port, bool raw);
I would prefer a public property for the Camera's port to configure these settings or a completely different public API for raw capture that also allows to take multiple images in a short time. In case we keep the low-level API for such complex scenarios, I would appreciate an example in the Wiki.
To clarify, the raw mode you are referring to tells the camera to send the Bayer data from the sensor, and is only supported with JPEG encoding.
I don't believe you should need to reconfigure the camera after simply disposing of an encoder, nor should you have to wait for the camera to warm up unless you've disabled the camera component. This is my fault for suggesting it was necessary in the "Change Encoding Type" examples in the wiki, I do apologise - copy and paste is bad! 👎
I believe there is also a way to do very fast image capture by connecting an Image Encoder to the camera's video port - I'd need to wire up another capture handler to support this though or edit the ImageStreamCaptureHandler
I think.
Just as a heads up, I've removed the redundant code from the "Change Encoding Type" examples, and also added some clarification on the camera warm up time code in the wiki examples page.
I just saw this Camera.StillPort.SetRawCapture(true)
call in the helper method TakeRawPicture
of MMALCamera
what I thought would produce an unencoded picture without Bayer data. If this call is not required in order to get an unencoded RGB bitmap, there is no API change necessary for my use cases.
Yep, removed it earlier in 0daf5b0 👍🏻
I think some classes in MMALCameraConfig.cs
could be replaced with structs. As example class Resolution
only holds to integer values. A struct which is allocated on the stack is slightly more efficient here. But immutability is much more important in this case. When using a struct the Pad method would return a new Resolution and nobody would have to take care about multiple usages of the same object.
If you agree I would work through these little classes and find out where structs would make sense.
This is a tricky one and I'm in two minds about it. The padding, as I'm sure you're aware, is to provide the firmware with the correct image stride (either padded 32x16 or 16x16). I'm unsure whether it's more helpful to the end user for a resolution to be automatically corrected for them, and querying the width/height properties of their Resolution
object will produce the padded result, or change to your suggestion (which, by the way I do appreciate as being a more correct way of defining such data).
To enable/disable the annotation functionality, two new API methods have been added to MMALCamera
: EnableAnnotation
and DisableAnnotation
.
A valid AnnotateImage
object still needs to be assigned to MMALCameraConfig.Annotate
.
Due to the work done in #12, the TextColour
and BgColour
properties' types are now Color
structs - I think this makes it more user friendly. The U
and V
properties in ColourEffects
have also been replaced with a Color
struct.
Oh, and I hate the way UK and US have different spellings of Colour/Color.
I'm planning on re-working the way the managed buffer header callback code is done. Currently it's being done by calling some delegates in MMALPortBase
that are set when MMALCamera.ProcessAsync
is called. The issue we currently have is that it's not intuitive to set any kind of overridden callback delegate; so although a user can intercept byte[]
data via the capture handlers e.g. StreamCaptureHandler
, I don't think the capture handlers are the best place structurally for manipulating buffer data before saving to a stream of some sort.
I've made a separate branch with my initial work in. It's really just an attempt at tidying up the way it's currently done, and abstracting the code to somewhere separate so it's easier to maintain.
If anyone has any strong objections or suggestions please let me know - it's still a WIP. My current changes do not require any alterations to user's code.
Merged callback rework in 0be5f94.
Also re-worked the StreamCaptureHandler
so it is now a generic class using a Stream
type constraint. Included a new MemoryStreamCaptureHandler
.
Will add examples to the wiki regarding callback rework.
Raised to discuss API changes for the 0.4 release