Hi Flavio,
Please find my reply inline below.
Thanks & Regards,
Prince
------- Original Message -------
Date: Wed, 20 Jun 2012 15:40:14 -0300
From: Flavio Ceolin
Subject: Re: [E-devel] enlightenment-devel Digest, Vol 74, Issue 78
To: Enlightenment developer list
Message-ID: <877gv13jzl....@voyager.profusion.mobi>
Content-Type: text/plain
PRINCE KUMAR DUBEY writes:
Hi Prince,
> Hi Flavio,
> I am still not able to reproduce the seg-fault. Is seg-fault happening
> always ?
Yep, it's happening always i use the edje_player with the multisense
example.
The problem is:
+ length = count * sizeof(RemixCount);
it makes length be greater than PA_PLAYER_BUFFERLEN then:
+ *(player->playbuffer + i) = (PLAYER_PCM) value;
will access invalid memory.
I rewrote this function and worked as expected. Please check if it makes sense.
static void
pa_player_playbuffer(RemixEnv *env __UNUSED__, PA_Player_Data *player, RemixPCM
*data, RemixCount count)
{
int ret;
RemixCount i, j;
RemixPCM value;
size_t length, total_written;
length = count * sizeof(RemixPCM);
total_written = 0;
while (total_written < length) {
j = length - total_written;
j = (j > PA_PLAYER_BUFFERLEN) ? PA_PLAYER_BUFFERLEN : j;
for (i = 0; i < j; i++)
{
value = *data++ * (player->max_value);
*(player->playbuffer + i) = (PLAYER_PCM) value;
}
ret = pa_simple_write(player->server, player->playbuffer, j,
&player->error);
if (ret < 0)
{
WRN("pa_simple_write() failed: (%s)", pa_strerror(player->error));
return;
}
total_written += j;
}
}
[Prince] make sense :), thanks.
> Please find my reply to your queries inline below.
>
> Thanks & Regards,
> Prince
>
> ------- Original Message -------
> Date: Mon, 18 Jun 2012 14:09:59 -0300
> From: Fl?vio Ceolin
> Subject: Re: [E-devel] [Patch] [Edje] Added: Pulseaudio remix plug-in
> for edje multisense module
> To: Enlightenment developer list
>
> Message-ID:
>
> Content-Type: text/plain; charset=ISO-8859-1
>
> Hi Prince,
>
> On Mon, Jun 18, 2012 at 9:56 AM, PRINCE KUMAR DUBEY
> 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);
> [Prince] you are right, it must be the size of same data type.
>
> +
> + for (i = 0; i < length; i++)
>
> Shouldn't check if length is greater than the size of the buffer ?
> [Prince] I agree. we can put the check so that written length must
> never exceeds the buffer size (PA_PLAYER_BUFFERLEN).
>
> + {
> + value = *data++ * (player->max_value);
> + *(player->playbuffer + i) = (PLAYER_PCM) value;
>
> With this cast we are discarding part of the data, is it ok ?
> [Prince] Type casting is redundant, not required at all.
Just to be clear, PLAYER_PCM is short while the data(RemixPCM) is a
float, so in this cast you are losing some data, I just don't know if
it's 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
>
> -------------------------------------------------------------------
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Prince I forgot to ask you if there is a reason to export this symbol:
+EAPI extern Eina_Bool _on_edjecc;
It seems to be not necessary.
[Prince] this variable is required, in order to provide check to unnecessary
multisense framework initialization during EDC compilation/decompilation mode.
So, during EDC compilation, _on_edjecc=EINA_TRUE in edje_cc.c and hence,
_edje_multisense_init() is being ignored in edje_main.c.
Best regards,
Flavio Ceolin
----------------------------------------------
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
