Skip to content

MMAL blocks on mmal_component_disable #435

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

Closed
spacehamster opened this issue Oct 24, 2017 · 9 comments
Closed

MMAL blocks on mmal_component_disable #435

spacehamster opened this issue Oct 24, 2017 · 9 comments

Comments

@spacehamster
Copy link

When mmal components are set up in certain configurations, presumably misconfigured, such as connecting a splitter component to the camera still port, the program never returns from vcos_semaphore_wait in mmal_component_disable(camera_component). If the program is killed and then started again, it blocks at mmal_component_create(camera_component). Is there a way of finding out when the components aren't configured properly or resetting mmal without rebooting the whole os?

@spacehamster
Copy link
Author

It wasn't connecting a splitter component to the still port that caused the problem, but rather connecting an image_encode component to the splitter port, setting the encoding to PNG, BMP or GIF (but not JPEG), setting the resolution to 1920x1080 (not 2592x1944) and not creating a preview component.

It also happens when I

  • connect a MJPEG video encoder component to the preview port
  • connect a PNG or GIF image encoder the preview port with a splitter which causes vcdbg log msg to output vcos_abort: Halting and vcodbg log assert to output
    assert( *p==0xa55a5aa5 ) failed; ../../../../../vcfw/rtos/common/rtos_common_malloc.c::rtos_pool_aligned_free line 205 rev a3d7660
  • connect a JPEG image encoder to the video port which causes vcdbg log msg to output vcos_abort: Halting and vcodbg log assert to output
    assert( ! (IS_ALIAS_NORMAL(Y) || IS_ALIAS_NORMAL(U) || IS_ALIAS_NORMAL(V)) ) failed; ../../../../../codecs/image/jpeghw/codec/jpe_enc.c::jpe_enc_rectangle line 295 rev a3d7660
  • connecting a PNG, GIF or BMP image encoder to the video port caused the whole os to lockup and become completely unresponsive

@spacehamster spacehamster changed the title MMAL blocks on mmal_component_disble MMAL blocks on mmal_component_disable Oct 29, 2017
@6by9
Copy link
Contributor

6by9 commented Oct 29, 2017

In answer to your original question, no there is no easy way to restart the MMAL service on the VPU if it wedges.
This sounds more of a fatal issue in the components than mmal_component_disable itself.

  • MJPEG to preview port should work. I'm guessing you're using MMAL_ENCODING_OPAQUE.
  • PNG/GIF to preview is a memory trample to fire that assert/abort - 0xa55a5aa5 is the guard word around an allocation. Again confirmation that you're using MMAL_ENCODING_OPAQUE (or what you are using if not) would be useful.
  • JPEG image encode to video port is a memory alias assert as the hardware block can't access cached memory. (a) it shouldn't be an abort as it isn't fatal (the JPEG codec like throwing toys out of the pram), and (b) I thought that was a standard use case with PiCamera (capture(video_port=true)), so I'm a little surprised it doesn't work.
  • PNG/GIF/BMP image_encode again sounds like a memory trample that is managed to take out stuff.

I'll have a look at these when in the office tomorrow. If you have any simple test apps that I can just run rather than having to write my own then it'd be most appreciated. (Then again it should be easy to hack raspistill to achieve these configs).

@spacehamster
Copy link
Author

All the camera_component ports are set to MMAL_ENCODING_OPAQUE with encoding_variant MMAL_ENCODING_I420

I'm still trying to wrap my head around the api, so the code is a bit messy but I've uploaded a test app here:
https://github.com/spacehamster/Picam

@6by9
Copy link
Contributor

6by9 commented Oct 30, 2017

Thanks for the sample code - that saved me a lot of time.

I've had a look through. The cases you're reporting a vcos_abort, which I believe are:
"2 1" camera video port-> splitter -> image_encode PNG
"3 4" camera preview port -> image_encode JPEG
"4 1" camera video port -> image_encode PNG
all are linked to a common image conversion that is required within image_encode. In the first case it appears to trashing memory. In the second case it's because the allocation is from the wrong cache alias and the JPEG codec throws its toys out of the pram rather than aborting cleanly (grr). The third case is similar to the first and is trashing memory.

Likewise your crash from "2 4" (PNG), "4 4" (BMP), and "5 4" (GIF) are the same root cause, just that the VPU trashes more of memory and has taken out the kernel.

"8 2" (textures off the stills port via a splitter) I'm not going to look at as textures isn't my thing, and it doesn't really have a valid use case.

"1 0" and "1 3" (MJPEG from preview port) is several things.

  • Firstly you haven't set the framerate (video_encode output port), so there's a divide by 0 when computing the number of bytes per frame.
  • Secondly is that video_encode is not set up to run from the preview port, so some signalling is missing and results in incorrect image conversion.
  • Finally is the known issue of the codec wedging if you haven't cleared the output FIFO sufficiently. It's an involved one to fix which requires a significant rewrite of the codec code.

TBH Connecting video_encode using MMAL_ENCODING_OPAQUE to anything other than camera video port, video_decode, or image_fx (mainly for deinterlace) is likely to cause issues, and I'm not sure the use case is strong enough to warrant huge investigation into fixing it up for other cases. Opaque handling is doing rather a lot under the bonnet which isn't always fully generic, and with those use cases there isn't a performance saving over using raw pixel buffers (eg MMAL_ENCODING_I420) as there is when you use the ports it is intended for.

@6by9
Copy link
Contributor

6by9 commented Oct 30, 2017

In better news, I think I've just found why we get memory corruption with the conversion. The fix should be relatively trivial.

@6by9
Copy link
Contributor

6by9 commented Oct 30, 2017

I missed out "11 5" - JPEG_NO_PREVIEW from the STILL port. Again that's not a useful use case as there will have been no auto exposure or white balance adaptation, so most likely to get randomly coloured and incorrectly exposed images.

@6by9
Copy link
Contributor

6by9 commented Oct 30, 2017

So I have a fix, and now
"2 4"
"4 4"
"5 4"
"2 1"
"4 1"
all work.

"3 4" I'd slightly mis-diagnosed and is more difficult to sort.
I've reopened #390 as that was where I was handling the mismatch of opaque buffers.

popcornmix added a commit to raspberrypi/firmware that referenced this issue Nov 3, 2017
kernel: ARM: dts: Add fake CTS signal to pi3-miniuart-bt
See: https://www.raspberrypi.org/forums/viewtopic.php?f=107&t=138223&start=100#p1228339

kernel: w1: add support for DS2438 Smart Battery Monitor
See: raspberrypi/linux#2246

firmware: image_encode and JPEG codec fixes
See: raspberrypi/userland#435
popcornmix added a commit to Hexxeh/rpi-firmware that referenced this issue Nov 3, 2017
kernel: ARM: dts: Add fake CTS signal to pi3-miniuart-bt
See: https://www.raspberrypi.org/forums/viewtopic.php?f=107&t=138223&start=100#p1228339

kernel: w1: add support for DS2438 Smart Battery Monitor
See: raspberrypi/linux#2246

firmware: image_encode and JPEG codec fixes
See: raspberrypi/userland#435
@6by9
Copy link
Contributor

6by9 commented Nov 15, 2017

Fixes got merged on 3rd Nov to the rpi-update releases.
Please run "sudo rpi-update" on a non-critical Pi (backup first!) and retest.

@JamesH65
Copy link
Collaborator

JamesH65 commented Dec 8, 2017

Please update to the latest software which now contains a fix for this issue.

Please reopen if you encounter any issues with this change.

@JamesH65 JamesH65 closed this as completed Dec 8, 2017
mkreisl added a commit to xbianonpi/xbian-package-firmware that referenced this issue Mar 17, 2018
- firmware: pwm_sdm multi-write support
  See: https://www.raspberrypi.org/forums/viewtopic.php?f=29&t=136445

- firmware: Remove cmaservice as it doesn't work reliably

- firmware: New mailbox fns for peripheral access
  See: raspberrypi/linux#2222

- firmware: image_encode and JPEG codec fixes
  See: raspberrypi/userland#435

- firmware: vc_image fixups
  See: raspberrypi/userland#433

- firmware: Support for independant display configuration

- firmware: arm_dt: Fixup camera gpios if overrides are found in dtoverlay

- firmware: mjpeg encoder: Add new thread to do the encoding

