search

raspberrypi / linux

Kernel source tree for Raspberry Pi-provided kernel builds. Issues unrelated to the linux kernel should be posted on the community forum at https://forums.raspberrypi.com/
Other
11.16k stars 5.01k forks source link

Regression on V4L2 playback of Rpi3/VC4Graphics #3246

Closed psivasubramanian closed 5 years ago

psivasubramanian commented 5 years ago

Issue description: V4L2 video playback is not working on Rpi3 (32bit/64bit) now, it was last working on commit https://github.com/raspberrypi/linux/commit/d5dc848c982dff2e020f294e384447efe6ea6617. Regression is observed in recent commits like https://github.com/raspberrypi/linux/commit/4d486c17f636bb339c10cd73a07292220a973e01.

Analysis: We also narrow down the issue to be in drivers/staging/vc04_services/bcm2835-codec/bcm2835-v4l2-codec.c by reverting only this file back to commit https://github.com/raspberrypi/linux/commit/d5dc848c982dff2e020f294e384447efe6ea6617 and see the v4l2 playback working.

To reproduce 1) git clone git@github.com:YoeDistro/yoe-distro.git and cd yoe-distro 2) . raspberrypi3-envsetup.sh (or) . raspberrypi3-64-envsetup.sh 3) yoe_setup 4) yoe_add_layer git@github.com:WebPlatformForEmbedded/meta-wpe master 5) Add below in conf/local.conf.sample DISTRO_FEATURES_remove = "x11" 6) Build an image: bitbake wpe-westeros-image 7) Insert SD card & run lsblk (note sd card device, and substitute for /dev/sdX below) 8) yoe_install_image /dev/sdX wpe-westeros-image 9) Boot Rpi3 with SD card & run ifconfig after login "root" 10) ssh root@<Rpi's IP> from host 11) Kill all the WPE process listed using (ps | grep wpe -i) & launch westeros compositor $ export XDG_RUNTIME_DIR=/tmp $ export LD_PRELOAD=/usr/lib/libwesteros_gl.so.0.0.0:/usr/lib/libEGL.so.1.0.0 $ /usr/bin/westeros --renderer /usr/lib/libwesteros_render_gl.so.0 --framerate 60 --display wayland-0 & $ export WAYLAND_DISPLAY=wayland-0 12) Launch media playback using $ gst-launch playbin uri= video-sink=westerossink

Expected behaviour Playback should happen properly with AV.

Actual behaviour Only first video frame is rendering but audio is coming for few secs.

pelwell commented 5 years ago

How far back do you need to rewind to make 32-bit work again? https://github.com/raspberrypi/linux/commit/d5dc848c982dff2e020f294e384447efe6ea6617 should have no effect on 32-bit.

6by9 commented 5 years ago

I'm assuming that you are running 4.19 based on the commit hashes you're referencing.

I suspect that it is the switch to the multiplanar API that has caused the issue. https://github.com/raspberrypi/linux/commit/5e484a3e2a1118e66ec39a007ac5a9f83fe20114

What version of GStreamer are you using? Raspbian Buster is on 1.14.4 and that generally works fine using v4l2h264dec. There have been a few fixes on the GStreamer side if you're further back.

psivasubramanian commented 5 years ago

I'm assuming that you are running 4.19 based on the commit hashes you're referencing. Yes.

What version of GStreamer are you using? Raspbian Buster is on 1.14.4 and that generally works fine using v4l2h264dec. There have been a few fixes on the GStreamer side if you're further back. Gstreamer 1.16.0.

psivasubramanian commented 5 years ago

Looping @kraj @kalyan-nagabhirava @Khan3033 @balav08 as well

psivasubramanian commented 5 years ago

How far back do you need to rewind to make 32-bit work again? d5dc848 should have no effect on 32-bit.

Yes, d5dc848 is working commit only as we mentioned.

6by9 commented 5 years ago

Yes, d5dc848 is working commit only as we mentioned.

Sorry, but that makes no sense. The next commit after d5dc848 is 0ad09ae "configs: Enable iio driver for TI ADS1015" . I don't believe enabling a couple of modules causes your image to fail on video decode.

psivasubramanian commented 5 years ago

Hi @6by9, Sorry for the confusion. We didn't still locate the exact commit within the kernel which regressed, but seeing regression when recipe SRCREV switched from https://github.com/raspberrypi/linux/commit/d5dc848c982dff2e020f294e384447efe6ea6617 to recent commits.

psivasubramanian commented 5 years ago

Yes, d5dc848 is working commit only as we mentioned.

Sorry, but that makes no sense. The next commit after d5dc848 is 0ad09ae "configs: Enable iio driver for TI ADS1015" . I don't believe enabling a couple of modules causes your image to fail on video decode.

Exactly changes in drivers/staging/vc04_services/bcm2835-codec/bcm2835-v4l2-codec.c after https://github.com/raspberrypi/linux/commit/d5dc848c982dff2e020f294e384447efe6ea6617 is causing it. That we can confirm.

6by9 commented 5 years ago

So my original comment was correct then seeing as d5dc848 is prior to 5e484a3 which converted to the multi planar API. Seeing as you can reproduce the failure it would have been useful for you to have confirmed by syncing bcm2835-v4l2-codec.c to that commit, build, test. Then sync to d5dc848~1, build, and test. That way you have a confirmed commit that changed behaviour.

Very odd seeing as GStreamer1.14 is still working fine. I've built GStreamer 1.16 from freedesktop.org git repos, and it decodes fine with either 

gst-launch-1.0 -e -vvv filesrc location=bbb_sunflower_1080p_30fps_normal.mp4 ! qtdemux ! h264parse ! v4l2h264dec ! video/x-raw,format=I420 ! kmssink
gst-launch-1.0 -e -vvvv playbin uri=file:///home/pi/bbb_sunflower_1080p_30fps_normal.mp4

Looking for this westeros-sink, I'm assuming it's the one mirrored at https://github.com/rdkcmf/westeros I'd assumed that you were using mainline GStreamer v4l2h264dec component for the decode, but this seems to be a completely new decoding component which is decoding and something for rendering. Sorry, I'm not about to do significant debug to your westerossink component. It's not a part of the upstream GStreamer packages, therefore I'm considering it very niche.

Based on a five minute scan of it I'd say https://github.com/rdkcmf/westeros/blob/master/v4l2/westeros-sink/westeros-sink-soc.c#L1238 is the issue.

V4L2_PIX_FMT_NV12 is defined as having a single plane - https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-nv12.html. Setting sink->soc.fmtOut.fmt.pix_mp.num_planes= 2; is invalid, and your buffers are incorrectly sized.

If you wished to use the genuine multi-planar API, then you should be asking for V4L2_PIX_FMT_NV12M - https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-nv12m.html. Note that that is not a supported format by bcm2835-codec as it requires support for the planes being non-contiguous in memory.

Making any assumptions about the formats supported by a V4L2 device is a very dangerous position to take. Please note the details at https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-g-fmt.html#vidioc-g-fmt

Drivers should not return an error code unless the type field is invalid, this is a mechanism to fathom device capabilities and to approach parameters acceptable for both the application and driver.

The driver completely at liberty to choose a totally different format on you, but westeros-sink pays no attention to the returned format. The application should enumerate the supported formats via https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-enum-fmt.html if it wishes to be able to check the format ahead of S_FMT, and therefore avoid it being changed by the driver.

At this present stage I see no kernel bug, only an application mishandling the multi-planar API.

Khan3033 commented 5 years ago

So my original comment was correct then seeing as d5dc848 is prior to 5e484a3 which converted to the multi planar API. Seeing as you can reproduce the failure it would have been useful for you to have confirmed by syncing bcm2835-v4l2-codec.c to that commit, build, test. Then sync to d5dc848~1, build, and test. That way you have a confirmed commit that changed behaviour.

Very odd seeing as GStreamer1.14 is still working fine. I've built GStreamer 1.16 from freedesktop.org git repos, and it decodes fine with either

gst-launch-1.0 -e -vvv filesrc location=bbb_sunflower_1080p_30fps_normal.mp4 ! qtdemux ! h264parse ! v4l2h264dec ! video/x-raw,format=I420 ! kmssink
gst-launch-1.0 -e -vvvv playbin uri=file:///home/pi/bbb_sunflower_1080p_30fps_normal.mp4

