[FFmpeg-devel] [PATCH v05] fbdetile cpu based detiling of framebuffer v05

[Lynne]

Jul 3, 2020, 07:14 by hanishkvc at gmail.com:

> Hi Lynne,
>
>
> On Fri, Jul 3, 2020 at 4:17 AM Lynne <dev at lynne.ee> wrote:
>
>> Jul 2, 2020, 22:51 by hanishkvc at gmail.com:
>>
>> > v05-20200703IST0255, fbdetile, hwdownload, kmsgrab, plus
>> >
>> > Moved the core detiling logic into libavutil/fbtile.[c|h]
>> >
>>
>> Please integrate this into hwcontext_drm. No stray files, nothing shared
>> between lavu and lavfi as there shouldn't be anything in lavfi in the
>> first place.
>> Detiling must be done directly in the hwcontext_drm code.
>>
> The reason why I have put frame buffer related tile/detile logic into a
> separate helper file which can be called from anywhere, is because, both
> technically as well as semantically the tiling/detiling can be used either
> by hardware drm context (its user today) or any other hardware context, if
> required in future. Or for that matter by any other logic including say a
> video filter. Also I have tried to implement a generic algorithm for
> detiling (which can also be easily adjusted to support tiling also), such
> that it can be easily extended to detile (/tile in future) any tiling
> layout and not just Intel Tile-X, Tile-Y or Tile-Yf. So it should be usable
> in different scenarios.
>
> THe reason why I called detiling from hwdownload, currently is because,
>
> a) it's the central location, which can fetch from any hardware context,
> and for some reason if a given hardware/xyz of the hardware context doesnt
> export linear layout by default (say similar to the drm context today),
> then the detiling can be handled at a central location.
>

No other API is so completely inept to give the user tiled frames
without either the ability to detile them or copy them such that they're never
detiled, so its really not a good place. Especially since we're already kind of abusing
the hwdownload/upload filter to do GPU memcpy between Vulkan and CUDA.
If any further APIs arise that used tiled frames we can always map their frames
to DRM (if possible on platform) and download them that way, since in theory
the frames are platform-invariant. Or we can do the same we do with DRM
and just detile them when downloading (which saves a copy, I should mention).

Or better yet, we can talk to them before they've made the API stable to tell
them what a hugely bad idea that is. We've already done this with some vendors
(whose name starts with I and ends with ntel) who wanted their own GPU software
pixel formats and convinced them otherwise.



> b) sometimes we may not be able to get the correct layout detail from the
> hardware context, so one requires a way for the user to be able to tell
> what detiling logic to apply, if required. And having the detiling in
> hwdownload allowed me to pass this as an option to hwdownload and not have
> to extend any other structures or mechanisms.
>

API users can already modify the tiling of hardware frames they get from either
kmsgrab or elsewhere, and if kmsgrab doesn't provide correct tiling then its a bug.



> c) Also by the time the frame data is in hwdownload, it will be in a known
> way, so it will keep the detiling flow and logic simple at one level.
>

I still want the tiling logic in either a separate file in libavutil (with no public shared API)
or in hwcontext_drm.c, which is honestly one of the smallest hwcontexts in terms
of lines, compared to giants like OpenCL and Vulkan, so it can easily fit
a few hundred (or thousand) lines or so to detile without being too big,
so I'd really prefer for it to be in hwcontext_drm.c

The detiling can be still used as a non-lavfi filter by API users, since DMABUF
FDs can be anything that's mmappable (and files are directly mmapable).
So API users can still do hacks if they want speed or to detile any old recordings.



>> > Currently it supports detiling, later will be updated to support
>> > tiling also using the same core funda. Currently detiling is
>> > supported for Intel Tile-X|Y|Yf layouts. However the logic is
>> > easily configurable to support additional tile layouts, in future.
>> >
>> > Inturn this is used to support detiling of hwdownload frames.
>> > So one can detile when capturing from a hardware context if reqd,
>> > provided it supports one of the intel tile-x|y|yf formats. This
>> > can be used to detile when using kmsgrab for example.
>> >
>> > kmsgrab updated to use GetFB2 drm api, so that format_modifier
>> > can be autodetected and used for detiling by hwdownload, if
>> > fbdetile=1 (auto mode) is used.
>> >
>>
>> Send this as a separate patch.
>>
> I assume you want the patch which added GetFB2 and inturn fetching the
> actual format_modifier in kmsgrab as a seperate patch. I will send it as an
> independent patch file in the next release I will be making, with a
> additional modification mentioned below.
>
> I will be modifying the default format_modifier set by kmsgrab to
> MOD_INVALID from MOD_NONE (because MOD_NONE is also equivalent to
> MOD_LINEAR, so can't tell whether user set it or was the default from
> kmsgrab). And in turn overwrite the format_modifier variable with the value
> got from GetFB2, only if the current value of the variable is MOD_INVALID.
> This will allow the user to force a different format_modifier (different
> from what GetFB2 says) if required from the command line still.
>

Yes, the MOD_LINEAR == MOD_NONE reason is exactly why we don't know
if the tiling is invalid or just linear. In the case of AMD, it can be both.
That's the only reason tiled frames are even being outputted, otherwise
I imagine an error would be printed on all tiled formats.
Your patch will hopefully fix this at least on Intel.



>> > However if one doesn't want to use detiling during capture, one
>> > can use the fbdetile video filter to apply the detiling at a later
>> > stage as a seperate pass.
>> >
>>
>> NAK. In case you don't realize what all my emails mean: we're not accepting
>> this as an optional filter in any way, shape or form. hwdownload
>> _must_always_ detile.
>> It must only return an untiled image if we don't have code to detile it,
>> and it should
>> emit a warning log in that case.
>>
> I understood and agree that the layout (tiling) details (other than the
> planar nature and pixel format) is not part of the info available in the
> filter path beyond hardware context frames, today (and potentially in
> future also). However at the same time it doesnt stop a human from building
> a custom filter chain to experiment with say tiling, operations on tiled
> data if required and detiling to see the impact of the operations. And
> ffmpeg and its filter chain provides an easy mechanism to do such
> experiments, without having to reinvent a lot of code, so also one of the
> reasons why I still believe you should rethink your opposition to having a
> seperate fbdetile (and in future a fbtile) video filter. It adds a lot of
> flexibility to people, if they want, without any ugly hack or any such
> thing, rather it provides the flexibility in a very simple, easy and clean
> way.
>
> Hope you will reconsider your opposition :-)
>

The easiest way to deal with tiled frames is to just not have any in the first place.
Considering its already a niche situation we're talking about, since the user doesn't
have a working Vulkan driver or a valid OpenCL driver or a valid VAAPI driver which
doesn't mess with the colors, the speed will always be less than with a hardware download,
especially with Vulkan since we can do asynchronous transfers.
But in all cases, 4k and 8k frames are simply too large to feasibly encode at high framerates
with a software encoder, so in almost all cases but taking screenshots, users will want to use
a hardware encoder, which does require that they map to Vulkan/OpenCL/VAAPI anyway.
Some hardware encoders can even do lossless 444 (hopefully RGB) encodes too.
So its simply not a big enough use case to consider having even bigger hacks in our codebase,
especially since vendors are working towards making Vulkan/VAAPI work well with DRM frames.