[FFmpeg-devel,v2,3/4] hwcontext_opencl: Remove the opencl_device_init in opencl_device_derive

Submitted by Jun Zhao on June 21, 2018, 11:45 a.m.

Details

Message ID 1529581546-5892-3-git-send-email-mypopydev@gmail.com
State New
Headers show

Commit Message

Jun Zhao June 21, 2018, 11:45 a.m.
In opencl device derived case, don't need to call opencl_device_init.

Signed-off-by: Jun Zhao <mypopydev@gmail.com>
---
 libavutil/hwcontext_opencl.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Carl Eugen Hoyos June 21, 2018, 11:50 a.m.
2018-06-21 13:45 GMT+02:00, Jun Zhao <mypopydev@gmail.com>:
> In opencl device derived case, don't need to call opencl_device_init.
>
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> ---
>  libavutil/hwcontext_opencl.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> index 9e96e96..295d6be 100644
> --- a/libavutil/hwcontext_opencl.c
> +++ b/libavutil/hwcontext_opencl.c
> @@ -1196,7 +1196,7 @@ static int opencl_device_derive(AVHWDeviceContext
> *hwdev,
>                                  AVHWDeviceContext *src_ctx,
>                                  int flags)
>  {
> -    int err;
> +    int err = 0;

Why is this necessary?
And if it is necessary, why is it part of this patch?

Carl Eugen
mypopy@gmail.com June 25, 2018, 1:55 a.m.
On Thu, Jun 21, 2018 at 7:50 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> 2018-06-21 13:45 GMT+02:00, Jun Zhao <mypopydev@gmail.com>:
> > In opencl device derived case, don't need to call opencl_device_init.
> >
> > Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> > ---
> >  libavutil/hwcontext_opencl.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> > index 9e96e96..295d6be 100644
> > --- a/libavutil/hwcontext_opencl.c
> > +++ b/libavutil/hwcontext_opencl.c
> > @@ -1196,7 +1196,7 @@ static int opencl_device_derive(AVHWDeviceContext
> > *hwdev,
> >                                  AVHWDeviceContext *src_ctx,
> >                                  int flags)
> >  {
> > -    int err;
> > +    int err = 0;
>
> Why is this necessary?
> And if it is necessary, why is it part of this patch?
Just give a default value , no special reason
Mark Thompson June 28, 2018, 3:50 p.m.
On 21/06/18 12:45, Jun Zhao wrote:
> In opencl device derived case, don't need to call opencl_device_init.
> 
> Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> ---
>  libavutil/hwcontext_opencl.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> index 9e96e96..295d6be 100644
> --- a/libavutil/hwcontext_opencl.c
> +++ b/libavutil/hwcontext_opencl.c
> @@ -1196,7 +1196,7 @@ static int opencl_device_derive(AVHWDeviceContext *hwdev,
>                                  AVHWDeviceContext *src_ctx,
>                                  int flags)
>  {
> -    int err;
> +    int err = 0;

This looks wrong - initialising it here may hide useful warnings.  If there is a specific case where it isn't set then set it there.

>      switch (src_ctx->type) {
>  
>  #if HAVE_OPENCL_DRM_BEIGNET
> @@ -1362,10 +1362,7 @@ static int opencl_device_derive(AVHWDeviceContext *hwdev,
>          break;
>      }
>  
> -    if (err < 0)
> -        return err;
> -
> -    return opencl_device_init(hwdev);
> +    return err;

This part looks good.  (Not sure how I missed this case, the hwdevice test does show it when run at higher debug level.)

>  }
>  
>  static int opencl_get_plane_format(enum AVPixelFormat pixfmt,
> 

Thanks,

- Mark
mypopy@gmail.com July 2, 2018, 2:15 a.m.
On Thu, Jun 28, 2018 at 11:50 PM Mark Thompson <sw@jkqxz.net> wrote:
>
> On 21/06/18 12:45, Jun Zhao wrote:
> > In opencl device derived case, don't need to call opencl_device_init.
> >
> > Signed-off-by: Jun Zhao <mypopydev@gmail.com>
> > ---
> >  libavutil/hwcontext_opencl.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
> > index 9e96e96..295d6be 100644
> > --- a/libavutil/hwcontext_opencl.c
> > +++ b/libavutil/hwcontext_opencl.c
> > @@ -1196,7 +1196,7 @@ static int opencl_device_derive(AVHWDeviceContext *hwdev,
> >                                  AVHWDeviceContext *src_ctx,
> >                                  int flags)
> >  {
> > -    int err;
> > +    int err = 0;
>
> This looks wrong - initialising it here may hide useful warnings.  If there is a specific case where it isn't set then set it there.
Fixed this part and applied, thanks.
>
> >      switch (src_ctx->type) {
> >
> >  #if HAVE_OPENCL_DRM_BEIGNET
> > @@ -1362,10 +1362,7 @@ static int opencl_device_derive(AVHWDeviceContext *hwdev,
> >          break;
> >      }
> >
> > -    if (err < 0)
> > -        return err;
> > -
> > -    return opencl_device_init(hwdev);
> > +    return err;
>
> This part looks good.  (Not sure how I missed this case, the hwdevice test does show it when run at higher debug level.)
>
> >  }
> >
> >  static int opencl_get_plane_format(enum AVPixelFormat pixfmt,
> >
>
> Thanks,
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Patch hide | download patch | download mbox

diff --git a/libavutil/hwcontext_opencl.c b/libavutil/hwcontext_opencl.c
index 9e96e96..295d6be 100644
--- a/libavutil/hwcontext_opencl.c
+++ b/libavutil/hwcontext_opencl.c
@@ -1196,7 +1196,7 @@  static int opencl_device_derive(AVHWDeviceContext *hwdev,
                                 AVHWDeviceContext *src_ctx,
                                 int flags)
 {
-    int err;
+    int err = 0;
     switch (src_ctx->type) {
 
 #if HAVE_OPENCL_DRM_BEIGNET
@@ -1362,10 +1362,7 @@  static int opencl_device_derive(AVHWDeviceContext *hwdev,
         break;
     }
 
-    if (err < 0)
-        return err;
-
-    return opencl_device_init(hwdev);
+    return err;
 }
 
 static int opencl_get_plane_format(enum AVPixelFormat pixfmt,