6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Mon Mar 09, 2015 7:45 am

jsh121988 wrote:Just wanted to let you know it is still occurring with the latest fw. I am using picamera, recording video to splitter port 1, and capturing with use_video_port. Don't worry about it though. I am just going to wait for picamera to support V3, then test it again. For now i have disabled the picture capture in my script.
Again, a simple test case please (not being grumpy, just too much on my plate to be trying to write tests which may behave totally differently to your app. The supported apps work as expected. If you can't share your full app publicly, then feel free to put it somewhere private and message me a link)

Unless picamera is doing something weird on annotate V2, or I've made a blunder on the backwards compatibility, then changing the client to V3 should have no effect.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

jsh121988
Posts: 11
Joined: Tue Feb 24, 2015 3:30 pm

Re: Annotate, any hope for future changes?

Mon Mar 09, 2015 2:51 pm

In essence, the below code is a simplified concept of what I am running. I just removed a bunch of variables. I probably imported unused headers for this sample, but I do use all of them.

Code: Select all

import picamera
import os
import sys
import threading
import socket
import math
import fractions
import time
import datetime as dt

camera = picamera.PiCamera()
camera.vflip = True
camera.hflip = True
camera.resolution = (1296, 972) #(width,height)
camera.awb_mode = 'off'
camera.awb_gains = (1.05, 1.40)
camera.meter_mode = 'matrix'
camera.annotate_background = True
camera.framerate = 5
camera.iso = 100
camera.shutter_speed = 0

#camera.start_recording(sys.stdout, format='h264', splitter_port=1) # I have the script piped to ffmpeg for live streaming
camera.start_recording('~/test.h264', format='h264', splitter_port=2)
camera.capture('~/test.jpg', format='jpeg', use_video_port=True)

iMainLoopInc = 0
fMainLoopTimeSleep = 0.5 # time in seconds for the main loop to sleep
iPictureIntervalSeconds = 30 # Take picture every 30 seconds
bCameraLed = True

while True: # Main Loop
	# Increment
	time.sleep(fMainLoopTimeSleep)
	iMainLoopInc += 1
	
	# Update Text
	camera.annotate_text = " Camera Name -- " + dt.datetime.now().strftime('%m/%d/%Y %H:%M:%S') + " -- Description "
		
	# Control camera LED to blink
	if bCameraLed == True:
		bCameraLed = False
	elif bCameraLed == False:
		bCameraLed = True
	
	camera.led = bCameraLed
	
	# Capture picture
	if math.fmod(iMainLoopInc, (iPictureIntervalSeconds / fMainLoopTimeSleep)) == 0:
		camera.capture('~/test.jpg', format='jpeg', use_video_port=True)

Last edited by jsh121988 on Mon Mar 09, 2015 5:33 pm, edited 1 time in total.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Mon Mar 09, 2015 3:07 pm

jsh121988 wrote:In essence, the below code is a simplified concept of what I am running. I just removed a bunch of variables. I probably imported unused headers for this sample, but I do use all of them.
Perfect. Thank you.
I'll try to give it a whirl tonight. It'll either be a client error or hopefully something trivial, but we'll wait and see.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

btidey
Posts: 1616
Joined: Sun Feb 17, 2013 6:51 pm

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 10:35 am

A few users of the RPi Web Cam Interface in this forum report the flickering annotation problem, alternating between small and large text. It is not clear to me what triggers this as not all systems suffer from this.

The web interface uses the raspimjpeg process to provide low level access using similar MMAL calls to those found in raspivid. Whilst in background mode it is actually using MMAL callbacks to capture a stream of medium resolution jpegs and it is these that provide the background display. So on some systems it appears that some of these capturse get different sized annotation giving the flickering effect.

Do we know if the MMAL calls have an influence here or are we waiting on some problem on the MMAL side to be understood and hopefully fixed at some point?

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 11:42 am

Sorry, this one had slipped off my radar and I had forgotten I had test code from jsh121988.
Raspistill and Raspivid both work fine with the new annotation code, so there is something funny about picamera and RaspiMJPEG.
I will try to find time tonight. I suspect it will be linked with using the old V2 config structure, but that's only a guess at the moment.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

btidey
Posts: 1616
Joined: Sun Feb 17, 2013 6:51 pm

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 1:31 pm

6by9 wrote:Sorry, this one had slipped off my radar and I had forgotten I had test code from jsh121988.
Raspistill and Raspivid both work fine with the new annotation code, so there is something funny about picamera and RaspiMJPEG.
I will try to find time tonight. I suspect it will be linked with using the old V2 config structure, but that's only a guess at the moment.
Thanks.

I think RaspiMJPEG is currently built in a userland from some time back and does use the V2 annotation config. I could also try moving it into the current userland and switching to V3 and see what happens. My set up doesn't exhibit the flickering so I would have to rely on others to test.

As an aside the userland buildme checks just for armv61 to determine native builds. I changed that to

Code: Select all

if [ "armv6l" = `arch` -o "armv71" = `arch` ]; then
for Pi2 as I find it builds pretty fast on that.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 1:55 pm

btidey wrote:I think RaspiMJPEG is currently built in a userland from some time back and does use the V2 annotation config. I could also try moving it into the current userland and switching to V3 and see what happens. My set up doesn't exhibit the flickering so I would have to rely on others to test.
Thank you. If you just amend the V2 to V3, and set
enable_text_background instead of black_text_background
custom_background_colour = MMAL_FALSE;
custom_text_colour = MMAL_FALSE
text_size = 0;

You should get the same behaviour as the old V2 flavour. The firmware should be doing that ... just checked. Bu**er. Forgot to set text size to 0 in the V2 mapping, so V2 clients will get a random text size via an unintialised variable - that explains the symptoms.
I set it correctly for V1 clients :(

I'll sort a patch tonight, but there's no harm in updating RaspiMJPEG to V3 too.
btidey wrote:As an aside the userland buildme checks just for armv61 to determine native builds. I changed that to

Code: Select all

if [ "armv6l" = `arch` -o "armv71" = `arch` ]; then
for Pi2 as I find it builds pretty fast on that.
Puzzling. On my Pi2 it was going through the armv6l path, as I was increasing the number of jobs to 4. I hadn't really thought about it any further than that.
Feel free to submit a pull request to the main userland repo to fix that. If you felt like bumping up the number of build jobs on Pi2 at the same time, then I wouldn't complain :)
I must admit that building on the Pi2 is much more pleasant than a Pi1. I just need to buy a few more now....
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

btidey
Posts: 1616
Joined: Sun Feb 17, 2013 6:51 pm

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 3:57 pm

6by9 wrote: Thank you. If you just amend the V2 to V3, and set
enable_text_background instead of black_text_background
custom_background_colour = MMAL_FALSE;
custom_text_colour = MMAL_FALSE
text_size = 0;

You should get the same behaviour as the old V2 flavour.

I'll sort a patch tonight, but there's no harm in updating RaspiMJPEG to V3 too.
[quote="btidey"
Puzzling. On my Pi2 it was going through the armv6l path, as I was increasing the number of jobs to 4.
OK. I built against latest userland OK and used v2 to start. Works fine.

I had done a quick try to V3 which worked ran but I got no annotation but wasn't sure what values to put in, so this is very helpful and I'll have another go.

The armv61 is strange; on my system it tried to use the external compiler and threw a complete wobbly until I put the armv71 in.

I will put a pull request in on that but I need to sort my fork out a bit first.

User avatar
waveform80
Posts: 303
Joined: Mon Sep 23, 2013 1:28 pm
Location: Manchester, UK

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 4:26 pm

6by9 wrote:
btidey wrote:I think RaspiMJPEG is currently built in a userland from some time back and does use the V2 annotation config. I could also try moving it into the current userland and switching to V3 and see what happens. My set up doesn't exhibit the flickering so I would have to rely on others to test.
Thank you. If you just amend the V2 to V3, and set
enable_text_background instead of black_text_background
custom_background_colour = MMAL_FALSE;
custom_text_colour = MMAL_FALSE
text_size = 0;

You should get the same behaviour as the old V2 flavour. The firmware should be doing that ... just checked. Bu**er. Forgot to set text size to 0 in the V2 mapping, so V2 clients will get a random text size via an unintialised variable - that explains the symptoms.
I set it correctly for V1 clients :(
Ah! That explains several of the issues I've been seeing when playing with V3 annotations in picamera. Thought I was going nuts there for a minute!

Dave.

btidey
Posts: 1616
Joined: Sun Feb 17, 2013 6:51 pm

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 4:33 pm

btidey wrote: I had done a quick try to V3 which worked ran but I got no annotation but wasn't sure what values to put in, so this is very helpful and I'll have another go.
Hmm must be missing something on V3. Runs OK but no annotation.

Code: Select all

  strcpy(anno.text, "Test");
  anno.enable = 1;
  anno.show_shutter = 0;
  anno.show_analog_gain = 0;
  anno.show_lens = 0;
  anno.show_caf = 0;
  anno.show_motion = 0;
  anno.show_frame_num = 0;
  anno.enable_text_background = cam_setting_annback;
  anno.text_size = 0;
  anno.custom_background_colour = MMAL_FALSE;
  anno.custom_text_colour = MMAL_FALSE;
Normally text and enable are set by user controls but I hardwired them for a test just to be sure. cam_setting_annback is also a user control. I tried making it true or false but no effect. Also tried setting text_size to 20 but no joy.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 4:37 pm

waveform80 wrote:Ah! That explains several of the issues I've been seeing when playing with V3 annotations in picamera. Thought I was going nuts there for a minute!
Using the V3 structure should always do the right thing. Are you really seeing issues there?

I suspect the reason I didn't see any issues in Raspistill/vid is that those set the annotation once at the start. I'd hazard a guess that Picamera and RaspiMJPEG are updating more regularly (timestamping via annotation?), and each call will potentially pick up a different random number from that uninitialised variable. Anything outside the valid range gets set to the default size, so generally it'd get that, but sometimes something else dependent on what rubbish was on the stack before.

I'll make no comments about your sanity ;)
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 4:42 pm

btidey wrote:Hmm must be missing something on V3. Runs OK but no annotation.

Code: Select all

  strcpy(anno.text, "Test");
  anno.enable = 1;
  anno.show_shutter = 0;
  anno.show_analog_gain = 0;
  anno.show_lens = 0;
  anno.show_caf = 0;
  anno.show_motion = 0;
  anno.show_frame_num = 0;
  anno.enable_text_background = cam_setting_annback;
  anno.text_size = 0;
  anno.custom_background_colour = MMAL_FALSE;
  anno.custom_text_colour = MMAL_FALSE;
Normally text and enable are set by user controls but I hardwired them for a test just to be sure. cam_setting_annback is also a user control. I tried making it true or false but no effect. Also tried setting text_size to 20 but no joy.
Hmm. Feel free to compare to https://github.com/raspberrypi/userland/pull/225 where I've added the parameter -ae (annotate_extra) to raspistill/vid. You may even be able to lift the code directly as I think RaspiMJPEG was originally based on Raspistill.

Teaching to suck eggs. You have updated to the new structure (must have done to get the new fields) and updated the size of the parameter in the MMAL_PARAMETER_HEADER_T? The firmware uses the structure size to determine which version of the structure it is being given.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

User avatar
waveform80
Posts: 303
Joined: Mon Sep 23, 2013 1:28 pm
Location: Manchester, UK

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 5:35 pm

6by9 wrote:
waveform80 wrote:Ah! That explains several of the issues I've been seeing when playing with V3 annotations in picamera. Thought I was going nuts there for a minute!
Using the V3 structure should always do the right thing. Are you really seeing issues there?
Yeah, definitely still seeing issues but then I haven't updated my firmware to your latest release yet (it's on #767 from uname) and I was definitely seeing the "random text size" thing. I thought at first I'd declared the structure wrong in my mmal header translation so that maybe some bytes of the text field were getting mis-interpreted as a size or something like that but after triple-checking I'd copied everything correctly and that sizeof() was reporting the same I started wondering about the dummy field; evaluating that showed random stuff appearing in there.

Still getting several other issues though - I can't seem to make any user text appear at the moment (stuff like frame-num works fine), and the background colour bits don't seem to want to play ball either (setting enable_text_background works, but then setting custom_background_colour results in a "?" appearing on the annotation and the background staying resolutely black). But I still need to check I'm not doing something random and stupid there.
6by9 wrote:I suspect the reason I didn't see any issues in Raspistill/vid is that those set the annotation once at the start. I'd hazard a guess that Picamera and RaspiMJPEG are updating more regularly (timestamping via annotation?), and each call will potentially pick up a different random number from that uninitialised variable. Anything outside the valid range gets set to the default size, so generally it'd get that, but sometimes something else dependent on what rubbish was on the stack before.
Basically picamera lets the user set the annotation whenever they please (the only limit I've placed on the property setters is that the camera hasn't been closed yet) - so they can set annotate_text or annotate_background and so on at any point after camera init (whether or not the preview is running, and in the middle of any recordings). Always seemed to be happy with the V2 structures, so I figured it'd be the same story for V3.

The other thing I'm slightly concerned about is that the new installation instructions for picamera 1.10 are going to de-emphasize using rpi-update (after documentation ticket #155). To that end I need to make sure that picamera 1.10 will work happily with people that only have annotate V2 capable firmwares.

I think that can do that in a reasonably backward compatible way but it's going to require some painful testing (I really should grab another Pi and set it up for testing with an old firmware for this rather than constantly swapping the firmware on my development Pi!)
6by9 wrote:I'll make no comments about your sanity ;)
A-bibble,

Dave.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 6:00 pm

waveform80 wrote:Yeah, definitely still seeing issues but then I haven't updated my firmware to your latest release yet (it's on #767 from uname) and I was definitely seeing the "random text size" thing. I thought at first I'd declared the structure wrong in my mmal header translation so that maybe some bytes of the text field were getting mis-interpreted as a size or something like that but after triple-checking I'd copied everything correctly and that sizeof() was reporting the same I started wondering about the dummy field; evaluating that showed random stuff appearing in there.

Still getting several other issues though - I can't seem to make any user text appear at the moment (stuff like frame-num works fine), and the background colour bits don't seem to want to play ball either (setting enable_text_background works, but then setting custom_background_colour results in a "?" appearing on the annotation and the background staying resolutely black). But I still need to check I'm not doing something random and stupid there.
Could you please grab my updated userland from https://github.com/6by9/userland/tree/PR20150310, build, and try

Code: Select all

raspistill -ae 64,0x00,0x8080ff "Wibble gibber\nThingy widget" -a 1025 -t 20000 -tl 2000 -o "output%d.jpg"
It should give black text on a white background. Likewise if you drop the -ae option, then you should get the default size text with white on black.
If that is misbehaving then I really have blundered. That would mean the structure is being misinterpreted which really shouldn't happen.
waveform80 wrote:Basically picamera lets the user set the annotation whenever they please (the only limit I've placed on the property setters is that the camera hasn't been closed yet) - so they can set annotate_text or annotate_background and so on at any point after camera init (whether or not the preview is running, and in the middle of any recordings). Always seemed to be happy with the V2 structures, so I figured it'd be the same story for V3.
My concern was initially that the firmware was spontaneously updating the text size. I suspect that is not the case, and just that RaspiMJPEG or Picamera are updating more often and getting random results via V2 structures.
waveform80 wrote:The other thing I'm slightly concerned about is that the new installation instructions for picamera 1.10 are going to de-emphasize using rpi-update (after documentation ticket #155). To that end I need to make sure that picamera 1.10 will work happily with people that only have annotate V2 capable firmwares.

I think that can do that in a reasonably backward compatible way but it's going to require some painful testing (I really should grab another Pi and set it up for testing with an old firmware for this rather than constantly swapping the firmware on my development Pi!)
I will get V2 handling fixed up, so stick with that. Once Raspbian has a packaged release that supports V3 then adopt it.
rpi-update may be ugly, but otherwise you're stuck in a much longer release cycle. You can't easily have both fast updates and full stability. I'm not getting into that discussion though.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

btidey
Posts: 1616
Joined: Sun Feb 17, 2013 6:51 pm

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 6:04 pm

I think I am getting seriously confused now.

I had rebuilt raspimjpeg against the latest raspberry/ userland and couldn't get v3 to put up user text. When I look at the standard host apps there then they still seem to be using v2. Not really any reference to v3 to be seen other than in the interface.

Aha I thought, so I switched to your 6by9 fork as that seems to have the latest changes in and the RaspiCamControl certainly seems to have stuff in there to support v3

Unfortunately, a build against that is still not giving me any joy for user text which I think is the same result as waveform80

In raspimjpeg the control is being called once per jpeg capture via the callback in order to support a user string with date / time substitutions etc. Maybe this is associated with it. It does work fine with v2 give or take size instability.

Just seen your latest which I think fits in with this and will be happy with a stable v2.

Thanks for the attention here.

User avatar
waveform80
Posts: 303
Joined: Mon Sep 23, 2013 1:28 pm
Location: Manchester, UK

Re: Annotate, any hope for future changes?

Fri Mar 13, 2015 11:23 pm

6by9 wrote:
waveform80 wrote:...
Still getting several other issues though - I can't seem to make any user text appear at the moment (stuff like frame-num works fine), and the background colour bits don't seem to want to play ball either (setting enable_text_background works, but then setting custom_background_colour results in a "?" appearing on the annotation and the background staying resolutely black). But I still need to check I'm not doing something random and stupid there.
Could you please grab my updated userland from https://github.com/6by9/userland/tree/PR20150310, build, and try

Code: Select all

raspistill -ae 64,0x00,0x8080ff "Wibble gibber\nThingy widget" -a 1025 -t 20000 -tl 2000 -o "output%d.jpg"
It should give black text on a white background. Likewise if you drop the -ae option, then you should get the default size text with white on black.
If that is misbehaving then I really have blundered. That would mean the structure is being misinterpreted which really shouldn't happen.
Okay, now I'm getting somewhere ... I've grabbed your repo, built your updated raspistill and with firmware #767 it's happily grabbing annotated images as you describe, with a minor correction on the command line (missing -a before the label):

Code: Select all

raspistill -ae 64,0x00,0x8080ff -a "Wibble gibber\nThingy widget" -a 1025 -t 20000 -tl 2000 -o "output%d.jpg"
Furthermore, if I modify the picamera code to only ever *set* the annotation parameters, things are happy there too. The problem comes when one attempts to *query* the annotation parameters first. With the following minor patch to RaspiCamControl.c I get the same "?" annotation I was seeing in picamera:

Code: Select all

--- a/host_applications/linux/apps/raspicam/RaspiCamControl.c
+++ b/host_applications/linux/apps/raspicam/RaspiCamControl.c
@@ -1409,6 +1409,8 @@ int raspicamcontrol_set_annotate(MMAL_COMPONENT_T *camera, const int settings, c
    MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T annotate =
       {{MMAL_PARAMETER_ANNOTATE, sizeof(MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)}};
 
+   mmal_port_parameter_get(camera->control, &annotate.hdr);
+
    if (settings)
    {
       time_t t = time(NULL);
I've played around with the structures on the python command line to try and get as much info as possible, setting a field in the annotate V3 structure then calling mmal_port_parameter_set followed by mmal_port_parameter_get to see if/how the firmware mutates the structure. Some observations:
  • The enable and show* fields seem to work correctly (preserved across the get/set).
  • The enable_text_background field also seems to work correctly.
  • The custom_background_color field seems broken (assigned 1 to it, and after the get/set it came back 0).
  • Not sure about the custom_background_{Y,U,V} fields. They mutate weirdly across set/get (e.g. assigned them all to 0 and after set/get they came back 0,112,23 - but then that could be a perfectly reasonable definition of black under YUV)
  • The text_size field seems to act oddly; sometimes it comes back verbatim, sometimes changed. For example, I just set it to 64, and after the set/get it came back as 30.
  • It seems like the text field is never preserved; I can set it but it always seems to come back blank.
It's interesting that all the fields up to enable_text_background seem to work properly, but all the fields after it seem a bit broken. Given that that's more or less the point where the V2 and V3 structures differ I wonder if this isn't something to do with the incorrect V2 structure mapping you mentioned?

Dave.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Sat Mar 14, 2015 7:39 am

Yes, get V3 was wrong too. Fixing it (if small child will let me).
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Sat Mar 14, 2015 8:53 am

Test firmware on https://github.com/6by9/RPiTest/commits ... tart_x.elf
I've done almost the same as you - set annotation, then read it back and check for diffs. Works fine now with V2 structures, and also tested and V3 set is retrieved correctly on get too.

Sorry, we almost never used to get parameters back for things the client sets as there's little point. Yes they should all work, but they often won't be tested. In theory we could write a regression test for them all, but I'm not intending to do so.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

User avatar
waveform80
Posts: 303
Joined: Mon Sep 23, 2013 1:28 pm
Location: Manchester, UK

Re: Annotate, any hope for future changes?

Sun Mar 15, 2015 1:53 pm

6by9 wrote:Test firmware on https://github.com/6by9/RPiTest/commits ... tart_x.elf
I've done almost the same as you - set annotation, then read it back and check for diffs. Works fine now with V2 structures, and also tested and V3 set is retrieved correctly on get too.
Yup, test firmware works very nicely - just need to test my v2 compat bits again on an old firmware and then I can cross that one off. Many thanks!
6by9 wrote:Sorry, we almost never used to get parameters back for things the client sets as there's little point. Yes they should all work, but they often won't be tested. In theory we could write a regression test for them all, but I'm not intending to do so.
Yeah, I kinda got the impression the getters didn't get as much attention as the setters. For the most part they work pretty well though; I think the only other place I've run into real issues with a getter was the image effect parameters (skimming the picamera getters it's the only one I can see that's conspicuously not making any MMAL calls).

When I started out writing the library, I was mindful of the possibility that people could use the library and additionally use the MMAL header translation within it to modify the state of the camera outside the "view" of picamera. Rather than have the library maintain it's own idea of the state of the camera, it seemed better to simply ask the firmware (one source of the truth and all that). Hopefully, that decision won't come back to bite me too much in the future :)

Dave.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Sun Mar 15, 2015 2:31 pm

waveform80 wrote:Yup, test firmware works very nicely - just need to test my v2 compat bits again on an old firmware and then I can cross that one off. Many thanks!
Good. I sent a patch to Dom this morning, so it should appear via rpi-update (no comments) in a day or so.
waveform80 wrote:Yeah, I kinda got the impression the getters didn't get as much attention as the setters. For the most part they work pretty well though; I think the only other place I've run into real issues with a getter was the image effect parameters (skimming the picamera getters it's the only one I can see that's conspicuously not making any MMAL calls).

When I started out writing the library, I was mindful of the possibility that people could use the library and additionally use the MMAL header translation within it to modify the state of the camera outside the "view" of picamera. Rather than have the library maintain it's own idea of the state of the camera, it seemed better to simply ask the firmware (one source of the truth and all that). Hopefully, that decision won't come back to bite me too much in the future :)
I try my best. Image effects is just that the default values aren't readable back out of the plugin, so it's not easy to get the appropriate numbers (plus it's a grotty API where things are passed in as strings!) In theory everything else that is writable should be readable too.
If you find weird things, then just be mindful that I may have blundered on getters. I'm happy to fix them when I have.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Mon Mar 16, 2015 11:49 am

My patch is included in the latest released firmware.
Hopefully should all be good now - please let me know if not.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

User avatar
waveform80
Posts: 303
Joined: Mon Sep 23, 2013 1:28 pm
Location: Manchester, UK

Re: Annotate, any hope for future changes?

Mon Mar 16, 2015 11:52 am

6by9 wrote:I try my best. Image effects is just that the default values aren't readable back out of the plugin, so it's not easy to get the appropriate numbers (plus it's a grotty API where things are passed in as strings!)
Oh dear - I don't envy you maintaining that stuff! String based APIs are rarely fun in C (or any language for that matter).
6by9 wrote:In theory everything else that is writable should be readable too.
If you find weird things, then just be mindful that I may have blundered on getters. I'm happy to fix them when I have.
No worries - I had a look through the picamera test suite while I was adding stuff for the new annotate code, and I think all the getters and setters are tested there, so even if there're any regressions at the firmware level I should still catch them before doing any releases.

That said, I have run into an issue with my backwards compatibility code ... quite a surprising one. My thought was that since the firmware relies on the size of the structure to determine which version of the annotate structure it's been given, that it would throw an error (MMAL_EINVAL or something similarly appropriate) if given a structure with a size it didn't recognize. Unfortunately, that doesn't seem to be the case.

The following is output from a console session on a Pi with firmware #740. As that's from Jan 22 I think it shouldn't know about ANNOTATE_V3, so first I try a get for the annotate settings with the V3 structure and when that works (which I was hoping it wouldn't) I try a set instead, but that works too:

Code: Select all

(picameraenv)[email protected] ~/picamera $ uname -a
Linux raspberrypi 3.18.3+ #740 PREEMPT Wed Jan 21 23:55:56 GMT 2015 armv6l GNU/Linux
(picameraenv)[email protected] ~/picamera $ python
Python 2.7.3 (default, Mar 18 2014, 05:13:23) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import picamera
>>> import picamera.mmal as mmal
>>> import ctypes as ct
>>> camera = picamera.PiCamera()
>>> mp = mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T(
...     mmal.MMAL_PARAMETER_HEADER_T(
...         mmal.MMAL_PARAMETER_ANNOTATE,
...         ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)))
>>> mmal.mmal_port_parameter_get(camera._camera[0].control, mp.hdr)
0L
>>> mmal.mmal_port_parameter_set(camera._camera[0].control, mp.hdr)
0L
At this point I thought "okay, maybe I haven't gone back far enough in the firmware history, but let's just prove the point - I'll bung an extra field on the end of the V3 structure so its size is all wrong and try get/set with that":

Code: Select all

(picameraenv)[email protected] ~/picamera $ vim picamera/mmal.py
(picameraenv)[email protected] ~/picamera $ python
Python 2.7.3 (default, Mar 18 2014, 05:13:23) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import picamera
>>> import picamera.mmal as mmal
>>> import ctypes as ct
>>> from pprint import pprint
>>> pprint(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T._fields_)
[(u'hdr', <class 'picamera.mmal.MMAL_PARAMETER_HEADER_T'>),
 (u'enable', <class 'ctypes.c_long'>),
 (u'show_shutter', <class 'ctypes.c_long'>),
 (u'show_analog_gain', <class 'ctypes.c_long'>),
 (u'show_lens', <class 'ctypes.c_long'>),
 (u'show_caf', <class 'ctypes.c_long'>),
 (u'show_motion', <class 'ctypes.c_long'>),
 (u'show_frame_num', <class 'ctypes.c_long'>),
 (u'enable_text_background', <class 'ctypes.c_long'>),
 (u'custom_background_color', <class 'ctypes.c_long'>),
 (u'custom_background_Y', <class 'ctypes.c_ubyte'>),
 (u'custom_background_U', <class 'ctypes.c_ubyte'>),
 (u'custom_background_V', <class 'ctypes.c_ubyte'>),
 (u'dummy1', <class 'ctypes.c_ubyte'>),
 (u'custom_text_color', <class 'ctypes.c_long'>),
 (u'custom_text_Y', <class 'ctypes.c_ubyte'>),
 (u'custom_text_U', <class 'ctypes.c_ubyte'>),
 (u'custom_text_V', <class 'ctypes.c_ubyte'>),
 (u'text_size', <class 'ctypes.c_ubyte'>),
 (u'text', <class 'picamera.mmal.c_char_Array_256'>),
 (u'foo', <class 'ctypes.c_long'>)]
>>> ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)
316
>>> camera = picamera.PiCamera()
>>> mp = mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T(
...     mmal.MMAL_PARAMETER_HEADER_T(
...         mmal.MMAL_PARAMETER_ANNOTATE,
...         ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)))
>>> mmal.mmal_port_parameter_get(camera._camera[0].control, mp.hdr)
0L
>>> mmal.mmal_port_parameter_set(camera._camera[0].control, mp.hdr)
0L
Huh ... okay, how about removing some bits off the text field so the structure is too short, but still longer than the V2 structure?

Code: Select all

(picameraenv)[email protected] ~/picamera $ vim picamera/mmal.py
(picameraenv)[email protected] ~/picamera $ python
Python 2.7.3 (default, Mar 18 2014, 05:13:23) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import picamera
>>> import picamera.mmal as mmal
>>> import ctypes as ct
>>> from pprint import pprint
>>> pprint(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T._fields_)
[(u'hdr', <class 'picamera.mmal.MMAL_PARAMETER_HEADER_T'>),
 (u'enable', <class 'ctypes.c_long'>),
 (u'show_shutter', <class 'ctypes.c_long'>),
 (u'show_analog_gain', <class 'ctypes.c_long'>),
 (u'show_lens', <class 'ctypes.c_long'>),
 (u'show_caf', <class 'ctypes.c_long'>),
 (u'show_motion', <class 'ctypes.c_long'>),
 (u'show_frame_num', <class 'ctypes.c_long'>),
 (u'enable_text_background', <class 'ctypes.c_long'>),
 (u'custom_background_color', <class 'ctypes.c_long'>),
 (u'custom_background_Y', <class 'ctypes.c_ubyte'>),
 (u'custom_background_U', <class 'ctypes.c_ubyte'>),
 (u'custom_background_V', <class 'ctypes.c_ubyte'>),
 (u'dummy1', <class 'ctypes.c_ubyte'>),
 (u'custom_text_color', <class 'ctypes.c_long'>),
 (u'custom_text_Y', <class 'ctypes.c_ubyte'>),
 (u'custom_text_U', <class 'ctypes.c_ubyte'>),
 (u'custom_text_V', <class 'ctypes.c_ubyte'>),
 (u'text_size', <class 'ctypes.c_ubyte'>),
 (u'text', <class 'picamera.mmal.c_char_Array_246'>)]
>>> ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)
304
>>> ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V2_T)
296
>>> camera = picamera.PiCamera()
>>> mp = mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T(
...     mmal.MMAL_PARAMETER_HEADER_T(
...         mmal.MMAL_PARAMETER_ANNOTATE,
...         ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)))
>>> mmal.mmal_port_parameter_get(camera._camera[0].control, mp.hdr)
0L
>>> mmal.mmal_port_parameter_set(camera._camera[0].control, mp.hdr)
0L
That's ... worrying (I wonder whether it'll happily write text off the end of the structure in the get call?). At this point I wondered if there were any circumstances where it'd actually throw an error so I tried removing the huge text field which should make the whole thing shorter than even the V1 structure:

Code: Select all

(picameraenv)[email protected] ~/picamera $ vim picamera/mmal.py
(picameraenv)[email protected] ~/picamera $ python
Python 2.7.3 (default, Mar 18 2014, 05:13:23) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import picamera
>>> import picamera.mmal as mmal
>>> import ctypes as ct
>>> from pprint import pprint
>>> pprint(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T._fields_)
[(u'hdr', <class 'picamera.mmal.MMAL_PARAMETER_HEADER_T'>),
 (u'enable', <class 'ctypes.c_long'>),
 (u'show_shutter', <class 'ctypes.c_long'>),
 (u'show_analog_gain', <class 'ctypes.c_long'>),
 (u'show_lens', <class 'ctypes.c_long'>),
 (u'show_caf', <class 'ctypes.c_long'>),
 (u'show_motion', <class 'ctypes.c_long'>),
 (u'show_frame_num', <class 'ctypes.c_long'>),
 (u'enable_text_background', <class 'ctypes.c_long'>),
 (u'custom_background_color', <class 'ctypes.c_long'>),
 (u'custom_background_Y', <class 'ctypes.c_ubyte'>),
 (u'custom_background_U', <class 'ctypes.c_ubyte'>),
 (u'custom_background_V', <class 'ctypes.c_ubyte'>),
 (u'dummy1', <class 'ctypes.c_ubyte'>),
 (u'custom_text_color', <class 'ctypes.c_long'>),
 (u'custom_text_Y', <class 'ctypes.c_ubyte'>),
 (u'custom_text_U', <class 'ctypes.c_ubyte'>),
 (u'custom_text_V', <class 'ctypes.c_ubyte'>),
 (u'text_size', <class 'ctypes.c_ubyte'>)]
>>> ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)
56
>>> ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_T)
64
>>> camera = picamera.PiCamera()
>>> mp = mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T(
...     mmal.MMAL_PARAMETER_HEADER_T(
...         mmal.MMAL_PARAMETER_ANNOTATE,
...         ct.sizeof(mmal.MMAL_PARAMETER_CAMERA_ANNOTATE_V3_T)))
>>> mmal.mmal_port_parameter_get(camera._camera[0].control, mp.hdr)
3L
>>> mmal.mmal_port_parameter_set(camera._camera[0].control, mp.hdr)
3L
Yay, an error (and it's EINVAL as expected)! My guess is that the check being done on the structure size goes something along the lines of "if it's >=312 bytes, it's V3, if it's >=296 bytes it's V2, if it's >=64 bytes it's V1, otherwise throw an error" and that historically the V3 bit was just missing, and before that the V2 bit. Surely it shouldn't be doing >= checks on structure sizes, though? Isn't equality is called for here?

Unfortunately, even if this is fixed in a future firmware this still scuppers my plan for providing backward compatibility as it's already-released firmwares that the code needs to detect. Is there any other way of detecting an old firmware?

The only one that springs to mind at the moment is shelling out for "uname" on camera init, but that's a *really* horrid idea which is going to make the init time even worse and push the test suite runtime off the charts (it spends most of its time initializing and closing the camera). If there's any better ways, I'd love to hear about them?


Dave.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Mon Mar 16, 2015 12:14 pm

You're correct - I've done it as
if(header->size >= sizeof(V3_T))
// Process V3
else if (header->size >= sizeof(V2_T))
//Process V2
else if (header->size >= sizeof(V1_T))
//Process V1
else
//Error.

The mechanism for reporting change events passes in a single "max size that a parameter should ever be" struct, and it expects to be told what size has been populated.
Actually, doesn't header.size get reset to the actual filled size on get? (I haven't got my laptop with me today to check source). That can be done if not, though it will leave a couple of versions in the wild that could support V3 but aren't detected.
My logic is then:
V3_T param = {{...ANNOTATE, sizeof(V3_T)+1} };
get(port, &param);
if (param.hdr.size == sizeof(V3_T))
//V3 goodness as size has been overwritten with correct V3 size.
else
{
// V2 on the assumption it will all be supported.
V2_T *v2_param = (V2_T*)param;
v2_param.hdr.size = sizeof(V2_T);
}
set(port, &param);

