| Message ID | 20231124061736.170131-1-haihao.xiang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavu/hwcontext_vaapi: ignore nonexistent device in default DRM device selection | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Quoting Xiang, Haihao (2023-11-24 07:17:36) > From: Haihao Xiang <haihao.xiang@intel.com> > > It is possible that renderD128 doesn't exist but renderD129 is > available in a system (see [1]). This change can make sure the default > DRM device selection works even if renderD128 doesn't exist. > > [1] https://github.com/intel/intel-device-plugins-for-kubernetes/blob/main/cmd/gpu_plugin/README.md#issues-with-media-workloads-on-multi-gpu-setups > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > --- > libavutil/hwcontext_vaapi.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > index 12bc95119a..6d542a9bc9 100644 > --- a/libavutil/hwcontext_vaapi.c > +++ b/libavutil/hwcontext_vaapi.c > @@ -1733,8 +1733,18 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device, > "/dev/dri/renderD%d", 128 + n); > priv->drm_fd = open(path, O_RDWR); > if (priv->drm_fd < 0) { > - av_log(ctx, AV_LOG_VERBOSE, "Cannot open " > - "DRM render node for device %d.\n", n); > + if (errno == ENOENT) { > + if (n != max_devices - 1) { > + av_log(ctx, AV_LOG_VERBOSE, > + "No render device %s, try next device for " > + "DRM render node.\n", path); > + continue; > + } else > + av_log(ctx, AV_LOG_VERBOSE, "No avaialbe render device " Typo in 'available'. Also, the else is unnecessary since the if() block ends with continue.
On So, 2023-11-26 at 11:55 +0100, Anton Khirnov wrote: > Quoting Xiang, Haihao (2023-11-24 07:17:36) > > From: Haihao Xiang <haihao.xiang@intel.com> > > > > It is possible that renderD128 doesn't exist but renderD129 is > > available in a system (see [1]). This change can make sure the default > > DRM device selection works even if renderD128 doesn't exist. > > > > [1] > > https://github.com/intel/intel-device-plugins-for-kubernetes/blob/main/cmd/gpu_plugin/README.md#issues-with-media-workloads-on-multi-gpu-setups > > > > Signed-off-by: Haihao Xiang <haihao.xiang@intel.com> > > --- > > libavutil/hwcontext_vaapi.c | 14 ++++++++++++-- > > 1 file changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c > > index 12bc95119a..6d542a9bc9 100644 > > --- a/libavutil/hwcontext_vaapi.c > > +++ b/libavutil/hwcontext_vaapi.c > > @@ -1733,8 +1733,18 @@ static int vaapi_device_create(AVHWDeviceContext > > *ctx, const char *device, > > "/dev/dri/renderD%d", 128 + n); > > priv->drm_fd = open(path, O_RDWR); > > if (priv->drm_fd < 0) { > > - av_log(ctx, AV_LOG_VERBOSE, "Cannot open " > > - "DRM render node for device %d.\n", n); > > + if (errno == ENOENT) { > > + if (n != max_devices - 1) { > > + av_log(ctx, AV_LOG_VERBOSE, > > + "No render device %s, try next device > > for " > > + "DRM render node.\n", path); > > + continue; > > + } else > > + av_log(ctx, AV_LOG_VERBOSE, "No avaialbe render > > device " > > Typo in 'available'. Thanks for catching this typo > > Also, the else is unnecessary since the if() block ends with continue. > I wanted to print a message if all devices don't exist. I'll remove it in the new version if you think such info is unnecessary. BRs Haihao
Quoting Xiang, Haihao (2023-11-27 05:42:20) > > > > Also, the else is unnecessary since the if() block ends with continue. > > > > I wanted to print a message if all devices don't exist. I'll remove it in the > new version if you think such info is unnecessary. I don't object to the message, I mean just that if (foo) { .... continue; } else bar; is equivalent to if (foo) { .... continue; } bar; The latter saves you one level of indentation and I find it more readable.
> Quoting Xiang, Haihao (2023-11-27 05:42:20) > > > > > > Also, the else is unnecessary since the if() block ends with continue. > > > > > > > I wanted to print a message if all devices don't exist. I'll remove it in > > the > > new version if you think such info is unnecessary. > > I don't object to the message, I mean just that > > if (foo) { > .... > continue; > } else > bar; > > is equivalent to > > if (foo) { > .... > continue; > } > > bar; > > The latter saves you one level of indentation and I find it more > readable. I got your point, many thanks for your explanations. BRs Haihao >
diff --git a/libavutil/hwcontext_vaapi.c b/libavutil/hwcontext_vaapi.c index 12bc95119a..6d542a9bc9 100644 --- a/libavutil/hwcontext_vaapi.c +++ b/libavutil/hwcontext_vaapi.c @@ -1733,8 +1733,18 @@ static int vaapi_device_create(AVHWDeviceContext *ctx, const char *device, "/dev/dri/renderD%d", 128 + n); priv->drm_fd = open(path, O_RDWR); if (priv->drm_fd < 0) { - av_log(ctx, AV_LOG_VERBOSE, "Cannot open " - "DRM render node for device %d.\n", n); + if (errno == ENOENT) { + if (n != max_devices - 1) { + av_log(ctx, AV_LOG_VERBOSE, + "No render device %s, try next device for " + "DRM render node.\n", path); + continue; + } else + av_log(ctx, AV_LOG_VERBOSE, "No avaialbe render device " + "for DRM render node.\n"); + } else + av_log(ctx, AV_LOG_VERBOSE, "Cannot open " + "DRM render node for device %d.\n", n); break; } #if CONFIG_LIBDRM