diff mbox series

[FFmpeg-devel,v06,3/5] hwcontext_drm detile non linear layout, if possible

Message ID 20200704131717.49428-4-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 
If the framebuffer is a tiled layout, use the fbtile helper routines
to try and detile it into linear layout, if supported by fbtile.

It uses the format_modifier associated with the framebuffer to decide
whether to apply detiling or not and inturn which specific detiling
to apply.

If user is using kmsgrab, they will have to use -format_modifer option
of kmsgrab to force a specific detile logic, in case they dont want to
use the original format_modifier related detiling. Or they could even
use -format_modifier 0 to make hwcontext_drm bypass this detiling.
---
 Changelog                 |  1 +
 libavutil/hwcontext_drm.c | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

Comments

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

> If the framebuffer is a tiled layout, use the fbtile helper routines
> to try and detile it into linear layout, if supported by fbtile.
>
> It uses the format_modifier associated with the framebuffer to decide
> whether to apply detiling or not and inturn which specific detiling
> to apply.
>
> If user is using kmsgrab, they will have to use -format_modifer option
> of kmsgrab to force a specific detile logic, in case they dont want to
> use the original format_modifier related detiling. Or they could even
> use -format_modifier 0 to make hwcontext_drm bypass this detiling.
> ---
>  Changelog                 |  1 +
>  libavutil/hwcontext_drm.c | 32 ++++++++++++++++++++++++++++++--
>  2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/Changelog b/Changelog
> index 3881587caa..b6a4ad1b34 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>:
> +- hwcontext_drm detiles non linear layouts, if possible
>  - kmsgrab GetFB2 format_modifier, if user doesnt specify
>  - AudioToolbox output device
>  - MacCaption demuxer
> diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
> index 32cbde82eb..bd74b3f13d 100644
> --- a/libavutil/hwcontext_drm.c
> +++ b/libavutil/hwcontext_drm.c
> @@ -21,6 +21,7 @@
>  #include <unistd.h>
>  
>  #include <drm.h>
> +#include <drm_fourcc.h>
>  #include <xf86drm.h>
>  
>  #include "avassert.h"
> @@ -28,6 +29,7 @@
>  #include "hwcontext_drm.h"
>  #include "hwcontext_internal.h"
>  #include "imgutils.h"
> +#include "fbtile.h"
>  
>  
>  static void drm_device_free(AVHWDeviceContext *hwdev)
> @@ -185,6 +187,32 @@ static int drm_transfer_get_formats(AVHWFramesContext *ctx,
>  return 0;
>  }
>  
> +// Can be overridden during compiling, if required.
> +#ifndef HWCTXDRM_SYNCRELATED_FORMATMODIFIER
> +#define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 1
> +#endif
>

This is also not acceptable, I'm afraid. We don't change behavior with compile-time checks,
and those are really not the correct way to do it.
I get why you want to be able to dump tiled video but please understand, we can't accept
this as-is.
Instead of looking for ways to make hacks part of the code base why not look into improving
the performance of detiling? There's so much more to do than copying a frame and detiling it.
You could copy and detile in-place, or you can avoid copying entirely by just directly detiling
out of place to the destination frame.
hanishkvc July 5, 2020, 8:26 p.m. UTC | #2 
Hi Lynne,

Am confused? Please look at this patch again. I directly detile from mmaped
hardware framebuffer into specified output buffer.

Only if the tile layout format is not supported, it falls back to the
original frame copy.

I am assuming you have misread the patch.

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

