New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frame rate adjustment #279

Closed
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@ethanrublee

ethanrublee commented Apr 25, 2016

Happy to amend, add tests, or take feedback.

@waveform80

This comment has been minimized.

Owner

waveform80 commented May 2, 2016

Interesting stuff - I've missed so much on the forums lately! Some notes on the PR:

  • Right now I'm in the midst of sticking a layer of stuff in between the PiCamera class and the MMAL layer to greatly simplify the code in PiCamera (basically this has to do with automating the setup and configuration of the MMAL pipeline and sorting out proper encodings for everything once and for all). End users shouldn't see any difference but it should make extensions like your proposed one considerably simpler to code.
  • Given the frame-rate parameter seems to be a rational, I'd prefer to use a Fraction for the value (simply for the accuracy)
  • The example is very long (admittedly so is the bayer recipe). I'd be tempted to stick a very simple snippet in the recipes chapter and then incorporate a more complete implementation with PID controllers into another project (compoundpi is probably a better place for this sort of thing - but I need to find some time to add init-sync to that anyway).

I'll leave this open for now because I sadly don't have the time to play with it right now - once I've got the new layer finished and pushed, I'll see if we can get this into the 1.11 release.

@ethanrublee

This comment has been minimized.

ethanrublee commented May 2, 2016

Thanks for the feedback. I updated it to use Fraction, and drastically simplified the example. I'm happy to contribute to compoundpi, ha didn't know it existed; will have a look.

@ethanrublee

This comment has been minimized.

ethanrublee commented May 4, 2016

This works well for the v1 camera module. Just tested it on v2 camera, and the video frame rate parameter does seem to work as expected. Need to dig a bit into why.

@waveform80

This comment has been minimized.

Owner

waveform80 commented May 5, 2016

To be honest if it only works on v1 that'd still be a good enough reason to include it (after all, it's the vast majority of current users :). The new layer is taking a bit longer than expected (I've only just got back to a point where both capture and recording fully work, but unencoded RGB/BGR captures are currently broken) - but I'll try and take a look at converting and including this if I get time at the weekend.

@waveform80 waveform80 added this to the 1.11 milestone May 5, 2016

@waveform80 waveform80 self-assigned this May 5, 2016

@ethanrublee

This comment has been minimized.

ethanrublee commented May 5, 2016

Ok, so turns out that limiting the denominator is necessary for the v2 board. Perhaps those rationales are really 16 bit, rather than 32 bit? Anyhow, tested on v1 and v2.

Thanks for the update.

If you need testers/reviewers for your big change; happy to try it out :)

# Need to limit the denominator, to something reasonable, I
# assume due to overflow. Can't find documentation on this,
# but 512 seems to work OK.
value = value.limit_denominator(512)

This comment has been minimized.

@6by9

6by9 May 26, 2016

As per https://www.raspberrypi.org/forums/viewtopic.php?f=43&t=48238&start=75#p908303, it's in 1/256th steps at the lowest level. There's going to be no gain in working in smaller steps than that.

camera.framerate = args.fps
# You may need to tune the PID values to
# get other values of framerate to work.
camera.framerate = 30
camera.exposure_mode = 'fixedfps'

This comment has been minimized.

@6by9

6by9 May 26, 2016

fixedfps exposure mode is a bit of a misnomer - it's not needed for fixed rate, as the frame rate parameter takes precedence anyway.

@waveform80

This comment has been minimized.

Owner

waveform80 commented May 26, 2016

Right, time to deal with this. I'm going to close this PR, but don't despair - the functionality will be going into 1.11 - just in a slightly different (easier) form. The basis for this is some experimenting with the VIDEO_FRAME_RATE parameter, and some offline discussions with @6by9. The upshot is as follows:

  • There's MMAL_PARAMETER_VIDEO_FRAME_RATE and MMAL_PARAMETER_FRAME_RATE. Under the covers there's no difference, so I'm going to use the shorter one :)
  • Via this parameter, the camera's frame-rate can be adjusted on the fly in increments of 1/256th (I don't think there's much point in picamera enforcing this limitation given the firmware will do it anyway, and we can read back the actual framerate with the same parameter)
  • Via this parameter, the camera's frame-rate can also be configured on the fly. I don't just mean in minor increments as above - you can change from 15 to 30fps just fine with that parameter.
  • In fact, you can even change the frame-rate on the fly such that the camera's mode changes (e.g. jumping from 30fps to 60fps which demands a binned mode) ... even while a recording's running!

