diff mbox series

[FFmpeg-devel,v06,4/5] hwdownload detile framebuffer, if requested by user

Message ID 20200704131717.49428-5-hanishkvc@gmail.com
State New
Headers show
Series KMSGrab, fbtile helpers, hwcontext_drm, hwdownload, fbdetilevf | expand

Checks

Context Check Description
andriy/default pending
andriy/make fail Make failed

Commit Message

hanishkvc July 4, 2020, 1:17 p.m. UTC 
Added logic to support detiling of framebuffer.

By default this is disabled. Only if requested by the user, the
logic will be triggered.

It uses the fbtile helper routines to do the detiling. Currently
32bit RGB pixel format based framebuffers are supported.

If the underlying hardware context provides linear layouts, then
nothing is done. Only if the underlying hardware context generates
tiled layout, then user can use this to detile, where possible.

./ffmpeg -f kmsgrab -i - -vf hwdownload=1,format=bgr0 out.mp4
---
 Changelog                   |  1 +
 doc/filters.texi            | 19 ++++++++++
 libavfilter/vf_hwdownload.c | 74 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 92 insertions(+), 2 deletions(-)

Comments

Lynne July 4, 2020, 7:29 p.m. UTC | #1 
Jul 4, 2020, 14:17 by hanishkvc@gmail.com:

> Added logic to support detiling of framebuffer.
>
> By default this is disabled. Only if requested by the user, the
> logic will be triggered.
>
> It uses the fbtile helper routines to do the detiling. Currently
> 32bit RGB pixel format based framebuffers are supported.
>
> If the underlying hardware context provides linear layouts, then
> nothing is done. Only if the underlying hardware context generates
> tiled layout, then user can use this to detile, where possible.
>
> ./ffmpeg -f kmsgrab -i - -vf hwdownload=1,format=bgr0 out.mp4 
>

Not acceptable for the reasons stated in the hwcontext patch.
Besides, allowing users to detile non-tiled images is a really, _really_ bad idea.
hanishkvc July 5, 2020, 8:46 p.m. UTC | #2 
Hi Lynne,

On Sun, 5 Jul, 2020, 00:59 Lynne, <dev@lynne.ee> wrote:

> Jul 4, 2020, 14:17 by hanishkvc@gmail.com:
>
> > Added logic to support detiling of framebuffer.
> >
> > By default this is disabled. Only if requested by the user, the
> > logic will be triggered.
> >
> > It uses the fbtile helper routines to do the detiling. Currently
> > 32bit RGB pixel format based framebuffers are supported.
> >
> > If the underlying hardware context provides linear layouts, then
> > nothing is done. Only if the underlying hardware context generates
> > tiled layout, then user can use this to detile, where possible.
> >
> > ./ffmpeg -f kmsgrab -i - -vf hwdownload=1,format=bgr0 out.mp4
> >
>
> Not acceptable for the reasons stated in the hwcontext patch.
> Besides, allowing users to detile non-tiled images is a really, _really_
> bad idea.
>

As already responded, in case of the hwcontext patch, I take care of
avoiding the frame copy, so that it's efficient, where possible.

While this patch is different, as mentioned in this patch note. The idea
here is to provide a fall back option for detiling such that it's
independent of the underlying hwcontext, in case if such a situation arises
in future.

And Or Parallely it allows the user to force a detile, if they so desire
explicitly. As this is not automatic, but has to be triggered by the user,
It doesn't add any overhead by default, but gives the flexibility in case
the user so desires.

I do understand that there is a fundamental difference of opinion wrt
whether to give this additional flexibility to user or not between my
thinking and yours. I wanted to just show that it can be done in a simple
and relatively clean way without overhead, when not desired.

However It's finally your+any additional ffmpeg teams call.



> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/Changelog b/Changelog
index b6a4ad1b34..6174770ce1 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,7 @@  Entries are sorted chronologically from oldest to youngest within each release,
 releases are sorted from youngest to oldest.
 
 version <next>:
+- hwdownload framebuffer layout detiling (Intel tile-x|y|yf layouts)
 - hwcontext_drm detiles non linear layouts, if possible
 - kmsgrab GetFB2 format_modifier, if user doesnt specify
 - AudioToolbox output device
diff --git a/doc/filters.texi b/doc/filters.texi
index 67892e0afb..c783e059c2 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -12097,6 +12097,25 @@  Not all formats will be supported on the output - it may be necessary to insert
 an additional @option{format} filter immediately following in the graph to get
 the output in a supported format.
 
+It supports the following optional parameters
+
+@table @option
+@item fbdetile
+Specify type of CPU based FrameBuffer layout detiling to apply. The supported values are
+@table @var
+@item 0
+Dont do sw detiling (the default).
+@item 1
+Auto detect detile logic to apply (for hwcontext_drm).
+@item 2
+intel tile-x to linear conversion.
+@item 3
+intel tile-y to linear conversion.
+@item 4
+intel tile-yf to linear conversion.
+@end table
+@end table
+
 @section hwmap
 
 Map hardware frames to system memory or to another device.
diff --git a/libavfilter/vf_hwdownload.c b/libavfilter/vf_hwdownload.c
index 33af30cf40..5413ff104d 100644
--- a/libavfilter/vf_hwdownload.c
+++ b/libavfilter/vf_hwdownload.c
@@ -22,6 +22,10 @@ 
 #include "libavutil/mem.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
+#include "libavutil/fbtile.h"
+#if CONFIG_LIBDRM
+#include "libavutil/hwcontext_drm.h"
+#endif
 
 #include "avfilter.h"
 #include "formats.h"
@@ -33,8 +37,23 @@  typedef struct HWDownloadContext {
 
     AVBufferRef       *hwframes_ref;
     AVHWFramesContext *hwframes;
+    int fbdetile;
 } HWDownloadContext;
 
+#define OFFSET(x) offsetof(HWDownloadContext, x)
+#define FLAGS AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM
+static const AVOption hwdownload_options[] = {
+    { "fbdetile", "set framebuffer detile mode", OFFSET(fbdetile), AV_OPT_TYPE_INT, {.i64=TILE_NONE}, 0, TILE_NONE_END-1, FLAGS, "fbdetile" },
+        { "none", "No SW detiling", 0, AV_OPT_TYPE_CONST, {.i64=TILE_NONE}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+        { "auto", "auto select based on format_modifier", 0, AV_OPT_TYPE_CONST, {.i64=TILE_AUTO}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+        { "intelx", "Intel Tile-X layout", 0, AV_OPT_TYPE_CONST, {.i64=TILE_INTELX}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+        { "intely", "Intel Tile-Y layout", 0, AV_OPT_TYPE_CONST, {.i64=TILE_INTELY}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+        { "intelyf", "Intel Tile-Yf layout", 0, AV_OPT_TYPE_CONST, {.i64=TILE_INTELYF}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+        { "intelgx", "Intel Tile-X layout, GenericDetile", 0, AV_OPT_TYPE_CONST, {.i64=TILE_INTELGX}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+        { "intelgy", "Intel Tile-Y layout, GenericDetile", 0, AV_OPT_TYPE_CONST, {.i64=TILE_INTELGY}, INT_MIN, INT_MAX, FLAGS, "fbdetile" },
+    { NULL }
+};
+
 static int hwdownload_query_formats(AVFilterContext *avctx)
 {
     AVFilterFormats  *infmts = NULL;
@@ -81,6 +100,16 @@  static int hwdownload_config_input(AVFilterLink *inlink)
 
     ctx->hwframes = (AVHWFramesContext*)ctx->hwframes_ref->data;
 
+    int found = 0;
+    if (ctx->fbdetile != 0) {
+        found = fbtile_checkpixformats(ctx->hwframes->sw_format, fbtilePixFormats[0]);
+        if (!found) {
+            av_log(ctx, AV_LOG_ERROR, "Invalid input format %s for fbdetile.\n",
+                   av_get_pix_fmt_name(ctx->hwframes->sw_format));
+            return AVERROR(EINVAL);
+        }
+    }
+
     return 0;
 }
 
@@ -116,6 +145,15 @@  static int hwdownload_config_output(AVFilterLink *outlink)
         return AVERROR(EINVAL);
     }
 
+    if (ctx->fbdetile != 0) {
+        found = fbtile_checkpixformats(outlink->format, fbtilePixFormats[0]);
+        if (!found) {
+            av_log(ctx, AV_LOG_ERROR, "Invalid output format %s for fbdetile.\n",
+                   av_get_pix_fmt_name(outlink->format));
+            return AVERROR(EINVAL);
+        }
+    }
+
     outlink->w = inlink->w;
     outlink->h = inlink->h;
 
@@ -128,6 +166,7 @@  static int hwdownload_filter_frame(AVFilterLink *link, AVFrame *input)
     AVFilterLink  *outlink = avctx->outputs[0];
     HWDownloadContext *ctx = avctx->priv;
     AVFrame *output = NULL;
+    AVFrame *output2 = NULL;
     int err;
 
     if (!ctx->hwframes_ref || !input->hw_frames_ctx) {
@@ -162,13 +201,44 @@  static int hwdownload_filter_frame(AVFilterLink *link, AVFrame *input)
     if (err < 0)
         goto fail;
 
+    if (ctx->fbdetile == 0) {
+        av_frame_free(&input);
+        return ff_filter_frame(avctx->outputs[0], output);
+    }
+
+    output2 = ff_get_video_buffer(outlink, ctx->hwframes->width,
+                                  ctx->hwframes->height);
+    if (!output2) {
+        err = AVERROR(ENOMEM);
+        goto fail;
+    }
+
+    output2->width  = outlink->w;
+    output2->height = outlink->h;
+    uint64_t formatModifier = 0;
+#if CONFIG_LIBDRM
+    if (input->format  == AV_PIX_FMT_DRM_PRIME) {
+        AVDRMFrameDescriptor *drmFrame = input->data[0];
+        formatModifier = drmFrame->objects[0].format_modifier;
+    }
+#endif
+    detile_this(ctx->fbdetile, formatModifier, output2->width, output2->height,
+                output2->data[0], output2->linesize[0],
+                output->data[0], output->linesize[0], 4);
+
+    err = av_frame_copy_props(output2, input);
+    if (err < 0)
+        goto fail;
+
     av_frame_free(&input);
+    av_frame_free(&output);
 
-    return ff_filter_frame(avctx->outputs[0], output);
+    return ff_filter_frame(avctx->outputs[0], output2);
 
 fail:
     av_frame_free(&input);
     av_frame_free(&output);
+    av_frame_free(&output2);
     return err;
 }
 
@@ -182,7 +252,7 @@  static av_cold void hwdownload_uninit(AVFilterContext *avctx)
 static const AVClass hwdownload_class = {
     .class_name = "hwdownload",
     .item_name  = av_default_item_name,
-    .option     = NULL,
+    .option     = hwdownload_options,
     .version    = LIBAVUTIL_VERSION_INT,
 };