drm_preview: Updated to allow rendering to RP1 DSI/DPI/VEC, and SPI d… #1000

6by9 · 2024-04-04T15:02:36Z

…isplays

vc4 supports overlay planes, scaling, and numerous pixel formats. RP1 and other DRM devices don't.

Handle falling back to the primary plane if there are no overlay planes.
Changing the hard coded format is a bit of a hack. The format should really be found from those supported by the display.

…isplays vc4 supports overlay planes, scaling, and numerous pixel formats. RP1 and other DRM devices don't. Handle falling back to the primary plane if there are no overlay planes. Changing the hard coded format is a bit of a hack. The format should really be found from those supported by the display. Signed-off-by: Dave Stevenson <[email protected]>

davidplowman · 2024-04-04T16:09:35Z

picamera2/previews/drm_preview.py

@@ -63,7 +63,7 @@ class DrmPreview(NullPreview):
    def __init__(self, x=0, y=0, width=640, height=480, transform=None):
        self.init_drm(x, y, width, height, transform)
        self.stop_count = 0
-        self.fb = pykms.DumbFramebuffer(self.card, width, height, "AB24")
+        self.fb = pykms.DumbFramebuffer(self.card, width, height, "XR24")


Just checking... presumably the display device understands these formats and won't cause a colour swap or anything?

This is why I put in the commit text that it should really be asking the display what it supports.

RP1 supports XRGB8888, XBGR8888, RGB888, BGR888, and RGB565.
SPI displays typically only support XRGB8888 and RGB565.
Other displays could support other format combinations.

As I read it, this buffer is only used if decoding MJPEG, so it would be nicer if this was only allocated when needed, not on init. Presumably this means it needs to match the format that the MJPEG decoder is going to produce.

Buffer allocation is validated against the formats supported, hence why I've hacked it.

Running on the Pi, I'm slightly surprised to find the both "XR24" and "XB24" work here, and there's no colour swap on the display. Do you think either of these is more likely to be supported on other devices?

davidplowman · 2024-04-04T16:11:22Z

picamera2/previews/drm_preview.py

@@ -150,23 +150,24 @@ def render_drm(self, picam2, completed_request):

            self.plane = self.resman.reserve_overlay_plane(self.crtc, format=fmt)


Are we happy that this will return None and won't raise an exception in all cases?

Not checked, but needs fixing if it does.
I'm certainly not seeing an exception raised if there is no overlay plane.

Perhaps I'll just ask our OP to give this a try, and go with it if it appears to work!

davidplowman · 2024-04-04T16:12:17Z

picamera2/previews/drm_preview.py

            drm_rotation = 1
            if self.transform.hflip:
                drm_rotation |= 16
            if self.transform.vflip:
                drm_rotation |= 32
-            self.plane.set_prop("rotation", drm_rotation)
+            #self.plane.set_prop("rotation", drm_rotation)


Does removing this break display transforms? What happens if we call it and it isn't supported?

set_prop barfs if the property doesn't exist, but I couldn't quickly find a way of checking if it existed or not.

pykms appears to populate prop_values at https://github.com/tomba/pykms/blob/master/kms/card.py#L331, but I didn't chase through whether we can iterate over that list easily.

davidplowman · 2024-04-04T16:18:11Z

Thanks for the PR! We normally target bug fixes at the "next" branch, but I can take care of that when applying it.

@ArtemKorr Are you able to try this branch and confirm that it works for you? Thanks!

davidplowman · 2024-04-04T16:36:02Z

Another thought... I wonder if set_overlay should check for overlay_plane being None (and then just returning). That way you could run code that uses overlays on other display devices, only you wouldn't get the overlay. Or perhaps it's better if it raises an error, at least you'd know what was happening. But then you might want to know that before you call it. Oh I don't know...!

davidplowman · 2024-04-04T17:00:44Z

picamera2/previews/drm_preview.py

@@ -110,7 +110,7 @@ def set_overlay(self, overlay):
        else:
            h, w, channels = overlay.shape
            # Should I be recycling these instead of making new ones all the time?
-            new_fb = pykms.DumbFramebuffer(self.card, w, h, "AB24")
+            new_fb = pykms.DumbFramebuffer(self.card, w, h, "XR24")


Might need to think about this... presumably an overlay has to have alpha to be useful?

Indeed. Again I was being a bit over-enthusiastic as I hadn't worked out the magic runes of picam2.create_preview_configuration({"format": "XRGB8888",, so was seeing bad formats kicking around and changed them.