I must admit I didn't realize the camera could just change framerate on the fly like this. So, for 1.11 what I'm going to do is re-write the framerate property entirely. Querying it will now return the framerate (read via MMAL_PARAMETER_FRAME_RATE) from the video port (if encoding is currently occurring) or the preview port (if it's not). Setting it will use MMAL_PARAMETER_FRAME_RATE to adjust the video and preview ports on the fly (no camera disable/enable because it's just not necessary). The still port will continue to use FPS_RANGE to set ranges when required for long exposures.

What does this mean for synchronization via minor framerate adjustments? Quite simply: no need for another parameter on PiCamera - just feed fractional values to the framerate property and the camera will be adjusted "live".

To @ethanrublee: apologies I couldn't merge this as it was - but many thanks for bringing the functionality to my attention! It's definitely worth including, and it will be there (in a slightly different form) in 1.11.

@waveform80 waveform80 closed this May 26, 2016

@ethanrublee

This comment has been minimized.

ethanrublee commented May 26, 2016

Sweet, no I don't mind. I've been using this now for a few weeks, and its been useful. Glad to get the functionality in somehow.

I'll test 1.11 when you have it ready :)

waveform80 added a commit that referenced this pull request May 26, 2016

Close #279
As discussed in PR #279, re-implement framerate to permit "live" changes
without calling camera_disable. This paves the way for synchronization
via minor increments/decrements to framerate.

Still need tests (and a good amount of testing), but the basics seem to
work happily.
@waveform80

This comment has been minimized.

Owner

waveform80 commented Jun 13, 2016

Hmm, looks like I'm going to have to redo this. I've been following the >100fps discussion on the 8MP Camera thread and there's several important points to take from ethanol100's patch. Firstly, a portion of the camera configuration (num_preview_video_frames) now derives from framerate, so the framerate property needs to disable and re-enable the camera to set that.

That in turn means framerate adjustment ought to be a separate property that doesn't modify the camera config and simply adjusts the framerate by minimal amounts (probably best to make it a delta to avoid confusion; i.e. default it to 0 so it doesn't need to shift value every time framerate changes).

@6by9

This comment has been minimized.

6by9 commented Jun 13, 2016

As per direct email, please ignore num_preview_video_frames changing with framerate.
It was requested to avoid a perceived increase in latency by having multiple buffers. In reality it will make no difference at all unless the client is too slow reading the resulting data. If you're slow reading data, then your code is the issue over most of the latency.

Use 10 frames and have done with it.

The one side I hadn't considered was memory requirements though. 10 frames at 1080P is fairly chunky at about 40MB. Perhaps further testing is required to see how many buffers are really needed to avoid starving the pipe.

@waveform80

This comment has been minimized.

Owner

waveform80 commented Jun 13, 2016

Yeah - I had a feeling memory might be an issue. I've been testing with 10, and (without tweaking gpu_mem) I can't set the maximum resolution on a V1 camera, let alone a V2 (I suspect this is partially down to picamera always using a full preview resolution too).

For now, I'll go with the variable level - it also keeps picamera closer to rapsivid, which is always nice. I'll probably have to re-visit the full preview resolution decision at some point though (it already causes issues with stereoscopic setups).

waveform80 added a commit that referenced this pull request Jun 14, 2016

Reimplement #279
Revert framerate to disabling / enabling the camera, and add a separate
framerate_delta property to handle live adjustments to the framerate.

@pyup-bot pyup-bot referenced this pull request Jun 12, 2017

Open

Update picamera to 1.13 #44

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment