Re: [Mesa-dev] [PATCH] st/omx: fix crash when vid_enc_Constructor fails

2016-07-07 Thread Leo Liu 



On 07/07/2016 11:17 AM, Julien Isorce wrote:

It happens when trying to use omxh264enc with nouveau driver
because it does not provide any encoder at the moment.

It crashes on enc_ReleaseTasks(>free_tasks) because
at this time the list is not initialized.
So this patch make sure the lists are initialized.

Another way to fix this would be to do an early return in
enc_ReleaseTasks if head->next is null.


This way could make more sense.
all the tasks list should be initialized after everything gets settled.

Regards,
Leo


---
  src/gallium/state_trackers/omx/vid_enc.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/gallium/state_trackers/omx/vid_enc.c 
b/src/gallium/state_trackers/omx/vid_enc.c
index d70439a..7df5565 100644
--- a/src/gallium/state_trackers/omx/vid_enc.c
+++ b/src/gallium/state_trackers/omx/vid_enc.c
@@ -158,9 +158,14 @@ static OMX_ERRORTYPE vid_enc_Constructor(OMX_COMPONENTTYPE 
*comp, OMX_STRING nam
 if (!priv)
return OMX_ErrorInsufficientResources;
  
+   LIST_INITHEAD(>free_tasks);

+   LIST_INITHEAD(>used_tasks);
+   LIST_INITHEAD(>b_frames);
+   LIST_INITHEAD(>stacked_tasks);
+
 r = omx_base_filter_Constructor(comp, name);
 if (r)
-   return r;
+  return r;
  
 priv->BufferMgmtCallback = vid_enc_BufferEncoded;

 priv->messageHandler = vid_enc_MessageHandler;
@@ -256,11 +261,6 @@ static OMX_ERRORTYPE vid_enc_Constructor(OMX_COMPONENTTYPE 
*comp, OMX_STRING nam
 priv->scale.xWidth = OMX_VID_ENC_SCALING_WIDTH_DEFAULT;
 priv->scale.xHeight = OMX_VID_ENC_SCALING_WIDTH_DEFAULT;
  
-   LIST_INITHEAD(>free_tasks);

-   LIST_INITHEAD(>used_tasks);
-   LIST_INITHEAD(>b_frames);
-   LIST_INITHEAD(>stacked_tasks);
-
 return OMX_ErrorNone;
  }
  
@@ -269,6 +269,9 @@ static OMX_ERRORTYPE vid_enc_Destructor(OMX_COMPONENTTYPE *comp)

 vid_enc_PrivateType* priv = comp->pComponentPrivate;
 int i;
  
+   if (!priv)

+  return OMX_ErrorBadParameter;
+
 enc_ReleaseTasks(>free_tasks);
 enc_ReleaseTasks(>used_tasks);
 enc_ReleaseTasks(>b_frames);
@@ -875,7 +878,7 @@ static void enc_ReleaseTasks(struct list_head *head)
 struct encode_task *i, *next;
  
 if (!head)

-  return;
+  return;
  
 LIST_FOR_EACH_ENTRY_SAFE(i, next, head, list) {

pipe_resource_reference(>bitstream, NULL);


___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] st/omx: fix crash when vid_enc_Constructor fails

2016-07-07 Thread Emil Velikov 
Hi Julien,

On 7 July 2016 at 16:17, Julien Isorce  wrote:
> It happens when trying to use omxh264enc with nouveau driver
> because it does not provide any encoder at the moment.
>
> It crashes on enc_ReleaseTasks(>free_tasks) because
> at this time the list is not initialized.
> So this patch make sure the lists are initialized.
>
> Another way to fix this would be to do an early return in
> enc_ReleaseTasks if head->next is null.
IMHO your patch makes sense as-is.

Please add the stable tag before committing. and (nit) keep the
whitespace changes as separate patch.

With that, the (resulting) two patches are
Reviewed-by: Emil Velikov 

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] st/omx: fix crash when vid_enc_Constructor fails

2016-07-07 Thread Julien Isorce 
It happens when trying to use omxh264enc with nouveau driver
because it does not provide any encoder at the moment.

It crashes on enc_ReleaseTasks(>free_tasks) because
at this time the list is not initialized.
So this patch make sure the lists are initialized.

Another way to fix this would be to do an early return in
enc_ReleaseTasks if head->next is null.
---
 src/gallium/state_trackers/omx/vid_enc.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/gallium/state_trackers/omx/vid_enc.c 
b/src/gallium/state_trackers/omx/vid_enc.c
index d70439a..7df5565 100644
--- a/src/gallium/state_trackers/omx/vid_enc.c
+++ b/src/gallium/state_trackers/omx/vid_enc.c
@@ -158,9 +158,14 @@ static OMX_ERRORTYPE vid_enc_Constructor(OMX_COMPONENTTYPE 
*comp, OMX_STRING nam
if (!priv)
   return OMX_ErrorInsufficientResources;
 
+   LIST_INITHEAD(>free_tasks);
+   LIST_INITHEAD(>used_tasks);
+   LIST_INITHEAD(>b_frames);
+   LIST_INITHEAD(>stacked_tasks);
+
r = omx_base_filter_Constructor(comp, name);
if (r)
-   return r;
+  return r;
 
priv->BufferMgmtCallback = vid_enc_BufferEncoded;
priv->messageHandler = vid_enc_MessageHandler;
@@ -256,11 +261,6 @@ static OMX_ERRORTYPE vid_enc_Constructor(OMX_COMPONENTTYPE 
*comp, OMX_STRING nam
priv->scale.xWidth = OMX_VID_ENC_SCALING_WIDTH_DEFAULT;
priv->scale.xHeight = OMX_VID_ENC_SCALING_WIDTH_DEFAULT;
 
-   LIST_INITHEAD(>free_tasks);
-   LIST_INITHEAD(>used_tasks);
-   LIST_INITHEAD(>b_frames);
-   LIST_INITHEAD(>stacked_tasks);
-
return OMX_ErrorNone;
 }
 
@@ -269,6 +269,9 @@ static OMX_ERRORTYPE vid_enc_Destructor(OMX_COMPONENTTYPE 
*comp)
vid_enc_PrivateType* priv = comp->pComponentPrivate;
int i;
 
+   if (!priv)
+  return OMX_ErrorBadParameter;
+
enc_ReleaseTasks(>free_tasks);
enc_ReleaseTasks(>used_tasks);
enc_ReleaseTasks(>b_frames);
@@ -875,7 +878,7 @@ static void enc_ReleaseTasks(struct list_head *head)
struct encode_task *i, *next;
 
if (!head)
-  return;
+  return;
 
LIST_FOR_EACH_ENTRY_SAFE(i, next, head, list) {
   pipe_resource_reference(>bitstream, NULL);
-- 
1.9.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev