VAVDA: Redesign stage 1.

content/common/gpu/media/vaapi_delegate.cc

Index: content/common/gpu/media/vaapi_delegate.cc
diff --git a/content/common/gpu/media/vaapi_delegate.cc b/content/common/gpu/media/vaapi_delegate.cc
new file mode 100644
index 0000000000000000000000000000000000000000..3233a5dd0cc7906a623ae1b4c6881f089ed6759d
--- /dev/null
+++ b/content/common/gpu/media/vaapi_delegate.cc
@@ -0,0 +1,466 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <dlfcn.h>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "content/common/gpu/media/vaapi_delegate.h"
+
+#define LOG_VA_ERROR_AND_REPORT(va_res, err_msg)           \

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

s/va_res/va_error/ here and in the next 4 lines

[Pawel Osciak, 2013/05/21 22:32:35]

Well, the result might be a success too, so I'd prefer keeping res, unless you
have strong feelings about this.

[Ami GONE FROM CHROMIUM, 2013/05/22 23:59:47]

It cannot be success in this macro, that's my point.

[Pawel Osciak, 2013/05/24 01:46:39]

Done.

+  do {                                                     \
+    DVLOG(1) << err_msg                                    \
+             << " VA error: " << VAAPI_ErrorStr(va_res);   \
+    report_error_cb_.Run();                                \
+  } while (0)
+
+#define VA_LOG_ON_ERROR(va_res, err_msg)                   \
+  do {                                                     \
+    if ((va_res) != VA_STATUS_SUCCESS)                     \
+      LOG_VA_ERROR_AND_REPORT(va_res, err_msg);            \
+  } while (0)
+
+#define VA_SUCCESS_OR_RETURN(va_res, err_msg, ret)         \
+  do {                                                     \
+    if ((va_res) != VA_STATUS_SUCCESS) {                   \
+      LOG_VA_ERROR_AND_REPORT(va_res, err_msg);            \
+      return (ret);                                        \
+    }                                                      \
+  } while (0)
+
+namespace content {
+
+void *vaapi_handle = NULL;
+void *vaapi_x11_handle = NULL;

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

static (this & above)

[Pawel Osciak, 2013/05/21 22:32:35]

Good point. Done.

+
+typedef VADisplay (*VaapiGetDisplay)(Display *dpy);
+typedef int (*VaapiDisplayIsValid)(VADisplay dpy);
+typedef VAStatus (*VaapiInitialize)(VADisplay dpy,
+                                    int *major_version,
+                                    int *minor_version);
+typedef VAStatus (*VaapiTerminate)(VADisplay dpy);
+typedef VAStatus (*VaapiGetConfigAttributes)(VADisplay dpy,
+                                             VAProfile profile,
+                                             VAEntrypoint entrypoint,
+                                             VAConfigAttrib *attrib_list,
+                                             int num_attribs);
+typedef VAStatus (*VaapiCreateConfig)(VADisplay dpy,
+                                      VAProfile profile,
+                                      VAEntrypoint entrypoint,
+                                      VAConfigAttrib *attrib_list,
+                                      int num_attribs,
+                                      VAConfigID *config_id);
+typedef VAStatus (*VaapiDestroyConfig)(VADisplay dpy, VAConfigID config_id);
+typedef VAStatus (*VaapiCreateSurfaces)(VADisplay dpy,
+                                        int width,
+                                        int height,
+                                        int format,
+                                        int num_surfaces,
+                                        VASurfaceID *surfaces);
+typedef VAStatus (*VaapiDestroySurfaces)(VADisplay dpy,
+                                         VASurfaceID *surfaces,
+                                         int num_surfaces);
+typedef VAStatus (*VaapiCreateContext)(VADisplay dpy,
+                                       VAConfigID config_id,
+                                       int picture_width,
+                                       int picture_height,
+                                       int flag,
+                                       VASurfaceID *render_targets,
+                                       int num_render_targets,
+                                       VAContextID *context);
+typedef VAStatus (*VaapiDestroyContext)(VADisplay dpy, VAContextID context);
+typedef VAStatus (*VaapiPutSurface)(VADisplay dpy,
+                                    VASurfaceID surface,
+                                    Drawable draw,
+                                    short srcx,
+                                    short srcy,
+                                    unsigned short srcw,
+                                    unsigned short srch,
+                                    short destx,
+                                    short desty,
+                                    unsigned short destw,
+                                    unsigned short desth,
+                                    VARectangle *cliprects,
+                                    unsigned int number_cliprects,
+                                    unsigned int flags);
+typedef VAStatus (*VaapiSyncSurface)(VADisplay dpy, VASurfaceID render_target);
+typedef VAStatus (*VaapiBeginPicture)(VADisplay dpy,
+                                      VAContextID context,
+                                      VASurfaceID render_target);
+typedef VAStatus (*VaapiRenderPicture)(VADisplay dpy,
+                                       VAContextID context,
+                                       VABufferID *buffers,
+                                       int num_buffers);
+typedef VAStatus (*VaapiEndPicture)(VADisplay dpy, VAContextID context);
+typedef VAStatus (*VaapiCreateBuffer)(VADisplay dpy,
+                                      VAContextID context,
+                                      VABufferType type,
+                                      unsigned int size,
+                                      unsigned int num_elements,
+                                      void *data,
+                                      VABufferID *buf_id);
+typedef VAStatus (*VaapiDestroyBuffer)(VADisplay dpy, VABufferID buffer_id);
+typedef const char* (*VaapiErrorStr)(VAStatus error_status);
+
+#define VAAPI_SYM(name, handle) Vaapi##name VAAPI_##name = NULL

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

#undef after done w/ these?

[Pawel Osciak, 2013/05/21 22:32:35]

Done.

+
+VAAPI_SYM(GetDisplay, vaapi_x11_handle);
+VAAPI_SYM(DisplayIsValid, vaapi_handle);
+VAAPI_SYM(Initialize, vaapi_handle);
+VAAPI_SYM(Terminate, vaapi_handle);
+VAAPI_SYM(GetConfigAttributes, vaapi_handle);
+VAAPI_SYM(CreateConfig, vaapi_handle);
+VAAPI_SYM(DestroyConfig, vaapi_handle);
+VAAPI_SYM(CreateSurfaces, vaapi_handle);
+VAAPI_SYM(DestroySurfaces, vaapi_handle);
+VAAPI_SYM(CreateContext, vaapi_handle);
+VAAPI_SYM(DestroyContext, vaapi_handle);
+VAAPI_SYM(PutSurface, vaapi_x11_handle);
+VAAPI_SYM(SyncSurface, vaapi_x11_handle);
+VAAPI_SYM(BeginPicture, vaapi_handle);
+VAAPI_SYM(RenderPicture, vaapi_handle);
+VAAPI_SYM(EndPicture, vaapi_handle);
+VAAPI_SYM(CreateBuffer, vaapi_handle);
+VAAPI_SYM(DestroyBuffer, vaapi_handle);
+VAAPI_SYM(ErrorStr, vaapi_handle);

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

Any reason not to do these (and the typedefs above and the dlsym's at the bottom
of the file) in sorted order?

[Pawel Osciak, 2013/05/21 22:32:35]

Well, they are thematically and chronologically sorted. CreateSurfaces() comes
together with DestroySurfaces() and they are called before CreateContext() in
the actual flow as well.

I can apply my dictionary sorting skills if you prefer it that way though.

+
+// Maps Profile enum values to VaProfile values.
+static bool ProfileToVAProfile(media::VideoCodecProfile profile,
+                               VAProfile& va_profile) {

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

non-const ref

[Pawel Osciak, 2013/05/21 22:32:35]

Done.

+  switch (profile) {
+    case media::H264PROFILE_BASELINE:
+      va_profile = VAProfileH264Baseline;
+      break;
+    case media::H264PROFILE_MAIN:
+      va_profile = VAProfileH264Main;
+      break;
+    case media::H264PROFILE_HIGH:

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

What about the variants of HIGH in media?  Do they not get handled by VA's
"high"? (also, does using VA "high" for media's EXTENDED not work?)

[Pawel Osciak, 2013/05/21 22:32:35]

The other ones are "HIGH10", 422 and 444, I might've had a reason for this, but
I don't remember now. I'll add a TODO to look at it again.

+      va_profile = VAProfileH264High;
+      break;
+    default:
+      return false;
+  }
+  return true;
+}
+
+VASurface::VASurface(VASurfaceID va_surface_id, const ReleaseCB& release_cb)
+    : va_surface_id_(va_surface_id),
+      release_cb_(release_cb) {
+  DCHECK(!release_cb_.is_null());
+}
+
+VASurface::~VASurface() {
+  release_cb_.Run(va_surface_id_);
+}
+
+VaapiDelegate::VaapiDelegate()
+    : va_display_(NULL),
+      va_config_id_(VA_INVALID_ID),
+      va_context_id_(VA_INVALID_ID) {
+}
+
+VaapiDelegate::~VaapiDelegate() {
+  DestroyPendingBuffers();
+  DestroySurfaces();
+  Deinitialize();
+}
+
+scoped_refptr<VaapiDelegate> VaapiDelegate::Create(
+    media::VideoCodecProfile profile,
+    Display* x_display,
+    const base::Closure& report_error_cb) {
+  scoped_refptr<VaapiDelegate> vaapi_delegate(new VaapiDelegate);

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

nit: never omit the parens on a new statement.

[Pawel Osciak, 2013/05/21 22:32:35]

Done.

+
+  if (!vaapi_delegate->Initialize(profile, x_display, report_error_cb))
+    vaapi_delegate = NULL;
+
+  return vaapi_delegate;
+}
+
+bool VaapiDelegate::Initialize(media::VideoCodecProfile profile,
+                               Display* x_display,
+                               const base::Closure& report_error_cb) {
+  static bool vaapi_functions_initialized = PostSandboxInitialization();

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

This idiom is not thread-safe.  
(https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/p6h3HC8Wro4...)

[Pawel Osciak, 2013/05/21 22:32:35]

Yes, but only one thread ever runs this...

[Ami GONE FROM CHROMIUM, 2013/05/22 23:59:47]

How's that?

[Pawel Osciak, 2013/05/24 01:46:39]

I meant PreSandboxInitialization is run once, which would otherwise cause some
problems, but Post yeah, can be run more than once, although it's not really a
leak of any sort and shouldn't have other ill effects, since dlsyms are just
returning addresses and in practice they will work the same way if executed more
than once. And well, as long as assignments are atomic on the platform.
Should be fixed nevertheless, submitted a bug, same thing happens in EVDA:
crbug.com/243597

+  if (!vaapi_functions_initialized) {
+    DVLOG(1) << "Failed to initialize VAAPI libs";
+    return false;
+  }
+
+  report_error_cb_ = report_error_cb;
+
+  base::AutoLock auto_lock(va_lock_);
+
+  VAProfile va_profile;
+  if (!ProfileToVAProfile(profile, va_profile)) {
+    DVLOG(1) << "Unsupported profile";
+    return false;
+  }
+
+  va_display_ = VAAPI_GetDisplay(x_display);
+  if (!VAAPI_DisplayIsValid(va_display_)) {
+    DVLOG(1) << "Could not get a valid VA display";
+    return false;
+  }
+
+  int major_version, minor_version;
+  VAStatus va_res;
+  va_res = VAAPI_Initialize(va_display_, &major_version, &minor_version);
+  VA_SUCCESS_OR_RETURN(va_res, "vaInitialize failed", false);
+  DVLOG(1) << "VAAPI version: " << major_version << "." << minor_version;
+
+  VAConfigAttrib attrib;

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

s/;/ = {0};/ ?

[Pawel Osciak, 2013/05/21 22:32:35]

Done (slightly differently).

+  attrib.type = VAConfigAttribRTFormat;
+
+  VAEntrypoint entrypoint = VAEntrypointVLD;

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

necessary?
(at the very least, should be const & name begin with k)

[Pawel Osciak, 2013/05/21 22:32:35]

Done.

+  va_res = VAAPI_GetConfigAttributes(va_display_, va_profile, entrypoint,
+                                     &attrib, 1);
+  VA_SUCCESS_OR_RETURN(va_res, "vaGetConfigAttributes failed", false);
+
+  if (!(attrib.value & VA_RT_FORMAT_YUV420)) {
+    DVLOG(1) << "YUV420 not supported";
+    return false;
+  }
+
+  va_res = VAAPI_CreateConfig(va_display_, va_profile, entrypoint,
+                              &attrib, 1, &va_config_id_);
+  VA_SUCCESS_OR_RETURN(va_res, "vaCreateConfig failed", false);
+
+  return true;
+}
+
+void VaapiDelegate::Deinitialize() {
+  base::AutoLock auto_lock(va_lock_);
+
+  if (va_config_id_ != VA_INVALID_ID) {
+    VAStatus va_res = VAAPI_DestroyConfig(va_display_, va_config_id_);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyConfig failed");
+  }
+
+  if (va_display_) {
+    VAStatus va_res = VAAPI_Terminate(va_display_);
+    VA_LOG_ON_ERROR(va_res, "vaTerminate failed");
+  }
+
+  va_config_id_ = VA_INVALID_ID;
+  va_display_ = NULL;
+}
+
+bool VaapiDelegate::CreateSurfaces(int width, int height,
+                                   size_t num_surfaces,
+                                   std::vector<VASurfaceID>& va_surfaces) {
+  base::AutoLock auto_lock(va_lock_);
+
+  DCHECK(va_surfaces.empty());
+  DCHECK(!va_surface_ids_.get());
+  va_surface_ids_.reset(new VASurfaceID[num_surfaces]);
+
+  // Allocate surfaces in driver.
+  VAStatus va_res = VAAPI_CreateSurfaces(va_display_, width, height,
+                                         VA_RT_FORMAT_YUV420,
+                                         num_surfaces, va_surface_ids_.get());
+  VA_SUCCESS_OR_RETURN(va_res, "vaCreateSurfaces failed", false);
+  num_va_surfaces_ = num_surfaces;
+
+  // And create a context associated with them.
+  va_res = VAAPI_CreateContext(va_display_, va_config_id_,
+                               width, height, VA_PROGRESSIVE,
+                               va_surface_ids_.get(),
+                               num_surfaces, &va_context_id_);
+
+  if (va_res != VA_STATUS_SUCCESS) {
+    DVLOG(1) << "Error creating context surfaces";
+    DestroySurfaces();

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

I think this isn't really necessary, in which case inline DestroySurfaces into
the dtor.

[Pawel Osciak, 2013/05/21 22:32:35]

Please see .h about inlining DestroySurfaces().
I'd prefer keeping it this way, doesn't really make a big difference, but at
least releases us from having a contract that you can't do anything with the
class after this fails.

[Ami GONE FROM CHROMIUM, 2013/05/22 23:59:47]

Why would you not want to have that contract?

[Pawel Osciak, 2013/05/24 01:46:39]

I prefer not having contracts when they can be avoided, even if they are very
unlikely (well, that is actually another argument for not having the contract).
I'm not sure what are we disagreeing here about, is it to save  us this one line
of code?

+    return false;
+  }
+
+  for (size_t i = 0; i < num_surfaces; ++i)
+    va_surfaces.push_back(va_surface_ids_[i]);

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

Once the member is a vector too you can simply assign the vector here (not
member-wise).

[Pawel Osciak, 2013/05/21 22:32:35]

Done.

+
+  return true;
+}
+
+void VaapiDelegate::DestroySurfaces() {
+  base::AutoLock auto_lock(va_lock_);
+
+  if (va_context_id_ != VA_INVALID_ID) {
+    VAStatus va_res = VAAPI_DestroyContext(va_display_, va_context_id_);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyContext failed");
+  }
+
+  if (va_surface_ids_) {
+    VAStatus va_res = VAAPI_DestroySurfaces(va_display_, va_surface_ids_.get(),
+                                            num_va_surfaces_);
+    VA_LOG_ON_ERROR(va_res, "vaDestroySurfaces failed");
+  }
+
+  va_surface_ids_.reset();
+  va_context_id_ = VA_INVALID_ID;
+}
+
+bool VaapiDelegate::SubmitBuffer(VABufferType va_buffer_type,
+                                 size_t size,
+                                 void* buffer) {
+  base::AutoLock auto_lock(va_lock_);
+
+  VABufferID buffer_id;
+  VAStatus va_res = VAAPI_CreateBuffer(va_display_, va_context_id_,
+                                       va_buffer_type, size,
+                                       1, buffer, &buffer_id);
+  VA_SUCCESS_OR_RETURN(va_res, "Failed to create a VA buffer", false);
+
+  switch (va_buffer_type) {
+    case VASliceParameterBufferType:
+    case VASliceDataBufferType:
+      pending_slice_bufs_.push_back(buffer_id);
+      break;
+
+    default:
+      pending_va_bufs_.push_back(buffer_id);
+      break;
+  }
+
+  return true;
+}
+
+void VaapiDelegate::DestroyPendingBuffers() {
+  base::AutoLock auto_lock(va_lock_);
+
+  for (size_t i = 0; i < pending_va_bufs_.size(); ++i) {
+    VAStatus va_res = VAAPI_DestroyBuffer(va_display_, pending_va_bufs_[i]);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyBuffer failed");
+  }
+
+  for (size_t i = 0; i < pending_slice_bufs_.size(); ++i) {
+    VAStatus va_res = VAAPI_DestroyBuffer(va_display_, pending_slice_bufs_[i]);
+    VA_LOG_ON_ERROR(va_res, "vaDestroyBuffer failed");
+  }
+
+  pending_va_bufs_.clear();
+  pending_slice_bufs_.clear();
+}
+
+bool VaapiDelegate::SubmitDecode(VASurfaceID va_surface_id) {
+  base::AutoLock auto_lock(va_lock_);
+
+  DVLOG(4) << "Pending VA bufs to commit: " << pending_va_bufs_.size();
+  DVLOG(4) << "Pending slice bufs to commit: " << pending_slice_bufs_.size();
+  DVLOG(4) << "Decoding into VA surface " << va_surface_id;
+
+  scoped_ptr<VABufferID[]> va_buffers(new VABufferID[pending_va_bufs_.size()]);
+  std::copy(pending_va_bufs_.begin(), pending_va_bufs_.end(), va_buffers.get());

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

Why the copies?  Is VAAPI_RenderPicture destructive of its arguments?
Or can you instead simply use &pending_va_bufs_[0] for the param?

[Pawel Osciak, 2013/05/21 22:32:35]

Yes, getting over my distaste for this syntax...

+
+  scoped_ptr<VABufferID[]> slice_buffers(
+      new VABufferID[pending_slice_bufs_.size()]);
+  std::copy(pending_slice_bufs_.begin(), pending_slice_bufs_.end(),
+            slice_buffers.get());

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

ditto

[Pawel Osciak, 2013/05/21 22:32:35]

Done.

+
+  // Get ready to decode into surface.
+  VAStatus va_res = VAAPI_BeginPicture(va_display_, va_context_id_,
+                                       va_surface_id);
+  VA_SUCCESS_OR_RETURN(va_res, "vaBeginPicture failed", false);
+
+  // Commit parameter and slice buffers.
+  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, va_buffers.get(),
+                               pending_va_bufs_.size());
+  VA_SUCCESS_OR_RETURN(va_res, "vaRenderPicture for va_bufs failed", false);
+
+  va_res = VAAPI_RenderPicture(va_display_, va_context_id_, slice_buffers.get(),
+                               pending_slice_bufs_.size());
+  VA_SUCCESS_OR_RETURN(va_res, "vaRenderPicture for slices failed", false);
+
+  // Instruct HW decoder to start processing committed buffers (decode this
+  // picture). This does not block until the end of decode.
+  va_res = VAAPI_EndPicture(va_display_, va_context_id_);
+  VA_SUCCESS_OR_RETURN(va_res, "vaEndPicture failed", false);
+
+  return true;
+}
+
+bool VaapiDelegate::DecodeAndDestroyPendingBuffers(VASurfaceID va_surface_id) {
+  bool result = SubmitDecode(va_surface_id);

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

The lock is not held across this & the next line, meaning that one thread's
DPB() call may nuke another thread's just-submitted DADPB() call?

Put another way, I suspect a better layout would be to rename
DestroyPendingBuffers to DestroyPendingBuffers_Locked, make
DestroyPendingBuffers acquire the lock and call DestroyPendingBuffers_Locked(),
inline SubmitDecode into DADPB(), and call DestroyPendingBuffers_Locked() from
the bottom of DADPB.

[Pawel Osciak, 2013/05/21 22:32:35]

I actually had it that way initially. But it was too much unnecessary code,
because then I needed a non-Locked() version of DestroyPendingBuffers() for
destroying buffers without Decode()ing and a _Locked() one for destroying after
decoding.

But there is no problem here. First of all, using this class in such a way from
multiple threads is not really allowed, Destroy could be called at any point
before a locked Decode to mess up things anyway.

It's locked well enough for the Decode (and HW itself) not to explode, because
the whole Decode is locked. If something calls Destroy, it will execute only
before or after a full Decode. Before, it will make Decode() fail, but it's not
really like we could prevent this, because it could have been called before we
locked ourselves in DecodeAndDestroyPendingBuffers anyway. And if the other
thread happens to Destroy() after Decode(), but before Destroy() executes here,
the Destroy() here will be a noop, but everything will stay in order, again not
exploding.
What you are trying to prevent is not preventable and not really a scenario that
is supported to work, but what is now here is sufficient to prevent things from
exploding at least.

+  DestroyPendingBuffers();
+  return result;
+}
+
+bool VaapiDelegate::PutSurfaceIntoPixmap(VASurfaceID va_surface_id,
+                                         Pixmap x_pixmap,
+                                         int width, int height) {
+  base::AutoLock auto_lock(va_lock_);
+
+  VAStatus va_res = VAAPI_SyncSurface(va_display_, va_surface_id);
+  VA_SUCCESS_OR_RETURN(va_res, "Failed syncing surface", false);
+
+  // Put the data into an X Pixmap.
+  va_res = VAAPI_PutSurface(va_display_,
+                            va_surface_id,
+                            x_pixmap,
+                            0, 0, width, height,
+                            0, 0, width, height,

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

is there an assumption that this class only knows about coded sizes, but not
visible rect or padding issues?

[Pawel Osciak, 2013/05/21 22:32:35]

Correct.

+                            NULL, 0, 0);
+  VA_SUCCESS_OR_RETURN(va_res, "Failed putting decode surface to pixmap",
+                       false);
+  return true;
+}
+
+// static
+bool VaapiDelegate::pre_sandbox_init_done_ = false;
+
+// static
+void VaapiDelegate::PreSandboxInitialization() {
+  DCHECK(!pre_sandbox_init_done_);
+  vaapi_handle = dlopen("libva.so", RTLD_NOW);
+  vaapi_x11_handle = dlopen("libva-x11.so", RTLD_NOW);
+  pre_sandbox_init_done_ = vaapi_handle && vaapi_x11_handle;
+}
+
+// static
+bool VaapiDelegate::PostSandboxInitialization() {
+  if (!pre_sandbox_init_done_)
+    return false;
+#define VAAPI_DLSYM(name, handle)                                       \
+  VAAPI_##name = reinterpret_cast<Vaapi##name>(dlsym((handle), "va"#name)) \
+
+  VAAPI_DLSYM(GetDisplay, vaapi_x11_handle);
+  VAAPI_DLSYM(DisplayIsValid, vaapi_handle);
+  VAAPI_DLSYM(Initialize, vaapi_handle);
+  VAAPI_DLSYM(Terminate, vaapi_handle);
+  VAAPI_DLSYM(GetConfigAttributes, vaapi_handle);
+  VAAPI_DLSYM(CreateConfig, vaapi_handle);
+  VAAPI_DLSYM(DestroyConfig, vaapi_handle);
+  VAAPI_DLSYM(CreateSurfaces, vaapi_handle);
+  VAAPI_DLSYM(DestroySurfaces, vaapi_handle);
+  VAAPI_DLSYM(CreateContext, vaapi_handle);
+  VAAPI_DLSYM(DestroyContext, vaapi_handle);
+  VAAPI_DLSYM(PutSurface, vaapi_x11_handle);
+  VAAPI_DLSYM(SyncSurface, vaapi_x11_handle);
+  VAAPI_DLSYM(BeginPicture, vaapi_handle);
+  VAAPI_DLSYM(RenderPicture, vaapi_handle);
+  VAAPI_DLSYM(EndPicture, vaapi_handle);
+  VAAPI_DLSYM(CreateBuffer, vaapi_handle);
+  VAAPI_DLSYM(DestroyBuffer, vaapi_handle);
+  VAAPI_DLSYM(ErrorStr, vaapi_handle);
+#undef VAAPI_DLSYM
+
+  return VAAPI_GetDisplay &&

[Ami GONE FROM CHROMIUM, 2013/05/17 23:19:15]

FWIW you could test for NULL and early-return as part of the macro, which would
also let you emit a more useful log message (about which symbol failed to load).

[Pawel Osciak, 2013/05/21 22:32:35]

Yeah, but honestly I don't think it'd really be that useful... It's just two
specific .so's that we have for all those functions.

[Ami GONE FROM CHROMIUM, 2013/05/22 23:59:47]

a) it would too be useful (I've had to add that code in a debugging session once
before)
b) it would allow you to drop this yet-another-list of these symbols (the return
statement would simply be "return true;" b/c all the return false's would be
part of the macro)

Reason b) is actually a good reason to do it this way IMO.

[Pawel Osciak, 2013/05/24 01:46:39]

Yeah, I see what you meant now, I like it. Done.

+      VAAPI_DisplayIsValid &&
+      VAAPI_Initialize &&
+      VAAPI_Terminate &&
+      VAAPI_GetConfigAttributes &&
+      VAAPI_CreateConfig &&
+      VAAPI_DestroyConfig &&
+      VAAPI_CreateSurfaces &&
+      VAAPI_DestroySurfaces &&
+      VAAPI_CreateContext &&
+      VAAPI_DestroyContext &&
+      VAAPI_PutSurface &&
+      VAAPI_SyncSurface &&
+      VAAPI_BeginPicture &&
+      VAAPI_RenderPicture &&
+      VAAPI_EndPicture &&
+      VAAPI_CreateBuffer &&
+      VAAPI_DestroyBuffer &&
+      VAAPI_ErrorStr;
+}
+
+}  // namespace content