[vlc-devel] [PATCH] Video_filter motiondetect to send shapes over UDP.
gonzague
gonzagueddr at gmail.com
Tue Feb 7 17:52:10 CET 2017
Yes you're rigth, i apologise for this copy.
I was needed this option again and i had to recode and recompile, so i
thougth it was a good idea to propose this patch.
I did not even think about browsing list archive to find this patch,
shame on me ...
Thanks fo your advices, i'll try to correct that.
Le 07/02/2017 à 15:49, Filip Roséen a écrit :
>
> Hi Gonzague,
>
> These changes seems to be an iteration of a patch posted by you back
> in June 2012, is that correct?
>
> * https://patches.videolan.org/patch/413/
>
> On 2017-02-07 15:14, Gonzague Defos du Rau wrote:
>
> |If the option --motiondetect-port is set, detected shapes are sent
> on localhost over UDP. Packet looks like "ShapeNumber x_min x_max
> y_min y_max ;\n..." --- modules/video_filter/motiondetect.c | 63
> +++++++++++++++++++++++++++++++++++++ 1 file changed, 63
> insertions(+) diff --git a/modules/video_filter/motiondetect.c
> b/modules/video_filter/motiondetect.c index fee7e60..a13d6b8
> 100644 --- a/modules/video_filter/motiondetect.c +++
> b/modules/video_filter/motiondetect.c @@ -36,6 +36,7 @@ #include
> <vlc_picture.h> #include "filter_picture.h" +#include
> <vlc_network.h>
> /*****************************************************************************
> * Module descriptor
> *****************************************************************************/
> @@ -44,6 +45,11 @@ static void Destroy ( vlc_object_t * ); #define
> FILTER_PREFIX "motiondetect-" +#define PORT_TEXT N_( "motiondetect
> resend udp port" ) +#define PORT_LONGTEXT N_( \ + "resend detected
> boxes values over udp, "\ + "default port is 0, that means
> resending is not active" )|
>
> Could be rephrased so easier explain the intent of the code.
>
> |+ vlc_module_begin () set_description( N_("Motion detect video
> filter") ) set_shortname( N_( "Motion Detect" )) @@ -51,6 +57,9 @@
> vlc_module_begin () set_subcategory( SUBCAT_VIDEO_VFILTER )
> set_capability( "video filter", 0 ) + add_integer(FILTER_PREFIX
> "port", 0, + PORT_TEXT, PORT_LONGTEXT, false) + add_shortcut(
> "motion" ) set_callbacks( Create, Destroy ) vlc_module_end () @@
> -66,6 +75,11 @@ static int FindShapes( uint32_t *, uint32_t *,
> int, int, int, static void Draw( filter_t *p_filter, uint8_t
> *p_pix, int i_pix_pitch, int i_pix_size ); #define NUM_COLORS
> (5000) +/* Boxes UDP resend, options */ +static const char *const
> ppsz_filter_options[] = { + "port", NULL +}; + struct filter_sys_t
> { bool is_yuv_planar; @@ -81,6 +95,10 @@ struct filter_sys_t int
> color_x_max[NUM_COLORS]; int color_y_min[NUM_COLORS]; int
> color_y_max[NUM_COLORS]; + + /* Boxes UDP resend , socket */ + int
> fd; + int port;|
>
> After the socket has been created there is no need for
> |filter_sys_t::port|, as such you can remove the /data-member/ from
> the above struct.
>
> Instead of using |p_sys->port > 0|, you can rely on the fact that
> /file-descriptors/ cannot be negative, and replace the former usage
> with |p_sys->fd != -1| (where |-1| denotes no connection, or similar).
>
> |};
> /*****************************************************************************
> @@ -115,6 +133,26 @@ static int Create( vlc_object_t *p_this ) if(
> p_filter->p_sys == NULL ) return VLC_ENOMEM; + /* Boxes UDP resend
> , open socket */ + int fd; + config_ChainParse( p_filter,
> FILTER_PREFIX, ppsz_filter_options, p_filter->p_cfg ); + int
> i_port = var_CreateGetIntegerCommand( p_filter, FILTER_PREFIX
> "port" );|
>
> Why not use |p_sys->port| all the way through, instead of having both
> |i_port| and |p_sys->port|?
>
> |+ if( i_port > 0 ) + { + fd = net_ConnectUDP( VLC_OBJECT( p_filter
> ), "127.0.0.1", i_port, -1 ); + if (fd == -1) + { +
> msg_Err(p_filter, "Can not open socket"); + i_port = 0 ; +
> p_sys->port = 0 ; + } + else + { + p_sys->fd = fd; + p_sys->port =
> i_port ; + } + } + p_sys->is_yuv_planar = is_yuv_planar;
> p_sys->b_old = false; p_sys->p_old = picture_NewFromFormat( p_fmt
> ); @@ -144,6 +182,11 @@ static void Destroy( vlc_object_t *p_this
> ) free( p_sys->p_buf2 ); free( p_sys->p_buf ); picture_Release(
> p_sys->p_old ); + + /* Boxes UDP resend , close socket */ + if(
> p_sys->port > 0 ) + net_Close( p_sys->fd ); + free( p_sys ); } @@
> -522,6 +565,8 @@ static void Draw( filter_t *p_filter, uint8_t
> *p_pix, int i_pix_pitch, int i_pix filter_sys_t *p_sys =
> p_filter->p_sys; int j = 0; + + char buffer[NUM_COLORS] = "";|
>
> Is the above guaranteed not to overflow when using |sprintf|?
>
> At least use |snprintf| if it is not 100% clear that it will not
> overflow, or make use of the functionality in |vlc_memstream.h| to
> easily create strings (though beware that they will be heap-allocated).
>
> |for( int i = 1; i < p_sys->i_colors; i++ ) { @@ -535,6 +580,17 @@
> static void Draw( filter_t *p_filter, uint8_t *p_pix, int
> i_pix_pitch, int i_pix const int color_y_min =
> p_sys->color_y_min[i]; const int color_y_max =
> p_sys->color_y_max[i]; + /** + * Boxes UDP resend, building datas
> packet. + * The packet concatenate lines of boxe's number, + *
> x_min, x_max, y_min and y_max separated by a space, + * and ending
> with "space semi-colon new-line" (" ;\n") + */ + if( p_sys->port >
> 0) + { + sprintf(buffer, "%s%d %d %d %d %d ;\n", buffer, j,
> color_x_min, color_x_max, color_y_min, color_y_max);|
>
> To second /Rémi/ in the previously linked discussion; where is this
> format specified?
>
> |+ } + if( color_x_min == -1 ) continue; if( ( color_y_max -
> color_y_min ) * ( color_x_max - color_x_min ) < 16 ) @@ -558,5
> +614,12 @@ static void Draw( filter_t *p_filter, uint8_t *p_pix,
> int i_pix_pitch, int i_pix for( y = color_y_min; y <= color_y_max;
> y++ ) p_pix[y*i_pix_pitch+x*i_pix_size] = 0xff; } + msg_Dbg(
> p_filter, "Counted %d moving shapes.", j ); + + /* Boxes UDP
> resend, sending buffer */|
>
> The below does not look correct as there is nothing stating that the
> entire data in |sizeof( buffer )| contains valid data, unless the (not
> mentioned) protocol specifies that trailing null-bytes are to be sent.
>
> It looks like you want to store the length of the generated data
> somewhere, and only send that.
>
> |+ if ( send( p_sys->fd, buffer, sizeof( buffer ), 0 ) <= 0 )|
>
> You should treat |send( p_sys->fd, ..., data_size ) != data_size| as
> an error, as send might not be able to send the full packet
> (|data_size| is an imaginary variable used as a replacement for
> |sizeof( buffer )| given the previous remark).
>
> |+ { + msg_Dbg( p_filter, "Can not send to socket");|
>
> An error such as the above is worthy of /at least/ |msg_Warn|.
>
> |+ } }|
>
>
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20170207/c14bc44c/attachment.html>
More information about the vlc-devel
mailing list