Re: [E-devel] [Patch] [Edje] Added: Pulseaudio remix plug-in for edje multisense module

[Flávio C. <fla...@pr...>]

Hi Prince,

On Mon, Jun 18, 2012 at 9:56 AM, PRINCE KUMAR DUBEY
<pri...@sa...> wrote:
> Hi Flavio,
>
> I tested "edje_player multisense.edje" to reproduce seg fault as mentioned below, but couldn't succeed with memcheck as well.
> My svn rev is 72374. Please let me know the scenario to reproduce it.
>
> And regarding the point 5, you are right, multisense will not build without remix as per current configure file.
> But, multisense is not all about only sound, it can have vibration etc. So, if we make remix optional for multisense feature,
> edje must compile e.g. ENABLE_MULTISENSE=1 and HAVE_REMIX=0.
>
> Regards,
> Prince
>

I've tested in rev 72117 and now 72400 in both the problem happened. I
built with these options:
/configure --enable-tests --enable-build-examples --enable-multisense
CFLAGS=-Wall -g -O0

As for the patch, I took a look in it and I would like to understand
some points.

+static void
+pa_player_playbuffer(RemixEnv *env __UNUSED__, PA_Player_Data
*player, RemixPCM *data, RemixCount count)
+{
+   int ret;
+   RemixCount i;
+   RemixPCM value;
+   size_t length;
+
+   length = count * sizeof(RemixCount);

Shouldn't it be
length = count * sizeof(RemixPCM);

+
+   for (i = 0; i < length; i++)

Shouldn't check if length is greater than the size of the buffer ?

+     {
+        value = *data++ * (player->max_value);
+        *(player->playbuffer + i) = (PLAYER_PCM) value;

With this cast we are discarding part of the data, is it ok ?

+     }
+
+   ret = pa_simple_write(player->server, player->playbuffer, length,
&player->error);
+
+   if (ret < 0) WRN("pa_simple_write() failed: (%s)",
pa_strerror(player->error));
+
+   return;
+}


Best regards,
Flavio Ceolin