Otherwise you may be able to preload your struct with some data and see what gets overwritten? That depends on exactly which version of MMAL we have, as at one point only the header was passed across to VC, and the whole size was copied back (or something).

Sorry, detecting backwards compatibilty also wasn't high on my list of priorities. I'll try to remember you in future.
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

User avatar
waveform80
Posts: 303
Joined: Mon Sep 23, 2013 1:28 pm
Location: Manchester, UK

Re: Annotate, any hope for future changes?

Mon Mar 16, 2015 1:13 pm

6by9 wrote:You're correct - I've done it as
if(header->size >= sizeof(V3_T))
// Process V3
else if (header->size >= sizeof(V2_T))
//Process V2
else if (header->size >= sizeof(V1_T))
//Process V1
else
//Error.

The mechanism for reporting change events passes in a single "max size that a parameter should ever be" struct, and it expects to be told what size has been populated.
Actually, doesn't header.size get reset to the actual filled size on get? (I haven't got my laptop with me today to check source).
Ah ha! Yes, it does - hadn't noticed that. That'll do nicely :)

Dave.

6by9
Raspberry Pi Engineer & Forum Moderator
Raspberry Pi Engineer & Forum Moderator
Posts: 7142
Joined: Wed Dec 04, 2013 11:27 am
Location: ZZ9 Plural Z Alpha, aka just outside Cambridge.

Re: Annotate, any hope for future changes?

Mon Mar 16, 2015 1:33 pm

waveform80 wrote:
6by9 wrote:Actually, doesn't header.size get reset to the actual filled size on get? (I haven't got my laptop with me today to check source).
Ah ha! Yes, it does - hadn't noticed that. That'll do nicely :)
Great - I like a satisfied customer! :)
Software Engineer at Raspberry Pi Trading. Views expressed are still personal views.
I'm not interested in doing contracts for bespoke functionality - please don't ask.

Return to “Camera board”