Understood! I'll probably leave this one as it was. If it fails, applications either shouldn't call it, or stick it in a try/except.

davidplowman · 2024-04-04T17:01:40Z

Maybe leave this one with me. I'll check up on some of the questions I had, and take it from there. Thanks!

6by9 · 2024-04-04T17:12:05Z

Another thought... I wonder if set_overlay should check for overlay_plane being None (and then just returning). That way you could run code that uses overlays on other display devices, only you wouldn't get the overlay. Or perhaps it's better if it raises an error, at least you'd know what was happening. But then you might want to know that before you call it. Oh I don't know...!

Yes it does. I was nobbling the bits that stopped it running, not providing a full fix.

Maybe leave this one with me. I'll check up on some of the questions I had, and take it from there. Thanks!

Ta. It was proof of concept rather than fully polished (I don't do Python if I can help it!)

ArtemKorr · 2024-04-05T05:48:37Z

Thanks for the PR! We normally target bug fixes at the "next" branch, but I can take care of that when applying it.

@ArtemKorr Are you able to try this branch and confirm that it works for you? Thanks!

Oh yes, I'll be happy to try it one of these days

ArtemKorr · 2024-04-10T12:30:10Z

@davidplowman Finally, I got around to trying it (I tried the branch 6by9:patch-2 if I understood correctly what changes were meant). It didn't work for me if the frame does not match the screen exactly..
But when I made the image output the same size as the screen (704x432) and XR24, tried it like this,

import time

from picamera2 import Picamera2, Preview

picam2 = Picamera2()
picam2.start_preview(Preview.DRM, x=0, y=0, width=704, height=432)

preview_config = picam2.create_preview_configuration({"size": (704, 432),  "format": "BGR888"})
picam2.configure(preview_config)

picam2.start()
time.sleep(5)

davidplowman · 2024-04-10T14:28:18Z

@ArtemKorr Thanks for trying that. Yes, I think it's expected that you have to use the right format and image size, but so long as that works I think it's OK (or at least, as good as it will get). I'll post another version of this PR with just a couple of little tidies, and then merge it. Thanks again!

davidplowman · 2024-05-20T15:38:29Z

I've committed a patch which implements the changes here with a couple of tweaks as discussed. So I'll close this one now. Thanks to everyone for putting together and testing the necessary fixes!

6by9 mentioned this pull request Apr 4, 2024

[BUG] "Failed to reserve DRM plane" when running python overlay_drm.py in RPi5 Composite #995

Closed

davidplowman reviewed Apr 4, 2024

View reviewed changes

davidplowman changed the base branch from main to next April 4, 2024 16:36

davidplowman reviewed Apr 4, 2024

View reviewed changes

davidplowman closed this May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drm_preview: Updated to allow rendering to RP1 DSI/DPI/VEC, and SPI d… #1000

drm_preview: Updated to allow rendering to RP1 DSI/DPI/VEC, and SPI d… #1000

6by9 commented Apr 4, 2024

davidplowman Apr 4, 2024

6by9 Apr 4, 2024

davidplowman Apr 8, 2024

davidplowman Apr 4, 2024

6by9 Apr 4, 2024

davidplowman Apr 8, 2024

davidplowman Apr 4, 2024

6by9 Apr 4, 2024

davidplowman commented Apr 4, 2024

davidplowman commented Apr 4, 2024

davidplowman Apr 4, 2024

6by9 Apr 4, 2024

davidplowman Apr 8, 2024

davidplowman commented Apr 4, 2024

6by9 commented Apr 4, 2024

ArtemKorr commented Apr 5, 2024

ArtemKorr commented Apr 10, 2024 •

edited

Loading

davidplowman commented Apr 10, 2024

davidplowman commented May 20, 2024

		@@ -150,23 +150,24 @@ def render_drm(self, picam2, completed_request):

		self.plane = self.resman.reserve_overlay_plane(self.crtc, format=fmt)

drm_preview: Updated to allow rendering to RP1 DSI/DPI/VEC, and SPI d… #1000

drm_preview: Updated to allow rendering to RP1 DSI/DPI/VEC, and SPI d… #1000

Conversation

6by9 commented Apr 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidplowman commented Apr 4, 2024

davidplowman commented Apr 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidplowman commented Apr 4, 2024

6by9 commented Apr 4, 2024

ArtemKorr commented Apr 5, 2024

ArtemKorr commented Apr 10, 2024 • edited Loading

davidplowman commented Apr 10, 2024

davidplowman commented May 20, 2024

ArtemKorr commented Apr 10, 2024 •

edited

Loading