Re: [FFmpeg-devel] [PATCH v2 1/3] kmsgrab: Use invalid modifier if modifiers weren't used.

[Mark Thompson]

On 13/01/2021 00:19, Bas Nieuwenhuizen wrote:

A friendly ping on reviewing this series. Thanks!


Apologies; last time I looked at this I got distracted by Intel having broken import 
of X-tiled surfaces from kmsgrab, which was a bit of a problem to my testing.  
(, since fixed.)

Patches 1 and 2 look good, applied.

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/3] kmsgrab: Use invalid modifier if modifiers weren't used.

[Bas Nieuwenhuizen]

A friendly ping on reviewing this series. Thanks!

On Sat, Nov 14, 2020 at 12:15 AM Bas Nieuwenhuizen
 wrote:
>
> The kernel defaults to initializing the field to 0 when modifiers
> are not used and this happens to be linear. If we end up actually
> passing the modifier to a driver, tiling issues happen.
>
> So if the kernel doesn't return a modifier set it explicitly to
> INVALID. That way later processing knows there is no explicit
> modifier.
>
> v2:
>   Fix support for modifier overrides in case the getfb2 call does
>   not return a modifier.
> ---
>  libavdevice/kmsgrab.c | 27 +--
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
> index a0aa9dc22f..b740a32171 100644
> --- a/libavdevice/kmsgrab.c
> +++ b/libavdevice/kmsgrab.c
> @@ -160,6 +160,7 @@ static int kmsgrab_get_fb2(AVFormatContext *avctx,
>  KMSGrabContext *ctx = avctx->priv_data;
>  drmModeFB2 *fb;
>  int err, i, nb_objects;
> +uint64_t modifier = ctx->drm_format_modifier;
>
>  fb = drmModeGetFB2(ctx->hwctx->fd, plane->fb_id);
>  if (!fb) {
> @@ -195,6 +196,9 @@ static int kmsgrab_get_fb2(AVFormatContext *avctx,
>  goto fail;
>  }
>
> +if (fb->flags & DRM_MODE_FB_MODIFIERS)
> +modifier = fb->modifier;
> +
>  *desc = (AVDRMFrameDescriptor) {
>  .nb_layers = 1,
>  .layers[0] = {
> @@ -243,7 +247,7 @@ static int kmsgrab_get_fb2(AVFormatContext *avctx,
>  desc->objects[obj] = (AVDRMObjectDescriptor) {
>  .fd  = fd,
>  .size= size,
> -.format_modifier = fb->modifier,
> +.format_modifier = modifier,
>  };
>  desc->layers[0].planes[i] = (AVDRMPlaneDescriptor) {
>  .object_index = obj,
> @@ -557,15 +561,18 @@ static av_cold int kmsgrab_read_header(AVFormatContext 
> *avctx)
>  err = AVERROR(EINVAL);
>  goto fail;
>  }
> -if (ctx->drm_format_modifier != DRM_FORMAT_MOD_INVALID &&
> -ctx->drm_format_modifier != fb2->modifier) {
> -av_log(avctx, AV_LOG_ERROR, "Framebuffer format modifier "
> -   "%"PRIx64" does not match expected modifier.\n",
> -   fb2->modifier);
> -err = AVERROR(EINVAL);
> -goto fail;
> -} else {
> -ctx->drm_format_modifier = fb2->modifier;
> +
> +if (fb2->flags & DRM_MODE_FB_MODIFIERS) {
> +if (ctx->drm_format_modifier != DRM_FORMAT_MOD_INVALID &&
> +ctx->drm_format_modifier != fb2->modifier) {
> +av_log(avctx, AV_LOG_ERROR, "Framebuffer format modifier "
> +   "%"PRIx64" does not match expected modifier.\n",
> +   fb2->modifier);
> +err = AVERROR(EINVAL);
> +goto fail;
> +} else {
> +ctx->drm_format_modifier = fb2->modifier;
> +}
>  }
>  av_log(avctx, AV_LOG_VERBOSE, "Format is %s, from "
> "DRM format %"PRIx32" modifier %"PRIx64".\n",
> --
> 2.29.2
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2 1/3] kmsgrab: Use invalid modifier if modifiers weren't used.

[Bas Nieuwenhuizen]

The kernel defaults to initializing the field to 0 when modifiers
are not used and this happens to be linear. If we end up actually
passing the modifier to a driver, tiling issues happen.

So if the kernel doesn't return a modifier set it explicitly to
INVALID. That way later processing knows there is no explicit
modifier.

v2:
  Fix support for modifier overrides in case the getfb2 call does
  not return a modifier.
---
 libavdevice/kmsgrab.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index a0aa9dc22f..b740a32171 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -160,6 +160,7 @@ static int kmsgrab_get_fb2(AVFormatContext *avctx,
 KMSGrabContext *ctx = avctx->priv_data;
 drmModeFB2 *fb;
 int err, i, nb_objects;
+uint64_t modifier = ctx->drm_format_modifier;
 
 fb = drmModeGetFB2(ctx->hwctx->fd, plane->fb_id);
 if (!fb) {
@@ -195,6 +196,9 @@ static int kmsgrab_get_fb2(AVFormatContext *avctx,
 goto fail;
 }
 
+if (fb->flags & DRM_MODE_FB_MODIFIERS)
+modifier = fb->modifier;
+
 *desc = (AVDRMFrameDescriptor) {
 .nb_layers = 1,
 .layers[0] = {
@@ -243,7 +247,7 @@ static int kmsgrab_get_fb2(AVFormatContext *avctx,
 desc->objects[obj] = (AVDRMObjectDescriptor) {
 .fd  = fd,
 .size= size,
-.format_modifier = fb->modifier,
+.format_modifier = modifier,
 };
 desc->layers[0].planes[i] = (AVDRMPlaneDescriptor) {
 .object_index = obj,
@@ -557,15 +561,18 @@ static av_cold int kmsgrab_read_header(AVFormatContext 
*avctx)
 err = AVERROR(EINVAL);
 goto fail;
 }
-if (ctx->drm_format_modifier != DRM_FORMAT_MOD_INVALID &&
-ctx->drm_format_modifier != fb2->modifier) {
-av_log(avctx, AV_LOG_ERROR, "Framebuffer format modifier "
-   "%"PRIx64" does not match expected modifier.\n",
-   fb2->modifier);
-err = AVERROR(EINVAL);
-goto fail;
-} else {
-ctx->drm_format_modifier = fb2->modifier;
+
+if (fb2->flags & DRM_MODE_FB_MODIFIERS) {
+if (ctx->drm_format_modifier != DRM_FORMAT_MOD_INVALID &&
+ctx->drm_format_modifier != fb2->modifier) {
+av_log(avctx, AV_LOG_ERROR, "Framebuffer format modifier "
+   "%"PRIx64" does not match expected modifier.\n",
+   fb2->modifier);
+err = AVERROR(EINVAL);
+goto fail;
+} else {
+ctx->drm_format_modifier = fb2->modifier;
+}
 }
 av_log(avctx, AV_LOG_VERBOSE, "Format is %s, from "
"DRM format %"PRIx32" modifier %"PRIx64".\n",
-- 
2.29.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".