jc-kynesim / rpi-ffmpeg

FFmpeg work for RPI
Other
111 stars 25 forks source link

v4l2_request_hevc: return sand128 instead of rpi4_8 and 10 #69

Closed EmperorPenguin18 closed 1 year ago

EmperorPenguin18 commented 1 year ago

This format seems to be correct and more importantly works better with mpv.

Resolves #64

jc-kynesim commented 1 year ago

Nope. You can make an argument that rpi4_8 == sand128, but rpi4_10 absolutely != sand128. If sand128 works better with mpv can you explain why (and why that isn't mpvs fault)? The sand128 & sand64_10 descriptors are mostly obsolete and were used by my Pi3 H265 decode (now rotted past redemption - the OS calls it needs no longer exist). Their use presupposes particular buf & data setup that isn't true for rpi4_8/10.

jc-kynesim commented 1 year ago

Sand formats don't fit at all well into ffmpegs idea of what a buffer looks like so its best to leave them opaque.

EmperorPenguin18 commented 1 year ago

Nope. You can make an argument that rpi4_8 == sand128, but rpi4_10 absolutely != sand128.

I tried sand64_10 with 10-bit and the video was pink. Having both be sand128 works for me, but maybe my eye for detail isn't good.

If sand128 works better with mpv can you explain why (and why that isn't mpvs fault)?

Originally I was just manually defining rpi4_8 and rpi4_10 in mpv to get it to work, this way just prevents that code duplication. My theory for why it works the way it does is as follows. pixel_format_from_format() is only used by frame_params(). In that function the pixel format is being assigned to "sw_format" which my guess is the same as "hw_subfmt" in mpv in video/out/hwdec/hwdec_drmprime.c. This is only one of two formats, and the other one (drm_prime) can't be changed. rpi4_8 and rpi4_10 are HWACCEL formats same as drm_prime, so I think they would go there, while sand128 goes in sw_format.

jc-kynesim commented 1 year ago

I'm honestly unsure how you get anything that looks even vaguely correct if you present an rpi4_10 buffer to something that is expecting sand128/rpi4_8 - rpi4_10 is a format with 10-bit quantities packed into a 32-bit word, rpi4_8 is an 8-bit format - the two are not even slightly compatible.

EmperorPenguin18 commented 1 year ago

It doesn't make sense to me either but this got it working. You can compile my branches for everything and try it yourself. https://gitlab.freedesktop.org/EmperorPenguin18/mesa/-/tree/dev/pi_drm_format https://github.com/EmperorPenguin18/rpi-ffmpeg/tree/dev/5.1.2/rpi_import_1 https://github.com/EmperorPenguin18/mpv/tree/pi_h265

EmperorPenguin18 commented 1 year ago

You can just compile mesa main branch now my change got merged

jc-kynesim commented 1 year ago

I'm glad it works for you and your mesa changes got accepted. I'm mildly astonished that you can successfully get at SAND30 that way (presumably via R16, R16G16) but I'll give the people who built the sand30 import into mesa extra credit as it clearly does for you.

jc-kynesim commented 1 year ago

Just to be clear - I currently have no intention of applying the requested rename. However if you can explain clearly why RPI4_8/10 causes mpv to fail and why that isn't mpvs fault then I'll consider it - so far I've read nothing along those lines.

jc-kynesim commented 1 year ago

In general, given a drm_prime frame you get to ignore the sw_format entirely and use the drm pixfmt + modifiers instead. That's how I'm expecting it to be used.

EmperorPenguin18 commented 1 year ago

It's working now in mpv so this change is no longer needed.