[vlc-devel] [Patch] New audio & video filters for audio bargraph display inside video

[Rémi Denis-Courmont]

	Hello,

Le lundi 16 novembre 2009 12:30:55 clement chesnin, vous avez écrit :
> Here is a new version made with git, including the changes you
> suggested. Please tell us if there are things to change or tweak...

Uuh, please double check your patches. You sent tens of thousands of extra 
lines through the PO files.

+	if (!(p_sys->TCPconnection = net_ConnectTCP(p_this,p_sys->address,p_sys-
>port))) {
+        free(p_sys);
+		return VLC_EGENERIC;
+	}

The error value is -1, not 0. That said, I wonder why you use TCP instead of 
an intra-process mechanism.

+	char *message = (char*)malloc(255*sizeof(char));

sizeof(char) is fundamentally always one.
'char message[255];' would be much simpler here.

+	nbChannels = aout_FormatNbChannels( &p_filter->fmt_in.audio );
+	if (p_sys->nbChannels != nbChannels) {
+		/*free(p_sys->value);
+		p_sys->value = (int*)malloc(nbChannels * sizeof(int));
+		for (i=0; i<nbChannels; i++) {
+			p_sys->value[i] = 0;
+		}*/
+		p_sys->nbChannels = nbChannels;
+	}

The if statement is useless here.

+			/* 5 - compare it to the threshold */
+			sprintf(message,"@audiobargraph_v audiobargraph_v-alarm ");
+			if (sum < p_sys->alarm_threshold) {
+				sprintf(message,"%s1\n",message);
+			} else {
+				sprintf(message,"%s0\n",message);
+			}

This is undefined use of sprintf() and will fail on some platforms.

+	if (p_sys->bargraph) {
+		/* 6 - sent the message with the values for the BarGraph */
+		if ((nbChannels > 0) && (p_sys->counter%(p_sys->bargraph_repetition) == 0)) 
{
+			sprintf(message,"@audiobargraph_v audiobargraph_v-i_values ");
+			for (i=0; i<(nbChannels-1); i++) {
+				sprintf(message,"%s%f:", message, i_value[i]);
+			}
+			sprintf(message,"%s%f\n", message, i_value[nbChannels-1]);

Same problem as above.

+    p_sys->p_blend = NULL;
+    if( !b_sub )
+    {
+
+        p_sys->p_blend = filter_NewBlend( VLC_OBJECT(p_filter),
+                                          &p_filter->fmt_in.video );
+        if( !p_sys->p_blend )
+        {
+            free( p_sys );
+            return VLC_EGENERIC;

You're leaking p_barGraph here.

+	res = strtok(i_values, delim);
+	while (res != NULL) {
+		nbChannels++;
+		p_BarGraph->i_values = (int*)realloc(p_BarGraph->i_values, 
nbChannels*sizeof(int));
+		p_BarGraph->i_values[nbChannels-1] = __MAX( __MIN( atof(res)*p_BarGraph-
>scale, p_BarGraph->scale ), 0 );
+		p_BarGraph->nbChannels = nbChannels;
+		res = strtok(NULL, delim);
+	}

strtok() is not thread-safe. Use strtok_r() instead.

+		res = strtok(i_values, delim);
+		while (res != NULL) {
+			p_BarGraph->nbChannels++;
+			p_BarGraph->i_values = (int*)realloc(p_BarGraph->i_values, p_BarGraph-
>nbChannels*sizeof(int));
+			p_BarGraph->i_values[p_BarGraph->nbChannels-1] = __MAX( __MIN( 
atof(res)*p_BarGraph->scale, p_BarGraph->scale ), 0 );
+			res = strtok(NULL, delim);
+		}

Same as above. You could probably factor this code.

-- 
Rémi Denis-Courmont
http://www.remlab.net/