<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<pre>Hello Rémi,
unfortunately, I was unable to find the "glaring invalid free" you mentioned. Do you mean the usage of "video_format_Clean"?
It was introduced in response to the first review by Filip Roséen (<a class="moz-txt-link-freetext" href="https://mailman.videolan.org/pipermail/vlc-devel/2017-May/113005.html">https://mailman.videolan.org/pipermail/vlc-devel/2017-May/113005.html</a>) exactly in the manner like in the already existing method "CreateTextRegion".
According to the "subpicture.c", the "video_format_t" get copied in case of "VLC_CODEC_YUVP" and should be therefore safe to use.
I hope that I'm not missing another bug you mean.
Christopher<b class="b2" style="font-weight: normal; color: rgb(36, 36, 36); background-color: rgb(255, 254, 239); font-family: Arial, sans-serif; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><span></span></b><b class="b5" style="font-weight: normal; color: rgb(0, 0, 0); background-color: rgb(255, 251, 184); font-family: Arial, sans-serif; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><span></span><span></span></b><b class="b4" style="font-weight: normal; color: rgb(12, 12, 12); background-color: rgb(255, 252, 207); font-family: Arial, sans-serif; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><span></span></b><b class="b4" style="font-weight: normal; color: rgb(12, 12, 12); background-color: rgb(255, 252, 207); font-family: Arial, sans-serif; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><span></span></b><b class="b4" style="font-weight: normal; color: rgb(12, 12, 12); background-color: rgb(255, 252, 207); font-family: Arial, sans-serif; font-size: 12px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; letter-spacing: normal; orphans: 2; text-align: left; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><span></span></b>
On May 29, 2017 1:48:54 PM GMT+03:00, Christopher Gundler <a class="moz-txt-link-rfc2396E" href="mailto:c.gundler@mail.de"><c.gundler@mail.de></a> wrote:
</pre>
<div class="moz-text-plain" wrap="true" style="font-family:
-moz-fixed; font-size: 12px;" lang="x-unicode">
<blockquote type="cite" style="color: #000000;">
<pre wrap="">From: Julius Schoening <a class="moz-txt-link-rfc2396E" href="mailto:julius.schoening@uni-osnabrueck.de"><julius.schoening@uni-osnabrueck.de></a>
Hello Filip,
thank you for your review of the patch. I have improve the code (and
change the author to Julius) and re-sent it now that you might have a
second look on it.
Best Regards,
Christopher
---
modules/codec/subsusf.c | 338
++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 338 insertions(+)
diff --git a/modules/codec/subsusf.c b/modules/codec/subsusf.c
index 1a94b6b4fd..a2f3dbdd8f 100644
--- a/modules/codec/subsusf.c
+++ b/modules/codec/subsusf.c
@@ -24,6 +24,7 @@
# include "config.h"
#endif
#include <assert.h>
+#include <limits.h>
#include <vlc_common.h>
#include <vlc_plugin.h>
@@ -821,7 +822,166 @@ static void ParseUSFHeaderTags( decoder_t *p_dec,
xml_reader_t *p_xml_reader )
free( p_ssa_style );
}
+/**
+ * Create a region with a white transparent picture.
+ */
+static subpicture_region_t *CreatePaintRegion( int x, int y, int
width, int height )
+{
+ video_palette_t palette = {
+ .i_entries = 2,
+ .palette = {
+ [0] = { 0xff, 0x80, 0x80, 0x00 },
+ [1] = { 0xff, 0x80, 0x80, 0xff },
+ },
+ };
+
+ video_format_t fmt;
+ video_format_Init( &fmt, VLC_CODEC_YUVP );
+ fmt.i_width =
+ fmt.i_visible_width = width;
+ fmt.i_height =
+ fmt.i_visible_height = height;
+ fmt.i_sar_num = 1;
+ fmt.i_sar_den = 1;
+ fmt.p_palette = &palette;
+
+ subpicture_region_t *r = subpicture_region_New( &fmt );
+ video_format_Clean( &fmt );
+
+ if ( !r )
+ return NULL;
+ r->i_x = x;
+ r->i_y = y;
+ memset( r->p_picture->p->p_pixels, 0, r->p_picture->p->i_pitch *
height );
+
+ return r;
+}
+
+/**
+ * Draw a rectangle into a paint region
+ */
+static void DrawRect( subpicture_region_t *r,
+ int x1, int y1, int x2, int y2 )
+{
+ uint8_t *p = r->p_picture->p->p_pixels;
+ int pitch = r->p_picture->p->i_pitch;
+
+ for ( int y = y1; y <= y2; y++ )
+ {
+ p[x1 + pitch * y] = 1;
+ p[x2 + pitch * y] = 1;
+ }
+
+ for ( int x = x1; x <= x2; x++ ) {
+ p[x + pitch * y1] = 1;
+ p[x + pitch * y2] = 1;
+ }
+}
+
+struct polydata {
+ int x;
+ int y;
+ struct polydata *next;
+};
+typedef struct polydata polydata;
+
+static void DrawPoly( subpicture_region_t *r, polydata *root )
+{
+ uint8_t *pY = r->p_picture->p[Y_PLANE].p_pixels;
+ int pitchY = r->p_picture->p[Y_PLANE].i_pitch;
+
+ polydata *curr = root;
+
+ do
+ {
+ // if end of list, set next to root for closed polygon
+ polydata *next = curr->next == NULL ? root : curr->next;
+
+ int x0 = curr->x;
+ int y0 = curr->y;
+ int x1 = next->x;
+ int y1 = next->y;
+
+ // bresenham's line algorithm
+ // from
<a class="moz-txt-link-freetext" href="http://rosettacode.org/wiki/Bitmap/Bresenham%27s_line_algorithm#C">http://rosettacode.org/wiki/Bitmap/Bresenham%27s_line_algorithm#C</a>
+ int dx = abs( x1 - x0 ), sx = x0 < x1 ? 1 : -1;
+ int dy = abs( y1 - y0 ), sy = y0 < y1 ? 1 : -1;
+ int err = ( dx > dy ? dx : -dy ) / 2, e2;
+
+ for( ; ; )
+ {
+ pY[ x0 + pitchY * y0 ] = 1;
+ if ( x0 == x1 && y0 == y1 )
+ break;
+
+ e2 = err;
+
+ if ( e2 >- dx )
+ {
+ err -= dy;
+ x0 += sx;
+ }
+
+ if ( e2 < dy )
+ {
+ err += dx;
+ y0 += sy;
+ }
+ }
+ }
+ while( ( curr = curr->next ) != NULL );
+}
+
+static void DrawCircle( subpicture_region_t *r, int x0, int y0, int
diameter )
+{
+ uint8_t *pY = r->p_picture->p[Y_PLANE].p_pixels;
+ int pitchY = r->p_picture->p[Y_PLANE].i_pitch;
+ // bresenham's circle algorithm
+ // slightly optimized from
<a class="moz-txt-link-freetext" href="https://de.wikipedia.org/wiki/Bresenham-Algorithmus#Kreisvariante_des_Algorithmus">https://de.wikipedia.org/wiki/Bresenham-Algorithmus#Kreisvariante_des_Algorithmus</a>
+ int x = diameter / 2;
+ int y = 0;
+ int err = x;
+
+ while( y <= x )
+ {
+ pY[ ( x0 + x ) + pitchY * ( y0 + y ) ] = 1;
+ pY[ ( x0 - x ) + pitchY * ( y0 + y ) ] = 1;
+ pY[ ( x0 + x ) + pitchY * ( y0 - y ) ] = 1;
+ pY[ ( x0 - x ) + pitchY * ( y0 - y ) ] = 1;
+ pY[ ( x0 + y ) + pitchY * ( y0 + x ) ] = 1;
+ pY[ ( x0 - y ) + pitchY * ( y0 + x ) ] = 1;
+ pY[ ( x0 + y ) + pitchY * ( y0 - x ) ] = 1;
+ pY[ ( x0 - y ) + pitchY * ( y0 - x ) ] = 1;
+ err -= 2 * y;
+ err++;
+ y++;
+
+ if(err < 0)
+ {
+ err--;
+ err += 2*x;
+ x--;
+ }
+ }
+}
+
+static int ParseInteger(char *str)
+{
+ long int value = strtol( str, NULL, 10 );
+ if ( value >= INT_MIN && value <= INT_MAX)
+ {
+ return (int) value;
+ }
+ else if( value < INT_MIN )
+ {
+ return INT_MIN;
+ }
+ else
+ {
+ return INT_MAX;
+ }
+}
static subpicture_region_t *ParseUSFString( decoder_t *p_dec,
char *psz_subtitle )
@@ -917,6 +1077,184 @@ static subpicture_region_t *ParseUSFString(
decoder_t *p_dec,
p_region_upto = p_region_upto->p_next;
}
}
+ else if(( !strncasecmp( psz_subtitle, "<polygon ", 9 )) ||
+ ( !strncasecmp( psz_subtitle, "<polygon>", 9 )))
+ {
+ psz_end = strstr ( psz_subtitle, "</polygon>" );
+
+ polydata *p_root = NULL;
+ polydata **pp_next = &p_root;
+
+ char *psz_point_start, *psz_point_end = NULL;
+
+ // Iterate thought the points of the polygon by moving
the pointer.
+ // For performance and readability reasons, the
assignments are exceptionally made in the condition part.
+ for(char *data = psz_subtitle;
+ data < psz_end && ( psz_point_start = strstr( data,
"<point " ) ) != NULL && ( psz_point_end = strstr( psz_point_start,
"/>" ) ) != NULL;
+ data = psz_point_end + 2)
+ {
+ char *psz_point_x = GrabAttributeValue( "posx",
psz_point_start );
+ char *psz_point_y = GrabAttributeValue( "posy",
psz_point_start );
+ *pp_next = malloc( sizeof( polydata ) );
+
+ if ( psz_point_x && psz_point_y && pp_next )
+ {
+ ( *pp_next )->x = ParseInteger( psz_point_x );
+ ( *pp_next )->y = ParseInteger( psz_point_y );
+ pp_next = &( ( *pp_next )->next );
+ }
+ else
+ {
+ pp_next = NULL;
+ }
+
+ free( psz_point_x );
+ free( psz_point_y );
+ }
+
+ if ( pp_next )
+ {
+ // create and draw picture region
+ subpicture_region_t *p_paint_region;
+ p_paint_region = CreatePaintRegion( 0, 0,
p_sys->i_original_width + 1, p_sys->i_original_height + 1 );
+
+ if( p_paint_region != NULL )
+ {
+ DrawPoly( p_paint_region, p_root );
+
+ // save picture region in list
+ if( !p_region_first )
+ {
+ p_region_first = p_region_upto =
p_paint_region;
+ }
+ else if( p_paint_region )
+ {
+ p_region_upto->p_next = p_paint_region;
+ p_region_upto = p_region_upto->p_next;
+ }
+ }
+ }
+
+ // free linked list
+ while( p_root )
+ {
+ polydata *p_tmp = p_root;
+ p_root = p_root->next;
+ free(p_tmp);
+ }
+ }
+ else if ( !strncasecmp( psz_subtitle, "<rectangle ", 11 ))
+ {
+ // get the end of the rectangle
+ psz_end = strstr ( psz_subtitle,"/>" );
+
+ // read attributes of rectangle
+ char *psz_rec_x = GrabAttributeValue( "posx",
psz_subtitle );
+ char *psz_rec_y = GrabAttributeValue( "posy",
psz_subtitle );
+ char *psz_rec_width = GrabAttributeValue( "width",
psz_subtitle );
+ char *psz_rec_height = GrabAttributeValue( "height",
psz_subtitle );
+
+ if ( psz_rec_x && psz_rec_y && psz_rec_width &&
psz_rec_height )
+ {
+ // create and draw picture region
+ subpicture_region_t *p_paint_region;
+ p_paint_region = CreatePaintRegion( 0, 0,
p_sys->i_original_width + 1, p_sys->i_original_height + 1 );
+
+ if( p_paint_region != NULL )
+ {
+ // convert to int
+ int x = ParseInteger( psz_rec_x );
+ int y = ParseInteger( psz_rec_y );
+ int w = ParseInteger( psz_rec_width );
+ int h = ParseInteger( psz_rec_height );
+
+ DrawRect( p_paint_region, x, y, x + w, y + h
);
+
+ // save picture region in list
+ if( !p_region_first )
+ {
+ p_region_first = p_region_upto =
p_paint_region;
+ }
+ else if( p_paint_region )
+ {
+ p_region_upto->p_next = p_paint_region;
+ p_region_upto = p_region_upto->p_next;
+ }
+ }
+ }
+
+ free( psz_rec_x );
+ free( psz_rec_y );
+ free( psz_rec_width );
+ free( psz_rec_height );
+ }
+ else if ( !strncasecmp( psz_subtitle, "<point ", 7 ) )
+ {
+ // get the end of the point
+ psz_end = strstr ( psz_subtitle, "/>" );
+
+ // read attributes of point
+ char *psz_point_x = GrabAttributeValue( "posx",
psz_subtitle );
+ char *psz_point_y = GrabAttributeValue( "posy",
psz_subtitle );
+ char *psz_point_d = GrabAttributeValue( "diameter",
psz_subtitle );
+
+ if ( psz_point_x && psz_point_y && psz_point_d )
+ {
+ // create and draw picture region
+ subpicture_region_t *p_paint_region;
+ p_paint_region = CreatePaintRegion( 0, 0,
p_sys->i_original_width + 1, p_sys->i_original_height + 1 );
+
+ if( p_paint_region != NULL )
+ {
+ // convert to int
+ int x = ParseInteger( psz_point_x );
+ int y = ParseInteger( psz_point_y );
+ int d = ParseInteger( psz_point_d );
+
+ DrawCircle( p_paint_region, x, y, d );
+
+ // save picture region in list
+ if( !p_region_first )
+ {
+ p_region_first = p_region_upto =
p_paint_region;
+ }
+ else if( p_paint_region )
+ {
+ p_region_upto->p_next = p_paint_region;
+ p_region_upto = p_region_upto->p_next;
+ }
+ }
+ }
+
+ free( psz_point_x );
+ free( psz_point_y );
+ free( psz_point_d );
+ }
+ else if(( !strncasecmp( psz_subtitle, "<text ", 6 )) ||
+ ( !strncasecmp( psz_subtitle, "<text>", 6 )))
+ {
+ subpicture_region_t *p_text_region;
+ psz_end = strstr( psz_subtitle, "</text>" );
+ p_text_region = CreateTextRegion( p_dec,
+ psz_subtitle,
+ p_sys->i_align );
+
+ if( p_text_region )
+ {
+ free( p_text_region->p_text->psz_text );
+ p_text_region->p_text->psz_text = CreatePlainText(
psz_subtitle );
+ }
+
+ if( !p_region_first )
+ {
+ p_region_first = p_region_upto = p_text_region;
+ }
+ else if( p_text_region )
+ {
+ p_region_upto->p_next = p_text_region;
+ p_region_upto = p_region_upto->p_next;
+ }
+ }
else
{
subpicture_region_t *p_text_region;
--
2.12.2
_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
<a class="moz-txt-link-freetext" href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a>
</pre>
</blockquote>
<pre wrap="">> There is a glaring invalid free in the first few lines. I stopped reviewing there.
<div class="moz-txt-sig">> --
> Rémi Denis-Courmont
</div></pre>
</div>
</body>
</html>