| Message ID | 1529581546-5892-3-git-send-email-mypopydev@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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,
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(-)