Re: [FFmpeg-devel] [PATCH] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-28 1:46 GMT+01:00 Colin NG : > +char tmp_str[MAX_URL_SIZE]; > +char tmp_str_2[MAX_URL_SIZE]; Assuming this is not speed-critical code, please also allocate these two with av_malloc(), FFmpeg can be compiled for systems with tiny stack. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-28 8:46 GMT+08:00 Colin NG :
> ---
> libavformat/dashdec.c | 95
> ---
> 1 file changed, 82 insertions(+), 13 deletions(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 3798649..5d5970e 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -148,6 +148,11 @@ static uint64_t get_current_time_in_sec(void)
> return av_gettime() / 100;
> }
>
> +static char * ishttp(char *url) {
> +char *proto_name = avio_find_protocol_name(url);
> +return av_strstart(proto_name, "http", NULL);
> +}
> +
> static uint64_t get_utc_date_time_insec(AVFormatContext *s, const char
> *datetime)
> {
> struct tm timeinfo;
> @@ -392,7 +397,9 @@ static int open_url(AVFormatContext *s, AVIOContext **pb,
> const char *url,
> else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5))
> return AVERROR_INVALIDDATA;
>
> -ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
> +av_freep(pb);
> +ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback, &tmp);
> +
> if (ret >= 0) {
> // update cookies on http response with setcookies.
> char *new_cookies = NULL;
> @@ -639,6 +646,74 @@ static int
> parse_manifest_segmenttimeline(AVFormatContext *s, struct representat
> return 0;
> }
>
> +static int resolve_content_path(AVFormatContext *s, const char *url,
> xmlNodePtr *baseurl_nodes, int n_baseurl_nodes) {
> +
> +int i;
> +char *text;
> +char tmp_str[MAX_URL_SIZE];
> +char tmp_str_2[MAX_URL_SIZE];
> +
> +char *path = av_mallocz(MAX_URL_SIZE);
> +int nameSize = 0;
> +int updated = 0;
> +
if (!path) {
return AVERROR(ENOMEM);
}
> +av_strlcpy(tmp_str, url, strlen(url)+1);
> +char *mpdName = strtok (tmp_str," /");
> +
> +while ((mpdName =strtok (NULL, "/"))) {
> +nameSize = strlen(mpdName);
> +}
> +
> +av_strlcpy (path, url, strlen(url)-nameSize+1);
> +
> +int rootId = 0;
> +xmlNodePtr *node = NULL;
> +for (rootId = n_baseurl_nodes-1; rootId >0; rootId--) {
> +if (!(node = baseurl_nodes[rootId])) continue;
> +if (ishttp(xmlNodeGetContent(node))) {
> +break;
> +}
> +}
> +
> +node = baseurl_nodes[rootId];
> +char *baseurl = xmlNodeGetContent(node);
> +char *root_url = (!av_strcasecmp(baseurl, ""))? path: baseurl;
av_strcasecmp(baseurl, "") ? baseurl : patch
> +
> +if (node) {
> +xmlNodeSetContent(node, root_url);
> +}
> +
> +int size = strlen(root_url);
> +char *isRootHttp= ishttp(root_url);
> +
> +char token ='/';
> +if (av_strncasecmp(&root_url[size-1],&token, 1) != 0) {
is this means if (&root_url[size-1] == &token) ?
> +av_strlcat(root_url, "/", size+2);
> +size+=2;
> +}
> +
> +for (i = 0; i < n_baseurl_nodes; ++i) {
> +if (i==rootId) continue;
> +text = xmlNodeGetContent(baseurl_nodes[i]);
> +if (text) {
> +memset(tmp_str, 0, strlen(tmp_str));
> +
> +if (!ishttp(text) && isRootHttp) {
> +av_strlcpy(tmp_str, root_url, size+1);
> +}
> +int start = (av_strncasecmp(text, &token, 1) == 0) ? 1: 0;
same with above comment.
> +memset(tmp_str_2, 0, strlen(tmp_str_2));
> +av_strlcat(tmp_str, text+start, MAX_URL_SIZE);
> +xmlFree(text);
> +xmlNodeSetContent(baseurl_nodes[i], tmp_str);
> +updated = 1;
> +}
> +}
> +
> +av_free(path);
> +return updated;
> +
> +}
> static int parse_manifest_representation(AVFormatContext *s, const char *url,
> xmlNodePtr node,
> xmlNodePtr adaptionset_node,
> @@ -698,6 +773,8 @@ static int parse_manifest_representation(AVFormatContext
> *s, const char *url,
> baseurl_nodes[2] = adaptionset_baseurl_node;
> baseurl_nodes[3] = representation_baseurl_node;
>
> +resolve_content_path(s, url, baseurl_nodes, 4);
check return value.
> +
> if (representation_segmenttemplate_node || fragment_template_node) {
> fragment_timeline_node = NULL;
> fragment_templates_tab[0] = representation_segmenttemplate_node;
> @@ -993,6 +1070,9 @@ static int parse_manifest(AVFormatContext *s, const char
> *url, AVIOContext *in)
> }
>
> mpd_baseurl_node = find_child_node_by_name(node, "BaseURL");
> +if (!mpd_baseurl_node) {
> +mpd_baseurl_node = xmlNewNode(node, "BaseURL");
> +}
>
> // at now we can handle only one period, with the longest duration
> node = xmlFirstElementChild(node);
> @@ -1315,6 +1395,7 @@ static int read_from_url(struct representation *pls,
> struct fragment *seg,
> } else {
> ret = avio_read(pls->input, buf, buf_size);
> }
> +
> if (ret > 0)
> pls->cur_seg_offset
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-23 9:16 GMT+08:00 Colin NG :
> Clean up coding style.
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 3798649..7e7ae52 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -148,6 +148,11 @@ static uint64_t get_current_time_in_sec(void)
return av_gettime() / 100;
}
+static char *ishttp(char *url) {
+char *proto_name = avio_find_protocol_name(url);
+return av_strstart(proto_name, "http", NULL);
+}
+
static uint64_t get_utc_date_time_insec(AVFormatContext *s, const
char *datetime)
{
struct tm timeinfo;
@@ -392,7 +397,8 @@ static int open_url(AVFormatContext *s,
AVIOContext **pb, const char *url,
else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5))
return AVERROR_INVALIDDATA;
-ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
+av_freep(pb);
+ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback, &tmp);
if (ret >= 0) {
// update cookies on http response with setcookies.
char *new_cookies = NULL;
@@ -639,6 +645,78 @@ static int
parse_manifest_segmenttimeline(AVFormatContext *s, struct representat
return 0;
}
+static int resolve_content_path(AVFormatContext *s, const char *url,
xmlNodePtr *baseurl_nodes, int n_baseurl_nodes)
+ {
+int i;
+char *text;
+char tmp_str[MAX_URL_SIZE];
+char tmp_str_2[MAX_URL_SIZE];
+
+char *path = av_mallocz(MAX_URL_SIZE);
+int nameSize = 0;
+int updated = 0;
+
+av_strlcpy(tmp_str, url, strlen(url)+1);
+char *mpdName = strtok(tmp_str, "/");
+
+while ((mpdName = strtok(NULL, "/"))) {
+nameSize = strlen(mpdName);
+}
+
+av_strlcpy(path, url, strlen(url)-nameSize+1);
+
+int rootId = 0;
+xmlNodePtr *node = NULL;
+for (rootId = n_baseurl_nodes-1; rootId > 0; rootId--) {
+if (!(node = baseurl_nodes[rootId])) {
+continue;
+}
+if (ishttp(xmlNodeGetContent(node))) {
+break;
+}
+}
+
+node = baseurl_nodes[rootId];
+char *baseurl = xmlNodeGetContent(node);
+char *root_url = (!av_strcasecmp(baseurl, ""))? path: baseurl;
+
+if (node) {
+xmlNodeSetContent(node, root_url);
+}
+
+int size = strlen(root_url);
+char *isRootHttp = ishttp(root_url);
+
+char token = '/';
+if (strncmp(&root_url[size-1], &token, 1) != 0) {
why not av_strncasecmp?
+av_strlcat(root_url, "/", size+2);
+size+=2;
+}
+
+for (i = 0; i < n_baseurl_nodes; i++) {
+if (i == rootId) {
+continue;
+}
+text = xmlNodeGetContent(baseurl_nodes[i]);
+if (text) {
+memset(tmp_str, 0, strlen(tmp_str));
+
+if (!ishttp(text) && isRootHttp) {
+av_strlcpy(tmp_str, root_url, size+1);
+}
+int start = (strncmp(text, &token, 1) == 0) ? 1: 0;
why not av_strncasecmp?
+memset(tmp_str_2, 0, strlen(tmp_str_2));
+av_strlcat(tmp_str, text+start, MAX_URL_SIZE);
+xmlFree(text);
+xmlNodeSetContent(baseurl_nodes[i], tmp_str);
+updated = 1;
+}
+}
+
+av_free(path);
+return updated;
+
+}
static int parse_manifest_representation(AVFormatContext *s, const char *url,
xmlNodePtr node,
xmlNodePtr adaptionset_node,
@@ -698,6 +776,8 @@ static int
parse_manifest_representation(AVFormatContext *s, const char *url,
baseurl_nodes[2] = adaptionset_baseurl_node;
baseurl_nodes[3] = representation_baseurl_node;
+resolve_content_path(s, url, baseurl_nodes, 4);
check this call return value. If there are no return value, make it void type.
+
if (representation_segmenttemplate_node || fragment_template_node) {
fragment_timeline_node = NULL;
fragment_templates_tab[0] = representation_segmenttemplate_node;
@@ -993,6 +1073,9 @@ static int parse_manifest(AVFormatContext *s,
const char *url, AVIOContext *in)
}
mpd_baseurl_node = find_child_node_by_name(node, "BaseURL");
+if (!mpd_baseurl_node) {
+mpd_baseurl_node = xmlNewNode(node, "BaseURL");
+}
// at now we can handle only one period, with the longest duration
node = xmlFirstElementChild(node);
BTW: use command looks like this:
1. git commit -a
2. git format-patch -1 or git format-patch -s -2 to make patch.
refer to: https://ffmpeg.org/developer.html#Submitting-patches-1
Thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
On Sat, Nov 18, 2017 at 00:16:33 +, Colin NG wrote:
> Cleaned the code up.
Please do read the ffmpeg style guide.
> +static char * ishttp(char *url) {
Asterisk attaches to the right hand side.
> -ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
> +av_freep(pb);
> +ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback, &tmp);
> +
> if (ret >= 0) {
No reason to introduce an exta empty line.
> +static int resolve_content_path(AVFormatContext *s, const char *url,
> xmlNodePtr *baseurl_nodes, int n_baseurl_nodes) {
Opening curly bracket for functions goes on a new line.
> +av_strlcpy(tmp_str, url, strlen(url)+1);
Operators get spaces around them.
> +char *mpdName = strtok (tmp_str," /");
Space after comma.
> +while ((mpdName =strtok (NULL, "/"))) {
Operators get spaces around them.
> +av_strlcpy (path, url, strlen(url)-nameSize+1);
No space before opening bracket (except for if, while, ...).
Operators get spaces around them.
> +xmlNodePtr *node = NULL;
Too much whitespace.
> +for (rootId = n_baseurl_nodes-1; rootId >0; rootId--) {
Operators get spaces around them.
> +if (!(node = baseurl_nodes[rootId])) continue;
Newline. Preferably with {}.
> +char *root_url = (!av_strcasecmp(baseurl, ""))? path: baseurl;
Operators get spaces around them.
> +char *isRootHttp= ishttp(root_url);
Operators get spaces around them.
> +char token ='/';
Operators get spaces around them.
> +if (strncmp(&root_url[size-1],&token, 1) != 0) {
Operators get spaces around them.
Spaces after commas.
> +size+=2;
Operators get spaces around them.
> +for (i = 0; i < n_baseurl_nodes; ++i) {
i++
> +if (i==rootId) continue;
Newline. Preferably with {}.
> +av_strlcpy(tmp_str, root_url, size+1);
Operators get spaces around them.
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
Cleaned the code up.
From: ffmpeg-devel on behalf of Carl Eugen
Hoyos
Sent: November 17, 2017 5:24 AM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-17 1:55 GMT+01:00 Colin NG :
> Excluded the fix for byte range issue and update some coding style issues.
> +if (pb) av_freep(pb);
The if() should be unnecessary.
Please avoid adding empty lines in unrelated parts of the file
and while there, try to fix "if (condition) {" to "if (condition) {"
(one space).
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
ffmpeg-devel Info Page<http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
ffmpeg.org
This list is about FFmpeg development discussions and patches; but not for
bug-reports. Please read the Code-of-conduct. To see the collection of prior
postings to ...
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 671ae9d..9bdfefd 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -148,6 +148,11 @@ static uint64_t get_current_time_in_sec(void)
return av_gettime() / 100;
}
+static char * ishttp(char *url) {
+char *proto_name = avio_find_protocol_name(url);
+return av_strstart(proto_name, "http", NULL);
+}
+
static uint64_t get_utc_date_time_insec(AVFormatContext *s, const char
*datetime)
{
struct tm timeinfo;
@@ -392,7 +397,9 @@ static int open_url(AVFormatContext *s, AVIOContext **pb,
const char *url,
else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5))
return AVERROR_INVALIDDATA;
-ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
+av_freep(pb);
+ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback, &tmp);
+
if (ret >= 0) {
// update cookies on http response with setcookies.
char *new_cookies = NULL;
@@ -640,6 +647,74 @@ static int parse_manifest_segmenttimeline(AVFormatContext
*s, struct representat
return 0;
}
+static int resolve_content_path(AVFormatContext *s, const char *url,
xmlNodePtr *baseurl_nodes, int n_baseurl_nodes) {
+
+int i;
+char *text;
+char tmp_str[MAX_URL_SIZE];
+char tmp_str_2[MAX_URL_SIZE];
+
+char *path = av_mallocz(MAX_URL_SIZE);
+int nameSize = 0;
+int updated = 0;
+
+av_strlcpy(tmp_str, url, strlen(url)+1);
+char *mpdName = strtok (tmp_str," /");
+
+while ((mpdName =strtok (NULL, "/"))) {
+nameSize = strlen(mpdName);
+}
+
+av_strlcpy (path, url, strlen(url)-nameSize+1);
+
+int rootId = 0;
+xmlNodePtr *node = NULL;
+for (rootId = n_baseurl_nodes-1; rootId >0; rootId--) {
+if (!(node = baseurl_nodes[rootId])) continue;
+if (ishttp(xmlNodeGetContent(node))) {
+break;
+}
+}
+
+node = baseurl_nodes[rootId];
+char *baseurl = xmlNodeGetContent(node);
+char *root_url = (!av_strcasecmp(baseurl, ""))? path: baseurl;
+
+if (node) {
+xmlNodeSetContent(node, root_url);
+}
+
+int size = strlen(root_url);
+char *isRootHttp= ishttp(root_url);
+
+char token ='/';
+if (strncmp(&root_url[size-1],&token, 1) != 0) {
+av_strlcat(root_url, "/", size+2);
+size+=2;
+}
+
+for (i = 0; i < n_baseurl_nodes; ++i) {
+if (i==rootId) continue;
+text = xmlNodeGetContent(baseurl_nodes[i]);
+if (text) {
+memset(tmp_str, 0, strlen(tmp_str));
+
+if (!ishttp(text) && isRootHttp) {
+av_strlcpy(tmp_str, root_url, size+1);
+}
+int start = (strncmp(text, &token, 1) == 0) ? 1: 0;
+memset(tmp_str_2, 0, strlen(tmp_str_2));
+av_strlcat(tmp_str, text+start, MAX_URL_SIZE);
+xmlFree(text);
+xmlNodeSetContent(baseurl_nodes[i], tmp_str);
+updated = 1;
+}
+}
+
+av_free(path);
+return updated;
+
+}
static int parse_manifest_representation(AVFormatContext *s, const char *url,
xmlNodePtr node,
xmlNodePtr adaptionset_node,
@@ -699,6 +774,8 @@ static int parse_manifest_representation(AVFormatContext
*s, const char *url,
baseurl_nodes[2] = adaptionset_baseurl_node;
baseurl_nodes[3] = representation_baseurl_node;
+resolve_content_path(s, url, baseurl_nodes, 4);
+
if (representation_segmenttemplate_node || fragment_template_node) {
fragment_timeline_node = NULL;
fragment_templates_tab[0] = representation_segmenttemplate_node;
@@ -994,6 +1071,9 @@ static int parse_manifest(AVFormatContext *s, const char
*url, AVIOContext *in)
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-17 1:55 GMT+01:00 Colin NG :
> Excluded the fix for byte range issue and update some coding style issues.
> +if (pb) av_freep(pb);
The if() should be unnecessary.
Please avoid adding empty lines in unrelated parts of the file
and while there, try to fix "if (condition) {" to "if (condition) {"
(one space).
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
Excluded the fix for byte range issue and update some coding style issues.
From: Colin NG
Sent: November 15, 2017 10:58 AM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
Have submitted already for the byte range issue in another email with title
"Download dash content with byte range info"
From: ffmpeg-devel on behalf of Carl Eugen
Hoyos
Sent: November 15, 2017 10:54 AM
To: FFmpeg development discussions and patches
Subject: Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-15 3:54 GMT+01:00 Colin NG :
> - Add a function to handle the base URL Processing described in section 5.6.5
> of IEC_23009-1.
>
> - Fix for downloading dash content with byte range info
As said: If these are two separate issues, please send two patches.
Use tools/patcheck to check your patch for many style issues.
(No Changelog entry necessary.)
Try to avoid if ((var1 = var2) > 1), instead add an additional line.
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
ffmpeg-devel Info Page<http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
ffmpeg.org
This list is about FFmpeg development discussions and patches; but not for
bug-reports. Please read the Code-of-conduct. To see the collection of prior
postings to ...
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
index 0e3afd2..4d89374 100644
--- a/libavformat/dashdec.c
+++ b/libavformat/dashdec.c
@@ -148,6 +148,11 @@ static uint64_t get_current_time_in_sec(void)
return av_gettime() / 100;
}
+static char * ishttp(char *url) {
+char *proto_name = avio_find_protocol_name(url);
+return av_strstart(proto_name, "http", NULL);
+}
+
static uint64_t get_utc_date_time_insec(AVFormatContext *s, const char
*datetime)
{
struct tm timeinfo;
@@ -392,7 +397,10 @@ static int open_url(AVFormatContext *s, AVIOContext **pb,
const char *url,
else if (strcmp(proto_name, "file") || !strncmp(url, "file,", 5))
return AVERROR_INVALIDDATA;
-ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
+if (pb) av_freep(pb);
+ // set_httpheader_options(c, &tmp);
+ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback, &tmp);
+
if (ret >= 0) {
// update cookies on http response with setcookies.
char *new_cookies = NULL;
@@ -613,6 +621,77 @@ static int parse_manifest_segmenttimeline(AVFormatContext
*s, struct representat
return 0;
}
+
+static int resolve_content_path(AVFormatContext *s, const char *url,
xmlNodePtr *baseurl_nodes, int n_baseurl_nodes) {
+
+int i;
+char *text;
+char tmp_str[MAX_URL_SIZE];
+char tmp_str_2[MAX_URL_SIZE];
+
+char *path = av_mallocz(MAX_URL_SIZE);
+int nameSize = 0;
+int updated = 0;
+
+av_strlcpy(tmp_str, url, strlen(url)+1);
+char *mpdName = strtok (tmp_str," /");
+
+while ((mpdName =strtok (NULL, "/"))) {
+nameSize = strlen(mpdName);
+}
+
+av_strlcpy (path, url, strlen(url)-nameSize+1 );
+
+int rootId = 0;
+xmlNodePtr *node = NULL;
+char *proto_name = NULL;
+for (rootId = n_baseurl_nodes-1; rootId >0; rootId--) {
+if (!(node = baseurl_nodes[rootId])) continue;
+
+ if (ishttp(xmlNodeGetContent(node))) {
+break;
+}
+}
+
+node = baseurl_nodes[rootId];
+char *baseurl = xmlNodeGetContent(node);
+char *root_url = (!av_strcasecmp(baseurl, ""))? path: baseurl;
+
+if (node) {
+xmlNodeSetContent(node, root_url);
+}
+
+int size = strlen(root_url);
+char *isRootHttp= ishttp(root_url);
+
+char token ='/';
+if (strncmp(&root_url[size-1],&token, 1) != 0) {
+av_strlcat(root_url, "/", size+2);
+size+=2;
+}
+
+for (i = 0; i < n_baseurl_nodes; ++i) {
+if (i==rootId) continue;
+text = xmlNodeGetContent(baseurl_nodes[i]);
+if (text) {
+memset(tmp_str, 0, sizeof(tmp_str));
+
+if (!ishttp(text) && isRootHttp) {
+av_strlcpy(tmp_str, root_url, size+1);
+}
+int start = (strncmp(text, &token, 1) == 0) ? 1: 0;
+memset(tmp_str_2, 0, sizeof(tmp_str_2));
+av_strlcat(tmp_str, text+start, MAX_URL_SIZE);
+xmlFree(text);
+xmlNodeSetContent(baseurl_nodes[i], tmp_str);
+updated = 1;
+}
+}
+
+av_free(path);
+return updated;
+
+}
static int parse_manifest_representation(AVFormatContext *s, const char *url,
xmlNodePtr node,
xmlNodePtr adaptionset
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
Have submitted already for the byte range issue in another email with title "Download dash content with byte range info" From: ffmpeg-devel on behalf of Carl Eugen Hoyos Sent: November 15, 2017 10:54 AM To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault) 2017-11-15 3:54 GMT+01:00 Colin NG : > - Add a function to handle the base URL Processing described in section 5.6.5 > of IEC_23009-1. > > - Fix for downloading dash content with byte range info As said: If these are two separate issues, please send two patches. Use tools/patcheck to check your patch for many style issues. (No Changelog entry necessary.) Try to avoid if ((var1 = var2) > 1), instead add an additional line. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ffmpeg-devel Info Page<http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> ffmpeg.org This list is about FFmpeg development discussions and patches; but not for bug-reports. Please read the Code-of-conduct. To see the collection of prior postings to ... ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-15 3:54 GMT+01:00 Colin NG : > - Add a function to handle the base URL Processing described in section 5.6.5 > of IEC_23009-1. > > - Fix for downloading dash content with byte range info As said: If these are two separate issues, please send two patches. Use tools/patcheck to check your patch for many style issues. (No Changelog entry necessary.) Try to avoid if ((var1 = var2) > 1), instead add an additional line. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
On Thu, Nov 09, 2017 at 00:19:33 +, Colin NG wrote:
Before the next attempt, please do have a look at the docs about
ffmpeg's programming style, especially bracket placement.
> +static int isLocal(char *url) {
> +
> +if (av_strstart(url, "http://";, NULL) || av_strstart(url, "https://";,
> NULL))
Apart from the fact that I'm not sure whether such a function doesn't
already exist:
> +{
> +return FALSE;
> +}
> +
> +return TRUE;
> +}
TRUE/FALSE? Are you sure? Apart from that, a ternary check would be
simpler.
> -ret = s->io_open(s, pb, url, AVIO_FLAG_READ, &tmp);
> +{
> +av_freep(pb);
> +AVDictionary *opts = NULL;
> +set_httpheader_options(c, opts);
> +ret = avio_open2(pb, url, AVIO_FLAG_READ, c->interrupt_callback,
> &opts);
> +av_dict_free(&opts);
> +if (ret < 0)
> +return ret;
> +}
Why a separate block?
> if (ret >= 0) {
And the return above obsoletes this check.
> +char *path = malloc(MAX_URL_SIZE);
ffmpeg has its own malloc variants, and you MUST check the result.
> -for (i = 0; i < n_baseurl_nodes; ++i) {
> +for (i = 0; i < n_baseurl_nodes; i++) {
Why this change? (Yes, the latter is the more correct style, but
doesn't have anything to do with your patch.)
> }
> +
> if (rep_bandwidth_val && tmp_str[0] != '\0') {
Why this change?
> -} else if (!av_strcasecmp(fragmenturl_node->name, (const char
> *)"SegmentURL")) {
> +}
> +else if (!av_strcasecmp(fragmenturl_node->name, (const char
> *)"SegmentURL")) {
Why this change?
> }
> +
> representation_segmenttemplate_node =
> find_child_node_by_name(representation_node, "SegmentTemplate");
Why this change?
> +av_log(s, AV_LOG_INFO, "representation_segmentlist_node \n");
And why whitespace before the line break?
> adaptionset_baseurl_node);
> -if (ret < 0) {
> + if (ret < 0) {
> return ret;
Why this change?
> close_in = 1;
> -
> set_httpheader_options(c, opts);
Why this change?
> +if ((mpd_baseurl_node = find_child_node_by_name(node, "BaseURL")) ==
> NULL)
> +{
> +mpd_baseurl_node = xmlNewNode(node, "BaseURL");
> + }
Watch your bracket style, and your indentation.
... and probably a lot of other issues which I cannot judge.
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
2017-11-09 1:19 GMT+01:00 Colin NG : > - Add a function to handle the base URL Processing described in section 5.6.5 > of IEC_23009-1. > > - Fix for downloading content with byte range selection This sounds as if the patch should be split in two. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