Looking for this westeros-sink, I'm assuming it's the one mirrored at https://github.com/rdkcmf/westeros I'd assumed that you were using mainline GStreamer v4l2h264dec component for the decode, but this seems to be a completely new decoding component which is decoding and something for rendering. Sorry, I'm not about to do significant debug to your westerossink component. It's not a part of the upstream GStreamer packages, therefore I'm considering it very niche.

Based on a five minute scan of it I'd say https://github.com/rdkcmf/westeros/blob/master/v4l2/westeros-sink/westeros-sink-soc.c#L1238 is the issue.

V4L2_PIX_FMT_NV12 is defined as having a single plane - https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-nv12.html. Setting sink->soc.fmtOut.fmt.pix_mp.num_planes= 2; is invalid, and your buffers are incorrectly sized.

If you wished to use the genuine multi-planar API, then you should be asking for V4L2_PIX_FMT_NV12M - https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/pixfmt-nv12m.html. Note that that is not a supported format by bcm2835-codec as it requires support for the planes being non-contiguous in memory.

Making any assumptions about the formats supported by a V4L2 device is a very dangerous position to take. Please note the details at https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-g-fmt.html#vidioc-g-fmt

Drivers should not return an error code unless the type field is invalid, this is a mechanism to fathom device capabilities and to approach parameters acceptable for both the application and driver.

The driver completely at liberty to choose a totally different format on you, but westeros-sink pays no attention to the returned format. The application should enumerate the supported formats via https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/vidioc-enum-fmt.html if it wishes to be able to check the format ahead of S_FMT, and therefore avoid it being changed by the driver.

At this present stage I see no kernel bug, only an application mishandling the multi-planar API.

Hi @6by9 We have tried testing using this commit https://github.com/raspberrypi/linux/commit/5e484a3e2a1118e66ec39a007ac5a9f83fe20114 its shows the same that video is not getting rendered. But, when we use this commit https://github.com/raspberrypi/linux/commit/0ad09aef40287ad0ea9185c8e43c5d06506f8a7d the a/v playback works fine.

6by9 commented 5 years ago

The switch from the single planar to multi planar API is not a regression in the kernel, it is a deliberate and V4L2 specification compliant change.

The failure of your application to handle the multi planar API is not our issue.

psivasubramanian commented 5 years ago

Closing this issue now @jeffwannamaker has proposed a fix https://github.com/rdkcmf/westeros/commit/dac5aabb80ca6b6e8e3331b52ef3de8e46f49a3b in westeros i.e

  1. driver is returning error for VIDIOC_G_SELECTION where it shouldn't, so ignore it.
  2. driver is returning 1 for V4L2_CID_MIN_BUFFERS_FOR_CAPTURE but decoder can't work with such a small number of buffers, so ignore driver's minimum capture buffer recommendation if it's too small.

FYI @kraj

6by9 commented 5 years ago

I'm glad you've solved your issues, however:

1) There is a patch to G_SELECTION as https://github.com/raspberrypi/linux/commit/c000dd63906ad1de4899fc62f960a9439c87b68b - it had been erroneously modified to expect V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE and V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE as part of 5e484a3. G_SELECTION is not a mandatory op anyway.

2) V4L2_CID_MIN_BUFFERS_FOR_CAPTURE as 1 is sufficient for the Pi decoder as it currently decodes via intermediate buffers. The fact that you may then wish to have more buffers to put stuff on the display is your problem, not the codec's.

Your patch would also appear to me to want to be if ( (sink->soc.minBuffersOut != 0) && (sink->soc.minBuffersOut > MIN_OUTPUT_BUFFERS) ) If MIN_OUTPUT_BUFFERS is sufficient if you get an error from V4L2_CID_MIN_BUFFERS_FOR_CAPTURE, then why are values of V4L2_CID_MIN_BUFFERS_FOR_CAPTURE between MIN_OUTPUT_BUFFERS and NUM_OUTPUT_BUFFERS invalid?

And I still say that your format selection for the MPLANE case selecting V4L2_PIX_FMT_NV12 with fmt.pix_mp.num_planes= 2 looks very dubious as it should be a single plane. (V4L2_PIX_FMT_NV12M has 2 planes).