[VDPAU] [PATCH] Add detection for multiple flash substrings

[Sergey Frolov]

Hi José,

Thanks for your reply.

> I think this is a nice change. I never liked having
> "libflashplayer.so" hard coded in vdpau_wrapper.c.
> 
> I worry, though, that this change exposes us to churn based on
> Mozilla and Opera policy decision about what they do with their
> handling of the Flash plugin. However, my observation has been that
> these policies do not change often. In fact the original motivation
> for having Flash detection was Adobe's unwillingness to fix their
> bugs.

My thinking behind providing default settings to vdpau_wrapper.cfg was:
it was a default behaviour for some time now, let someone else break it
(Adobe). And it's well advertised too, since when I was finally able to
experience the issue years after everyone else (thanks to
libvdpau-va-gl1 on Intel HD3000), I knew where to look for solution.

That's when I was greeted with hardcoded value.

> How is _flash_process_signatures initialized or terminated? This line
> worries me:
> 
>  for (signature = _flash_process_signatures[0]; signature != NULL;
>       signature++) {
> 
> If static char _flash_process_signatures[16][64] is uninitialized and
> contains random data this loop will run off into the weeds. We also
> need to assume that a user will modify their vdpau_wrapper.cfg and
> not have any workarounds (unlikely, but this must work correctly). Or
> maybe I am wrong here; if so please tell me how.

Yeah, I've totally missed this case, now I'm initializing array of
pointers with NULL.

> Using a statically sized array means VDPAU will only support the
> first N entries of flash_process=. This is okay but should be
> documented.

I understand why statically allocated chunk of memory may hurt user
experience along with developer's common sense. This was a quick
decision in order to make it work, made in "ought to be enough for
everybody" mentality. Hypothetically, somewhere for someone that might
not be true.

I propose a middle ground here: statically allocated array of pointers
enough for about 8-16 times amount of flash applications on *nix
systems with no explicit restrictions on length of the parameter.

> 
> Mechanical nit: I prefer "char *signature" instead of "char *
> signature". If you do not do this I will do so in a follow on change.

Corrected.


Since I've finished K&R a week before I sent this patch any of your
remarks are welcome.

Regards,
Sergey.


diff --git a/src/vdpau_wrapper.c b/src/vdpau_wrapper.c
index 8efbd39..5d3a347 100644
--- a/src/vdpau_wrapper.c
+++ b/src/vdpau_wrapper.c
@@ -239,6 +239,7 @@ static void _vdp_close_driver(void)
 static VdpGetProcAddress * _imp_get_proc_address;
 static VdpVideoSurfacePutBitsYCbCr * _imp_vid_put_bits_y_cb_cr;
 static VdpPresentationQueueSetBackgroundColor * _imp_pq_set_bg_color;
+static char *_flash_process_signatures[128] = {NULL};
 static int _running_under_flash;
 static int _enable_flash_uv_swap = 1;
 static int _disable_flash_pq_bg_color = 1;
@@ -324,6 +325,7 @@ static void init_running_under_flash(void)
 {
     FILE *fp;
     char buffer[1024];
+    char **signature;
     int ret, i;
 
     fp = fopen("/proc/self/cmdline", "r");
@@ -346,8 +348,11 @@ static void init_running_under_flash(void)
     }
     buffer[ret] = '\0';
 
-    if (strstr(buffer, "libflashplayer") != NULL) {
-        _running_under_flash = 1;
+    for (signature = &_flash_process_signatures[0]; *signature !=
NULL; signature++) {
+        if (strstr(buffer, *signature) != NULL) {
+            _running_under_flash = 1;
+        }
+        free(*signature);
     }
 }
 
@@ -355,12 +360,15 @@ static void init_config(void)
 {
     FILE *fp;
     char buffer[1024];
+    char **signature;
 
     fp = fopen(VDPAU_SYSCONFDIR "/vdpau_wrapper.cfg", "r");
     if (!fp) {
         return;
     }
 
+    signature = &_flash_process_signatures[0];
+
     while (fgets(buffer, sizeof(buffer), fp) != NULL) {
         char * equals = strchr(buffer, '=');
         char * param;
@@ -371,6 +379,7 @@ static void init_config(void)
 
         *equals = '\0';
         param = equals + 1;
+        param[strcspn(param, "\n")] = '\0';
 
         if (!strcmp(buffer, "enable_flash_uv_swap")) {
             _enable_flash_uv_swap = atoi(param);
@@ -378,6 +387,11 @@ static void init_config(void)
         else if (!strcmp(buffer, "disable_flash_pq_bg_color")) {
             _disable_flash_pq_bg_color = atoi(param);
         }
+        else if (!strcmp(buffer, "flash_process")) {
+            *signature = (char *) malloc(strlen(param)+1);
+            strncpy(*signature, param, strlen(param));
+            signature++;
+        }
     }
 
     fclose(fp);
@@ -385,8 +399,8 @@ static void init_config(void)
 
 static void init_fixes(void)
 {
-    init_running_under_flash();
     init_config();
+    init_running_under_flash();
 }
 
 VdpStatus vdp_device_create_x11(
diff --git a/src/vdpau_wrapper.cfg b/src/vdpau_wrapper.cfg
index 21d5b8c..bd20fcd 100644
--- a/src/vdpau_wrapper.cfg
+++ b/src/vdpau_wrapper.cfg
@@ -1,2 +1,5 @@
+flash_process=libflashplayer.so
+flash_process=flash-mozilla.so
+flash_process=operapluginwrapper-native
 enable_flash_uv_swap=1
 disable_flash_pq_bg_color=1