hello_mmal_encode fails to compile on upgraded Stretch Lite

[davecrump]

I have been using hello_mmal_encode for over a year now on Jessie Lite and Stretch Lite. However, in the last month or so it has started throwing an error at compile time on Stretch Lite. If I build a new card using the 2018-06-27-raspbian-stretch-lite.img (on a RPi 3B) and then
cd /opt/vc/src/hello_pi/ ./rebuild.sh
I get no errors. However, if I upgrade and then compile:
cd ~ sudo apt-get update sudo apt-get -y dist-upgrade cd /opt/vc/src/hello_pi/ ./rebuild.sh
I get this error:
make -C hello_mmal_encode make[1]: Entering directory '/opt/vc/src/hello_pi/hello_mmal_encode' cc -DSTANDALONE -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -DTARGET_POSIX -D_LINUX -fPIC -DPIC -D_REENTRANT -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -U_FORTIFY_SOURCE -Wall -g -DHAVE_LIBOPENMAX=2 -DOMX -DOMX_SKIP64BIT -ftree-vectorize -pipe -DUSE_EXTERNAL_OMX -DHAVE_LIBBCM_HOST -DUSE_EXTERNAL_LIBBCM_HOST -DUSE_VCHIQ_ARM -Wno-psabi -I/opt/vc/include/ -I/opt/vc/include/interface/vcos/pthreads -I/opt/vc/include/interface/vmcs_host/linux -I./ -I/opt/vc/src/hello_pi/libs/ilclient -I/opt/vc/src/hello_pi/libs/vgfont -g -c mmal_encode.c -o mmal_encode.o -Wno-deprecated-declarations cc -o hello_mmal_encode.bin -Wl,--whole-archive mmal_encode.o -lmmal -lmmal_core -lmmal_components -lmmal_util -lmmal_vc_client --no-as-needed -L/opt/vc/lib/ -lbrcmGLESv2 -lbrcmEGL -lopenmaxil -lbcm_host -lvcos -lvchiq_arm -lpthread -lrt -lm -L/opt/vc/src/hello_pi/libs/ilclient -L/opt/vc/src/hello_pi/libs/vgfont -Wl,--no-whole-archive -rdynamic cc: error: unrecognized command line option ‘--no-as-needed’; did you mean ‘--no-assert’? ../Makefile.include:19: recipe for target 'hello_mmal_encode.bin' failed make[1]: *** [hello_mmal_encode.bin] Error 1 rm mmal_encode.o make[1]: Leaving directory '/opt/vc/src/hello_pi/hello_mmal_encode' Makefile:10: recipe for target 'apps' failed make: *** [apps] Error 2
Adding -Wl, in front of --no-as-needed in the Makefile clears the compile error, but hello_mmal_encode seems to be non-functional afterwards. it looks like commit 418b77e is causing the problem.

How can i get hello_mmal_encode to compile and function on a clean upgraded Stretch Lite build?

Thanks, Dave

[6by9]

Please try

cd ~
git clone https://github.com/raspberrypi/userland.git
cd userland/host_applications/linux/apps/hello_pi
./rebuild.sh

The userland repo is the master copy of that code. I've just tried it and it builds fine.
It's possible that something has got corrupted when updating the Raspbian repositories.

