[FFmpeg-devel] [Patch] Fix for ticket 6658 (Dash demuxer segfault)
Moritz Barsnick
barsnick at gmx.net
Sun Nov 12 23:31:31 EET 2017
On Thu, Nov 09, 2017 at 00:19:33 +0000, 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
More information about the ffmpeg-devel
mailing list