> Jul 4, 2020, 14:17 by hanishkvc@gmail.com:
>
> > If the framebuffer is a tiled layout, use the fbtile helper routines
> > to try and detile it into linear layout, if supported by fbtile.
> >
> > It uses the format_modifier associated with the framebuffer to decide
> > whether to apply detiling or not and inturn which specific detiling
> > to apply.
> >
> > If user is using kmsgrab, they will have to use -format_modifer option
> > of kmsgrab to force a specific detile logic, in case they dont want to
> > use the original format_modifier related detiling. Or they could even
> > use -format_modifier 0 to make hwcontext_drm bypass this detiling.
> > ---
> >  Changelog                 |  1 +
> >  libavutil/hwcontext_drm.c | 32 ++++++++++++++++++++++++++++++--
> >  2 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/Changelog b/Changelog
> > index 3881587caa..b6a4ad1b34 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>:
> > +- hwcontext_drm detiles non linear layouts, if possible
> >  - kmsgrab GetFB2 format_modifier, if user doesnt specify
> >  - AudioToolbox output device
> >  - MacCaption demuxer
> > diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
> > index 32cbde82eb..bd74b3f13d 100644
> > --- a/libavutil/hwcontext_drm.c
> > +++ b/libavutil/hwcontext_drm.c
> > @@ -21,6 +21,7 @@
> >  #include <unistd.h>
> >
> >  #include <drm.h>
> > +#include <drm_fourcc.h>
> >  #include <xf86drm.h>
> >
> >  #include "avassert.h"
> > @@ -28,6 +29,7 @@
> >  #include "hwcontext_drm.h"
> >  #include "hwcontext_internal.h"
> >  #include "imgutils.h"
> > +#include "fbtile.h"
> >
> >
> >  static void drm_device_free(AVHWDeviceContext *hwdev)
> > @@ -185,6 +187,32 @@ static int
> drm_transfer_get_formats(AVHWFramesContext *ctx,
> >  return 0;
> >  }
> >
> > +// Can be overridden during compiling, if required.
> > +#ifndef HWCTXDRM_SYNCRELATED_FORMATMODIFIER
> > +#define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 1
> > +#endif
> >
>
> This is also not acceptable, I'm afraid. We don't change behavior with
> compile-time checks,
> and those are really not the correct way to do it.
> I get why you want to be able to dump tiled video but please understand,
> we can't accept
> this as-is.
> Instead of looking for ways to make hacks part of the code base why not
> look into improving
> the performance of detiling? There's so much more to do than copying a
> frame and detiling it.
> You could copy and detile in-place, or you can avoid copying entirely by
> just directly detiling
> out of place to the destination frame.
>
> _______________________________________________
> 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 3881587caa..b6a4ad1b34 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>:
+- hwcontext_drm detiles non linear layouts, if possible
 - kmsgrab GetFB2 format_modifier, if user doesnt specify
 - AudioToolbox output device
 - MacCaption demuxer
diff --git a/libavutil/hwcontext_drm.c b/libavutil/hwcontext_drm.c
index 32cbde82eb..bd74b3f13d 100644
--- a/libavutil/hwcontext_drm.c
+++ b/libavutil/hwcontext_drm.c
@@ -21,6 +21,7 @@ 
 #include <unistd.h>
 
 #include <drm.h>
+#include <drm_fourcc.h>
 #include <xf86drm.h>
 
 #include "avassert.h"
@@ -28,6 +29,7 @@ 
 #include "hwcontext_drm.h"
 #include "hwcontext_internal.h"
 #include "imgutils.h"
+#include "fbtile.h"
 
 
 static void drm_device_free(AVHWDeviceContext *hwdev)
@@ -185,6 +187,32 @@  static int drm_transfer_get_formats(AVHWFramesContext *ctx,
     return 0;
 }
 
+// Can be overridden during compiling, if required.
+#ifndef HWCTXDRM_SYNCRELATED_FORMATMODIFIER
+#define HWCTXDRM_SYNCRELATED_FORMATMODIFIER 1
+#endif
+static int drm_transfer_with_detile(const AVFrame *hwAVFrame, AVFrame *dst, const AVFrame *src)
+{
+    int err = 0;
+
+    if (hwAVFrame->format  == AV_PIX_FMT_DRM_PRIME) {
+        AVDRMFrameDescriptor *drmFrame = (AVDRMFrameDescriptor*)hwAVFrame->data[0];
+        uint64_t formatModifier = drmFrame->objects[0].format_modifier;
+        if (formatModifier != DRM_FORMAT_MOD_LINEAR) {
+            err = detile_this(TILE_AUTO, formatModifier, dst->width, dst->height,
+                              dst->data[0], dst->linesize[0],
+                              src->data[0], src->linesize[0], 4);
+            if (!err) {
+#if HWCTXDRM_SYNCRELATED_FORMATMODIFIER
+                drmFrame->objects[0].format_modifier = DRM_FORMAT_MOD_LINEAR;
+#endif
+                return 0;
+            }
+        }
+    }
+    return av_frame_copy(dst, src);
+}
+
 static int drm_transfer_data_from(AVHWFramesContext *hwfc,
                                   AVFrame *dst, const AVFrame *src)
 {
@@ -206,7 +234,7 @@  static int drm_transfer_data_from(AVHWFramesContext *hwfc,
     map->width  = dst->width;
     map->height = dst->height;
 
-    err = av_frame_copy(dst, map);
+    err = drm_transfer_with_detile(src, dst, map);
     if (err)
         goto fail;
 
@@ -238,7 +266,7 @@  static int drm_transfer_data_to(AVHWFramesContext *hwfc,
     map->width  = src->width;
     map->height = src->height;
 
-    err = av_frame_copy(map, src);
+    err = drm_transfer_with_detile(dst, map, src);
     if (err)
         goto fail;