There is a PR (https://github.com/raspberrypi/userland/pull/459/files) to set --no-as-needed on various libraries.
I think you may be correct that there is a typo in there, but it hasn't been merged.

[6by9]

Hmm, it looks like the content of that commit got merged internally but not into userland.
I'll fix the PR and push a commit internally to fix it up in the firmware tree.

[davecrump]

Yes, the userland Repo builds OK, but the firmware Repo does not. Thanks for starting to sort this out, I'll wait for the changes to work their way through the system. In the meantime I'll tell those users who have not already broken their systems with an upgrade not to upgrade.

[popcornmix]

@davecrump is this now fixed for you with latest update?

[6by9]

is this now fixed for you with latest update?

Note that that is update via rpi-update, and not via apt (yet).

[davecrump]

Thanks for your efforts guys, but I'm not out of the woods yet. I have built a new card using the 2018-06-27-raspbian-stretch-lite.img, then run sudo rpi-update, then run my installer, which among other things does cd /opt/vc/src/hello_pi/ ./rebuild.sh. The hello_mmal_encode compiles without error and I do not see any other errors.
However, in my application I have a piece of c++ (actually avc2ts.cpp from F5OEO https://github.com/davecrump/portsdown/blob/master/src/avc2ts/avc2ts.cpp ) that has the line ERR_OMX( OMX_SetupTunnel(camera.component(), Camera::OPORT_VIDEO, encoder.component(), Encoder::IPORT), "tunnel camera.video -> encoder.input"); and this throws the error: OMX error: avc2ts.cpp:2204 OpenMAX IL error: 0x8000100c. tunnel camera.video -> encoder.input: 0x8000100c. This error is the same as I was getting when hello_mmal_encode would not compile.
Has something else changed that I need to amend the application for?
Thanks again, Dave
Edit: If I roll-back the firmware to ec9d84e1d2ba701fd28897809269d8116b31dbf5 it all works perfectly (including the camera when called from avc2ts.cpp). This is without recompiling the hello_video applications, so the change that causes the camera not to work with the encoder seems to be elsewhere.

[6by9]

Sorry, you'll never get an IL error and a MMAL error being the same. Or are you meaning that if you ran your app on the same version firmware as had the MMAL compile failure then you got that error? They are two totally independent things.

There have been changes to video encode to support additional input formats and using a hardware block instead of software for internal conversions. Most of those went in with ec9d84e, although there was a followup in 911147a due to #182 (the software requirements on stride were looser than the hardware version).

/** Ports being connected are not compatible */
OMX_ErrorPortsNotCompatible = (OMX_S32) 0x8000100C,
Seems an odd error, but may just be a quirk of how the tunnel is configured. It's possible that the negotiation code does something odd that the new components don't like - I'll have a look on Monday. (I really hate IL!)

[davecrump]

My mistake; I hadn't thought it through. The MMAL error seems to have been cured; the separate IL error has occurred since a change that came after ec9d84e. I'll try to provide more detail later. Thanks!

[davecrump]

Before the error occurs, the camera port is port is set up with this
void setVideoFromat(const VideoFromat& videoFormat)
{
Parameter<OMX_PARAM_PORTDEFINITIONTYPE> portDef;
getPortDefinition(OPORT_VIDEO, portDef);
portDef->format.video.nFrameWidth = videoFormat.width;
portDef->format.video.nFrameHeight = videoFormat.height;
portDef->format.video.xFramerate = videoFormat.framerate << 16;
portDef->format.video.nStride = align(portDef->format.video.nFrameWidth, portDef->nBufferAlignment);
portDef->format.video.eColorFormat = OMX_COLOR_FormatYUV420PackedPlanar;
setPortDefinition(OPORT_VIDEO, portDef);

Has something changed in ec9d84e that might have broken this?

Dave

[F5OEO]

It seems that this firmware upgrade break ALL openmax based softwares that use Camera (NO backward compatible)

Most of those went in with ec9d84e, although there was a followup in 911147a due to #182 (the software requirements on stride were looser than the hardware version).

@6by9 : Can you update IL documentation or explain in details what are changes in order so fix the issue (until you can provide a backward compatible firmware)

[6by9]

I don't have any IL test apps to hand other than hello_encode which is working fine. That seriously hampers any form of debugging. MMAL all seems to be working properly.

I can't amend documentation or fix something if I can't reproduce! The one linked project is huge, so there is no way I'm going to try and build it.

Minor observations:

portDef->format.video.nStride = align(portDef->format.video.nFrameWidth, portDef->nBufferAlignment);

is wrong.

nBufferAlignment is a read-only field that specifies the alignment the port
requires for each of its buffers (e.g. a value of 4 denotes that each buffer shall be
4-byte aligned).

That's the base buffer address takes that alignment, not the stride.

The code snippet above appears not to set nSliceHeight either, although https://github.com/davecrump/portsdown/blob/master/src/avc2ts/avc2ts.cpp#L1090 does, but without any alignment (likely to fail on 1080P as 1080 is not a multiple of 16).

[F5OEO]

Thx for observations, will check that. If the project is too big, you can also try with a minimal project like : https://github.com/gagle/raspberrypi-openmax-h264
which reproduce the same issue.

[6by9]

Thank you - https://github.com/gagle/raspberrypi-openmax-h264 does show the issue, and it is in not setting nSliceHeight sensibly when combined with IL tunnelling.

Camera defaults to producing 16 line stripes. Video_encode can't cope with that as the codec requires full frames. When tunnelled it attempts to set the video encode input port format to only have 16 line stripes, and that fails.

My quick patch

diff --git a/h264.c b/h264.c
index 1c6e317..473288c 100644
--- a/h264.c
+++ b/h264.c
@@ -872,6 +872,8 @@ void set_h264_settings (component_t* encoder){
   //https://github.com/gagle/raspberrypi-omxcam/blob/master/src/video.c
 }
 
+#define ALIGN(x,a) ((x+a-1) & ~(a-1))
+
 int main (){
   OMX_ERRORTYPE error;
   OMX_BUFFERHEADERTYPE* encoder_output_buffer;
@@ -920,7 +922,8 @@ int main (){
   
   port_st.format.video.nFrameWidth = CAM_WIDTH;
   port_st.format.video.nFrameHeight = CAM_HEIGHT;
-  port_st.format.video.nStride = CAM_WIDTH;
+  port_st.format.video.nStride = ALIGN(CAM_WIDTH,16);
+  port_st.format.video.nSliceHeight = ALIGN(CAM_HEIGHT,16);
   port_st.format.video.xFramerate = VIDEO_FRAMERATE << 16;
   port_st.format.video.eCompressionFormat = OMX_VIDEO_CodingUnused;
   port_st.format.video.eColorFormat = OMX_COLOR_FormatYUV420PackedPlanar;
@@ -974,7 +977,8 @@ int main (){
   }
   port_st.format.video.nFrameWidth = CAM_WIDTH;
   port_st.format.video.nFrameHeight = CAM_HEIGHT;
-  port_st.format.video.nStride = CAM_WIDTH;
+  port_st.format.video.nStride = ALIGN(CAM_WIDTH,16);
+  port_st.format.video.nSliceHeight = ALIGN(CAM_HEIGHT,16);
   port_st.format.video.xFramerate = VIDEO_FRAMERATE << 16;
   //Despite being configured later, these two fields need to be set
   port_st.format.video.nBitrate = VIDEO_QP ? 0 : VIDEO_BITRATE;

The old firmware didn't sanity check the port definition if proprietary tunnelling had been agreed, whilst the new one does (unnecessarily). I'll try to sort out a patch to remove that requirement.

Yes the IL spec does say that nSliceHeight is read only, but that is a barking made state of affairs. The components have no context over what they are connected to, nor whether they are in a low memory or max performance use case, hence why it is implemented as read/write.

[F5OEO]

@6by9 Confirm that your patch works.
Thanks for explanation about "16 line stripes" vs Encoder.
About nSliceHeight, good to know as contradict with documentation ;)

I will modify avc2ts and confirm all is OK. @davecrump could then close issue.

[6by9]

@davecrump could then close issue.

A polite request to please keep to one problem per raised issue - this was raised as an unrelated MMAL issue and should have remained one.
The IL issue should have had a separate issue raised, otherwise life can get far too confusing with conversations crossing.

[davecrump]

A valid request. Sorry - I did not realise that there were 2 issues here until we were well into it. That was due to my unfamiliarity with the video encoding side of the RPi. Thanks both for progressing the resolution.

[davecrump]

2 last questions before I close the issue:

Reference the hello_mmal_encode compile error that is fixed by kernel 4.14.66. How long till this gets into the apt update (as opposed to rpi-update)?

Reference the change to the IL video encode. Where is the documentation referred to by @6by9 ?

Some final tests today and then I will close this issue.

Thanks, Dave

[6by9]

Reference the hello_mmal_encode compile error that is fixed by kernel 4.14.66. How long till this gets into the apt update (as opposed to rpi-update)?

That depends on when one of us nudges @XECDesign to pick up the changes and repackage them.

Reference the change to the IL video encode. Where is the documentation referred to by @6by9 ?

Which documentation reference?
The OpenMax IL Spec is the one I referenced for saying nSliceHeight is read only. See 4.3.4.1 on page 236.
The Broadcom IL component docs are all supplied as HTML in https://github.com/raspberrypi/firmware/tree/master/documentation/ilcomponents, although someone has kindly hosted them at http://www.jvcref.com/files/PI/documentation/ilcomponents/.
The MMAL Doxygen docs are also there at http://www.jvcref.com/files/PI/documentation/html/index.html, built from the headers in the userland repo.

[davecrump]

Thanks @6by9 very useful. Now that @F5OEO has updated avc2ts https://github.com/F5OEO/avc2ts/commit/b4ff9d2ce1cc80e85f0927f60e567b5a253e519a (which works well!) I can close this issue and wait for @XECDesign to repackage kernel 4.14.66 so that I can move my user community on to it (when they feel like upgrading...).