- firmware: arm_dt: Suppress non-fatal errors
  See: #906

- firmware: dtoverlay: Create "/aliases" node when needed
  See: #906

- firmware: dtoverlay app: Keep overlay symbols private

- firmware: dtoverlay app: Report unknown parameters in help

- firmware: IL ISP: Remove DPCM10_8 compressed input
- firmware: mmal_il: Add missing mappings for 8 bit Bayer encodings
- firmware: IMX219 tuning: enable motion detection
- firmware: IL camera: increase minimum resolution to 32x32

- firmware: audioplus: hdmi: Remove spamming logging message

- firmware: arm_loader: Make program_usb_boot_mode more flexible

- firmware: ov5647: Comment Y offset in mode 5 (2x2 binned 16:9)

- firmware: Video encode: Add option to set number of droppable P frames
  See: https://www.raspberrypi.org/forums/viewtopic.php?f=38&t=201882

- firmware: i2c_gpio: Preserve errors when asserts are suppressed

- firmware: power: Refactor get/set_voltage API, add SDRAM voltage

- firmware: Install interface/peer headers
  See: raspberrypi/userland#166

- firmware: gx_create_window error cleanup fix
  See: #930

bootcode: Insert delay between disconnect and reconnect for USB device booting

- firmware: IL ISP: Fix the black level control to do sensible things

- firmware: IL ISP fixes for JC

- firmware: raspividyuv: Fix saving timestamps
  See: raspberrypi/userland#453
- firmware: tidy: Platform cull

- firmware: Default to audio_pwm_mode=2
  See: https://www.raspberrypi.org/forums/viewtopic.php?f=29&t=195178

- firmware: arm_loader: Distinguish multiple turbo users
  power: Used cached values for reads
  See: Hexxeh/rpi-firmware#165

- firmware: dtoverlay: Use SUDO_USER in -pre and -post scripts

bootcode: Fix network booting on Pi1/Pi2
See: #935

- firmware: arm_loader: Fix power-related crashes around bootup
  See: Hexxeh/rpi-firmware#165

- firmware: imx219: Increase max frame rate at 640x480 to 200fps

- firmware: vc_image: Add plumbing for side-by-side YUV420 format

- firmware: arm_loader: Cache the non-limited voltage
  See: Hexxeh/rpi-firmware#165

- firmware: IL video_encode: Fix small memory leak

- firmware: arm_loader: Add get_throttled mailbox call
  See: raspberrypi/linux#2367

- firmware: video_encode: Free conv and subsample pools on disabling port

- firmware: camera: Set fixed sensor mode on all sensors

- firmware: Remove intermediate use of RGB888 in converting I420 to BGR888

- firmware: camera: Remove video tone mapping curves
- firmware: camera: AGC should not copy the tone map from VIDEO to STILLS

- firmware: arm_dt: Fix up case of NUM_CAMERAS = -1

- firmware: dtoverlay: Also allow fragment-<n> in overlays

- firmware: i2c_gpio: Optimise and run clients faster

- firmware: Rework the frequency/voltage scaling logic

- firmware: Clamp SDRAM frequencies only when sdm audio is active
  See: Hexxeh/rpi-firmware#172

- firmware: arm_dt: Improve DTB location, upstream kernel support
  See: #943

- firmware: platform: Should limit sdram voltage when turbo is throttled

- firmware: video_decode: Allow setting the output format twice

- firmware: vmcs: Increase service limit for vcsm to 2

- firmware: arm_dt: Restore support for user-selected DTB file
  See: #943

- firmware clock: WIP: Avoid temporary high clock when switching PLL and divisor
  See: https://forum.kodi.tv/showthread.php?tid=298461&pid=2713270#pid2713270

- firmware: platform: Make sure boosted frequency uses boosted voltage
- firmware: power: Add 10ms timeout for PMIC voltage control
- firmware: platform: Reduce i2c-gpio clock speed back to 200KHz

- firmware: ldconfig: Support Pi3+ and Pi0W sections

- firmware: platform: Update firmware dt-blob for pi3+

- firmware: video_decode: Allow setting the resolution on the output port

- firmware: arm_loader: Fall back to stored frequency for clocks platform doesn't have fixed values for
  See: #951
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants