Discussion:
[PATCH] avoid asserts when color convertion fails
(too old to reply)
Víctor Manuel Jáquez Leal
2016-11-14 14:44:29 UTC
Permalink
This patch exposes the color convertion failure in this case

gst-play-1.0 HPCAMOLQ_BRCM_B.264 --videosink=xvimagesink

This pipeline will ask to the VPP to convert from GRAY8 to YV12,
which is the negotiated format with the XV renderer.

But this conversion fails. Without this patch, an assert will show
up:

i965_proc_picture(VADriverContextP, VAProfile, union codec_state *,
struct hw_context *): Assertion `status == 0x00000000' failed.

With this patch, the error is handled correctly, throwing a
meaningful error in GStreamer:

0:00:00.802303348 3584 0x7feff0003400 ERROR vaapipostproc
gstvaapipostproc.c:805:gst_vaapipostproc_process_vpp:<vaapipostproc0>
failed to apply VPP filters (error 2)

Though, the correct fix implies to enable VPP with this color
conversion.
---
src/gen75_picture_process.c | 4 +++-
src/i965_post_processing.c | 6 ++++--
2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/gen75_picture_process.c b/src/gen75_picture_process.c
index 0b681f1..2b82314 100644
--- a/src/gen75_picture_process.c
+++ b/src/gen75_picture_process.c
@@ -286,7 +286,9 @@ gen75_proc_picture(VADriverContextP ctx,
if(pipeline_param->num_filters == 0 || pipeline_param->filters == NULL ){
/* implicity surface format coversion and scaling */

- gen75_vpp_fmt_cvt(ctx, profile, codec_state, hw_context);
+ status = gen75_vpp_fmt_cvt(ctx, profile, codec_state, hw_context);
+ if(status != VA_STATUS_SUCCESS)
+ goto error;
}else if(pipeline_param->num_filters == 1) {
struct object_buffer * obj_buf = BUFFER((*filter_id) + 0);

diff --git a/src/i965_post_processing.c b/src/i965_post_processing.c
index 969f84b..1b4036f 100755
--- a/src/i965_post_processing.c
+++ b/src/i965_post_processing.c
@@ -6034,7 +6034,8 @@ i965_proc_picture(VADriverContextP ctx,
VA_RT_FORMAT_YUV420,
1,
&out_surface_id);
- assert(status == VA_STATUS_SUCCESS);
+ if (status != VA_STATUS_SUCCESS)
+ goto error;
tmp_surfaces[num_tmp_surfaces++] = out_surface_id;
obj_surface = SURFACE(out_surface_id);
assert(obj_surface);
@@ -6053,7 +6054,8 @@ i965_proc_picture(VADriverContextP ctx,
&src_rect,
&dst_surface,
&dst_rect);
- assert(status == VA_STATUS_SUCCESS);
+ if (status != VA_STATUS_SUCCESS)
+ goto error;

src_surface.base = (struct object_base *)obj_surface;
src_surface.type = I965_SURFACE_TYPE_SURFACE;
--
2.10.2
Víctor M. Jáquez L.
2016-11-14 14:52:47 UTC
Permalink
Post by Víctor Manuel Jáquez Leal
This patch exposes the color convertion failure in this case
gst-play-1.0 HPCAMOLQ_BRCM_B.264 --videosink=xvimagesink
I cooked this patch while I was working on

https://bugzilla.gnome.org/show_bug.cgi?id=765039

But I forgot to send it for code review :S

This is what happen when you rebase automatically you master branch.

vmjl
Sean V Kelley
2016-11-14 18:03:21 UTC
Permalink
Post by Víctor Manuel Jáquez Leal
This patch exposes the color convertion failure in this case
gst-play-1.0 HPCAMOLQ_BRCM_B.264 --videosink=xvimagesink
This pipeline will ask to the VPP to convert from GRAY8 to YV12,
which is the negotiated format with the XV renderer.
But this conversion fails. Without this patch, an assert will show
i965_proc_picture(VADriverContextP, VAProfile, union codec_state *,
struct hw_context *): Assertion `status == 0x00000000' failed.
With this patch, the error is handled correctly, throwing a
0:00:00.802303348  3584 0x7feff0003400 ERROR          vaapipostproc
gstvaapipostproc.c:805:gst_vaapipostproc_process_vpp:<vaapipostproc0>
failed to apply VPP filters (error 2)
Though, the correct fix implies to enable VPP with this color
conversion.
lgtm, applied.

Thanks,

Sean
Post by Víctor Manuel Jáquez Leal
---
 src/gen75_picture_process.c | 4 +++-
 src/i965_post_processing.c  | 6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/gen75_picture_process.c
b/src/gen75_picture_process.c
index 0b681f1..2b82314 100644
--- a/src/gen75_picture_process.c
+++ b/src/gen75_picture_process.c
@@ -286,7 +286,9 @@ gen75_proc_picture(VADriverContextP ctx,
         if(pipeline_param->num_filters == 0 || pipeline_param-
Post by Víctor Manuel Jáquez Leal
filters == NULL ){
             /* implicity surface format coversion and scaling */
 
-            gen75_vpp_fmt_cvt(ctx, profile, codec_state,
hw_context);
+            status = gen75_vpp_fmt_cvt(ctx, profile, codec_state,
hw_context);
+            if(status != VA_STATUS_SUCCESS)
+                goto error;
         }else if(pipeline_param->num_filters == 1) {
            struct object_buffer * obj_buf = BUFFER((*filter_id) +
0);
 
diff --git a/src/i965_post_processing.c b/src/i965_post_processing.c
index 969f84b..1b4036f 100755
--- a/src/i965_post_processing.c
+++ b/src/i965_post_processing.c
@@ -6034,7 +6034,8 @@ i965_proc_picture(VADriverContextP ctx,
                                      VA_RT_FORMAT_YUV420,
                                      1,
                                      &out_surface_id);
-        assert(status == VA_STATUS_SUCCESS);
+        if (status != VA_STATUS_SUCCESS)
+            goto error;
         tmp_surfaces[num_tmp_surfaces++] = out_surface_id;
         obj_surface = SURFACE(out_surface_id);
         assert(obj_surface);
@@ -6053,7 +6054,8 @@ i965_proc_picture(VADriverContextP ctx,
                                        &src_rect,
                                        &dst_surface,
                                        &dst_rect);
-        assert(status == VA_STATUS_SUCCESS);
+        if (status != VA_STATUS_SUCCESS)
+            goto error;
 
         src_surface.base = (struct object_base *)obj_surface;
         src_surface.type = I965_SURFACE_TYPE_SURFACE;
Loading...