[FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
This addresses ticket 5023From a2a0b9e0da14b6e82aa783535ec1878c8ffbede0 Mon Sep 17 00:00:00 2001 From: Alex Agranovsky Date: Tue, 24 Nov 2015 00:06:14 -0500 Subject: [PATCH 1/2] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header. Fixes ticket 5023 Signed-off-by: Alex Agranovsky --- libavformat/mpjpegdec.c | 176 1 file changed, 133 insertions(+), 43 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 2749a48..e494f1a 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -23,22 +23,16 @@ #include "avformat.h" #include "internal.h" +#include "avio_internal.h" -static int get_line(AVIOContext *pb, char *line, int line_size) -{ -int i = ff_get_line(pb, line, line_size); -if (i > 1 && line[i - 2] == '\r') -line[i - 2] = '\0'; -if (pb->error) -return pb->error; - -if (pb->eof_reached) -return AVERROR_EOF; - -return 0; -} +/** Context for demuxing an MPJPEG stream. */ +typedef struct MPJPEGDemuxContext { +char* boundary; +char* searchstr; +int searchstr_len; +} MPJPEGDemuxContext; static void trim_right(char* p) @@ -46,13 +40,32 @@ static void trim_right(char* p) char *end; if (!p || !*p) return; -end = p + strlen(p) - 1; -while (end != p && av_isspace(*end)) { +int len = strlen(p); +if ( !len ) +return; +end = p + len - 1; +while (end >= p && av_isspace(*end)) { *end = '\0'; end--; } } +static int get_line(AVIOContext *pb, char *line, int line_size) +{ +ff_get_line(pb, line, line_size); + +if (pb->error) +return pb->error; + +if (pb->eof_reached) +return AVERROR_EOF; + +trim_right(line); +return 0; +} + + + static int split_tag_value(char **tag, char **value, char *line) { char *p = line; @@ -86,12 +99,24 @@ static int split_tag_value(char **tag, char **value, char *line) return 0; } -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); +static int parse_multipart_header(AVIOContext *pb, +int* size, +const char* expected_boundary, +void *log_ctx); + +static int mpjpeg_read_close(AVFormatContext *s) +{ +MPJPEGDemuxContext *mpjpeg = s->priv_data; +av_freep(&mpjpeg->boundary); +av_freep(&mpjpeg->searchstr); +return 0; +} static int mpjpeg_read_probe(AVProbeData *p) { AVIOContext *pb; int ret = 0; +int size = 0; if (p->buf_size < 2 || p->buf[0] != '-' || p->buf[1] != '-') return 0; @@ -100,7 +125,7 @@ static int mpjpeg_read_probe(AVProbeData *p) if (!pb) return 0; -ret = (parse_multipart_header(pb, NULL)>0)?AVPROBE_SCORE_MAX:0; +ret = ( parse_multipart_header(pb, &size, "--", NULL) > 0 ) ? AVPROBE_SCORE_MAX : 0; av_free(pb); @@ -110,17 +135,19 @@ static int mpjpeg_read_probe(AVProbeData *p) static int mpjpeg_read_header(AVFormatContext *s) { AVStream *st; -char boundary[70 + 2 + 1]; +char boundary[70 + 2 + 1] = {0}; int64_t pos = avio_tell(s->pb); int ret; +do { +ret = get_line(s->pb, boundary, sizeof(boundary)); +if (ret < 0) +return ret; +} while (!boundary[0]); -ret = get_line(s->pb, boundary, sizeof(boundary)); -if (ret < 0) -return ret; - -if (strncmp(boundary, "--", 2)) +if (strncmp(boundary, "--", 2)) { return AVERROR_INVALIDDATA; +} st = avformat_new_stream(s, NULL); if (!st) @@ -147,28 +174,44 @@ static int parse_content_length(const char *value) return val; } -static int parse_multipart_header(AVIOContext *pb, void *log_ctx) +static int parse_multipart_header(AVIOContext *pb, +int* size, +const char* expected_boundary, +void *log_ctx) { char line[128]; int found_content_type = 0; -int ret, size = -1; +int ret; + +*size = -1; // get the CRLF as empty string ret = get_line(pb, line, sizeof(line)); -if (ret < 0) +if (ret < 0) { return ret; +} /* some implementation do not provide the required * initial CRLF (see rfc1341 7.2.1) */ -if (!line[0]) { +while (!line[0]) { ret = get_line(pb, line, sizeof(line)); -if (ret < 0) +if (ret < 0) { return ret; +} } -if (strncmp(line, "--", 2)) +if ( strncmp(line, expected_boundary, strlen(expected_boundary) ) ) { +if (log_ctx) { +av_log(log_ctx, +AV_LOG_ERROR, +"Expected boundary '%s' not found, instead found a line of %lu bytes\n", +expected_boundary, +strlen(line) ); +} + ret
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Tue, 24 Nov 2015 00:16:06 -0500 Alex Agranovsky wrote: > From a2a0b9e0da14b6e82aa783535ec1878c8ffbede0 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:06:14 -0500 > Subject: [PATCH 1/2] Allow mpjpeg demuxer to process MIME parts which do not > include Content-Length header. > > Fixes ticket 5023 > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 176 > > 1 file changed, 133 insertions(+), 43 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index 2749a48..e494f1a 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -23,22 +23,16 @@ > > #include "avformat.h" > #include "internal.h" > +#include "avio_internal.h" > > -static int get_line(AVIOContext *pb, char *line, int line_size) > -{ > -int i = ff_get_line(pb, line, line_size); > > -if (i > 1 && line[i - 2] == '\r') > -line[i - 2] = '\0'; > > -if (pb->error) > -return pb->error; > - > -if (pb->eof_reached) > -return AVERROR_EOF; > - > -return 0; > -} > +/** Context for demuxing an MPJPEG stream. */ This comment is really not needed. > +typedef struct MPJPEGDemuxContext { > +char* boundary; > +char* searchstr; > +int searchstr_len; > +} MPJPEGDemuxContext; > > > static void trim_right(char* p) > @@ -46,13 +40,32 @@ static void trim_right(char* p) > char *end; > if (!p || !*p) > return; > -end = p + strlen(p) - 1; > -while (end != p && av_isspace(*end)) { > +int len = strlen(p); > +if ( !len ) > +return; > +end = p + len - 1; > +while (end >= p && av_isspace(*end)) { Why this change? Note that strlen(p)>0 is already guaranteed by the "!*p" check before. > *end = '\0'; > end--; > } > } > > +static int get_line(AVIOContext *pb, char *line, int line_size) > +{ > +ff_get_line(pb, line, line_size); > + > +if (pb->error) > +return pb->error; > + > +if (pb->eof_reached) > +return AVERROR_EOF; > + > +trim_right(line); > +return 0; > +} > + > + > + > static int split_tag_value(char **tag, char **value, char *line) > { > char *p = line; > @@ -86,12 +99,24 @@ static int split_tag_value(char **tag, char **value, char > *line) > return 0; > } > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); > +static int parse_multipart_header(AVIOContext *pb, > +int* size, > +const char* expected_boundary, > +void *log_ctx); > + > +static int mpjpeg_read_close(AVFormatContext *s) > +{ > +MPJPEGDemuxContext *mpjpeg = s->priv_data; > +av_freep(&mpjpeg->boundary); > +av_freep(&mpjpeg->searchstr); > +return 0; > +} > > static int mpjpeg_read_probe(AVProbeData *p) > { > AVIOContext *pb; > int ret = 0; > +int size = 0; > > if (p->buf_size < 2 || p->buf[0] != '-' || p->buf[1] != '-') > return 0; > @@ -100,7 +125,7 @@ static int mpjpeg_read_probe(AVProbeData *p) > if (!pb) > return 0; > > -ret = (parse_multipart_header(pb, NULL)>0)?AVPROBE_SCORE_MAX:0; > +ret = ( parse_multipart_header(pb, &size, "--", NULL) > 0 ) ? > AVPROBE_SCORE_MAX : 0; A stray space got in. > > av_free(pb); > > @@ -110,17 +135,19 @@ static int mpjpeg_read_probe(AVProbeData *p) > static int mpjpeg_read_header(AVFormatContext *s) > { > AVStream *st; > -char boundary[70 + 2 + 1]; > +char boundary[70 + 2 + 1] = {0}; > int64_t pos = avio_tell(s->pb); > int ret; > > +do { > +ret = get_line(s->pb, boundary, sizeof(boundary)); > +if (ret < 0) > +return ret; > +} while (!boundary[0]); > > -ret = get_line(s->pb, boundary, sizeof(boundary)); > -if (ret < 0) > -return ret; > - Can you explain why it suddenly has to skip multiple lines? > -if (strncmp(boundary, "--", 2)) > +if (strncmp(boundary, "--", 2)) { > return AVERROR_INVALIDDATA; > +} Another change that looks like a stray change. > > st = avformat_new_stream(s, NULL); > if (!st) > @@ -147,28 +174,44 @@ static int parse_content_length(const char *value) > return val; > } > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx) > +static int parse_multipart_header(AVIOContext *pb, > +int* size, > +const char* expected_boundary, > +void *log_ctx) > { > char line[128]; > int found_content_type = 0; > -int ret, size = -1; > +int ret; > + > +*size = -1; > > // get the CRLF as empty string > ret = get_line(pb, line, sizeof(line)); > -if (ret < 0) > +if (ret < 0) { > return ret; > +} Another stray change? > >
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On November 24, 2015 at 10:32:47 AM, wm4 (nfx...@googlemail.com) wrote: On Tue, 24 Nov 2015 00:16:06 -0500 Alex Agranovsky wrote: > From a2a0b9e0da14b6e82aa783535ec1878c8ffbede0 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:06:14 -0500 > Subject: [PATCH 1/2] Allow mpjpeg demuxer to process MIME parts which do not > include Content-Length header. > > Fixes ticket 5023 > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 176 > 1 file changed, 133 insertions(+), 43 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index 2749a48..e494f1a 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -23,22 +23,16 @@ > > #include "avformat.h" > #include "internal.h" > +#include "avio_internal.h" > > -static int get_line(AVIOContext *pb, char *line, int line_size) > -{ > - int i = ff_get_line(pb, line, line_size); > > - if (i > 1 && line[i - 2] == '\r') > - line[i - 2] = '\0'; > > - if (pb->error) > - return pb->error; > - > - if (pb->eof_reached) > - return AVERROR_EOF; > - > - return 0; > -} > +/** Context for demuxing an MPJPEG stream. */ This comment is really not needed. Will be removed in follow up submission. > +typedef struct MPJPEGDemuxContext { > + char* boundary; > + char* searchstr; > + int searchstr_len; > +} MPJPEGDemuxContext; > > > static void trim_right(char* p) > @@ -46,13 +40,32 @@ static void trim_right(char* p) > char *end; > if (!p || !*p) > return; > - end = p + strlen(p) - 1; > - while (end != p && av_isspace(*end)) { > + int len = strlen(p); > + if ( !len ) > + return; > + end = p + len - 1; > + while (end >= p && av_isspace(*end)) { Why this change? Note that strlen(p)>0 is already guaranteed by the "!*p" check before. Consider input of a single ‘\r’. It will never get trimmed with the old code. > *end = '\0'; > end--; > } > } > > +static int get_line(AVIOContext *pb, char *line, int line_size) > +{ > + ff_get_line(pb, line, line_size); > + > + if (pb->error) > + return pb->error; > + > + if (pb->eof_reached) > + return AVERROR_EOF; > + > + trim_right(line); > + return 0; > +} > + > + > + > static int split_tag_value(char **tag, char **value, char *line) > { > char *p = line; > @@ -86,12 +99,24 @@ static int split_tag_value(char **tag, char **value, char > *line) > return 0; > } > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); > +static int parse_multipart_header(AVIOContext *pb, > + int* size, > + const char* expected_boundary, > + void *log_ctx); > + > +static int mpjpeg_read_close(AVFormatContext *s) > +{ > + MPJPEGDemuxContext *mpjpeg = s->priv_data; > + av_freep(&mpjpeg->boundary); > + av_freep(&mpjpeg->searchstr); > + return 0; > +} > > static int mpjpeg_read_probe(AVProbeData *p) > { > AVIOContext *pb; > int ret = 0; > + int size = 0; > > if (p->buf_size < 2 || p->buf[0] != '-' || p->buf[1] != '-') > return 0; > @@ -100,7 +125,7 @@ static int mpjpeg_read_probe(AVProbeData *p) > if (!pb) > return 0; > > - ret = (parse_multipart_header(pb, NULL)>0)?AVPROBE_SCORE_MAX:0; > + ret = ( parse_multipart_header(pb, &size, "--", NULL) > 0 ) ? > AVPROBE_SCORE_MAX : 0; A stray space got in. Parameters to parse_multipart_header had changed. > > av_free(pb); > > @@ -110,17 +135,19 @@ static int mpjpeg_read_probe(AVProbeData *p) > static int mpjpeg_read_header(AVFormatContext *s) > { > AVStream *st; > - char boundary[70 + 2 + 1]; > + char boundary[70 + 2 + 1] = {0}; > int64_t pos = avio_tell(s->pb); > int ret; > > + do { > + ret = get_line(s->pb, boundary, sizeof(boundary)); > + if (ret < 0) > + return ret; > + } while (!boundary[0]); > > - ret = get_line(s->pb, boundary, sizeof(boundary)); > - if (ret < 0) > - return ret; > - Can you explain why it suddenly has to skip multiple lines? MIME standard, as old as it is, is poorly implemented by many camera manufacturers. In the last few weeks, I have seen things ranging from not sending a proper boundary, to not including a CRLF after a body part, to including multiples. When we process the headers, it is assumed body part had been consumed, and we need to get to the next non-empty lines. It is solving the same problem as 18f9308e6a96bbeb034ee5213a6d41e0b6c2ae74, just the other manifestation of it. > - if (strncmp(boundary, "--", 2)) > + if (strncmp(boundary, "--", 2)) { > return AVERROR_INVALIDDATA; > + } Another change that looks like a stray change. Will remove in follow-up submission. > > st = avformat_new_stream(s, NULL); > if (!st) > @@ -147,28 +174,44 @@ static int parse_content_length(const char *value) > return val; > } > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx) > +static int parse_multipart_header(AVIOContext *pb, > + int* size, > + const char* expected_boundary, > + void *log_ctx) > { > char line[128]; > int found_content_type = 0; > - int ret, size = -1; > + int ret; > +
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
-- Alex Agranovsky Sighthound, Inc www.sighthound.com On November 24, 2015 at 12:36:30 PM, wm4 (nfx...@googlemail.com) wrote: On Tue, 24 Nov 2015 11:39:07 -0500 Alex Agranovsky wrote: > On November 24, 2015 at 10:32:47 AM, wm4 (nfx...@googlemail.com) wrote: > > On Tue, 24 Nov 2015 00:16:06 -0500 > Alex Agranovsky wrote: > > > From a2a0b9e0da14b6e82aa783535ec1878c8ffbede0 Mon Sep 17 00:00:00 2001 > > From: Alex Agranovsky > > Date: Tue, 24 Nov 2015 00:06:14 -0500 > > Subject: [PATCH 1/2] Allow mpjpeg demuxer to process MIME parts which do > > not > > include Content-Length header. > > > > Fixes ticket 5023 > > > > Signed-off-by: Alex Agranovsky > > --- > > libavformat/mpjpegdec.c | 176 > > > > 1 file changed, 133 insertions(+), 43 deletions(-) > > > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > > index 2749a48..e494f1a 100644 > > --- a/libavformat/mpjpegdec.c > > +++ b/libavformat/mpjpegdec.c > > @@ -23,22 +23,16 @@ > > > > #include "avformat.h" > > #include "internal.h" > > +#include "avio_internal.h" > > > > -static int get_line(AVIOContext *pb, char *line, int line_size) > > -{ > > - int i = ff_get_line(pb, line, line_size); > > > > - if (i > 1 && line[i - 2] == '\r') > > - line[i - 2] = '\0'; > > > > - if (pb->error) > > - return pb->error; > > - > > - if (pb->eof_reached) > > - return AVERROR_EOF; > > - > > - return 0; > > -} > > +/** Context for demuxing an MPJPEG stream. */ > > This comment is really not needed. > Will be removed in follow up submission. > > > > > +typedef struct MPJPEGDemuxContext { > > + char* boundary; > > + char* searchstr; > > + int searchstr_len; > > +} MPJPEGDemuxContext; > > > > > > static void trim_right(char* p) > > @@ -46,13 +40,32 @@ static void trim_right(char* p) > > char *end; > > if (!p || !*p) > > return; > > - end = p + strlen(p) - 1; > > - while (end != p && av_isspace(*end)) { > > + int len = strlen(p); > > + if ( !len ) > > + return; > > + end = p + len - 1; > > + while (end >= p && av_isspace(*end)) { > > Why this change? Note that strlen(p)>0 is already guaranteed by the > "!*p" check before. > > > Consider input of a single ‘\r’. It will never get trimmed with the old code. > I don't really see how most of the changes fix this case. The only change that does anything is replacing the != operator with >= . Which is invalid as I see just now: it would set end to p-1, which AFAIK is undefined behavior. Setting end to p-1 would force us to exit the loop, no? We’re just comparing two pointer values, without dereferencing them. I do see that length check is redundant with !*p, so removing that. > > *end = '\0'; > > end--; > > } > > } > > > > +static int get_line(AVIOContext *pb, char *line, int line_size) > > +{ > > + ff_get_line(pb, line, line_size); > > + > > + if (pb->error) > > + return pb->error; > > + > > + if (pb->eof_reached) > > + return AVERROR_EOF; > > + > > + trim_right(line); > > + return 0; > > +} > > + > > + > > + > > static int split_tag_value(char **tag, char **value, char *line) > > { > > char *p = line; > > @@ -86,12 +99,24 @@ static int split_tag_value(char **tag, char **value, > > char *line) > > return 0; > > } > > > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); > > +static int parse_multipart_header(AVIOContext *pb, > > + int* size, > > + const char* expected_boundary, > > + void *log_ctx); > > + > > +static int mpjpeg_read_close(AVFormatContext *s) > > +{ > > + MPJPEGDemuxContext *mpjpeg = s->priv_data; > > + av_freep(&mpjpeg->boundary); > > + av_freep(&mpjpeg->searchstr); > > + return 0; > > +} > > > > static int mpjpeg_read_probe(AVProbeData *p) > > { > > AVIOContext *pb; > > int ret = 0; > > + int size = 0; > > > > if (p->buf_size < 2 || p->buf[0] != '-' || p->buf[1] != '-') > > return 0; > > @@ -100,7 +125,7 @@ static int mpjpeg_read_probe(AVProbeData *p) > > if (!pb) > > return 0; > > > > - ret = (parse_multipart_header(pb, NULL)>0)?AVPROBE_SCORE_MAX:0; > > + ret = ( parse_multipart_header(pb, &size, "--", NULL) > 0 ) ? > > AVPROBE_SCORE_MAX : 0; > > A stray space got in. > Parameters to parse_multipart_header had changed. Yes, but the 2 additional spaces after the ( and before the ) are not consistent with the rest of the coding style. (Though I admit the FFmpeg code has some consistency problems, e.g. the line you changed was missing some spaces.) While these cosmetic issues are not that important, I think at least some effort should be put into not making it look worse. Agreed. It takes effort, though, to switch to a specific project’s style, so I apologize for having to go through that. I’ll try to be more conscious of specific style nuances in the futur
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Tue, Nov 24, 2015 at 03:01:28PM -0500, Alex Agranovsky wrote: [...] > From 2c253d7978a6c9c2dc701d393eb5b9d68e831c98 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:07:34 -0500 > Subject: [PATCH 2/2] If available, use the actual boundary in HTTP response's > Content-Type header, to separate MIME parts in mpjpeg stream > > This code is disabled by default so not to break some fragile endpoints > sending invalid MIME, but can be enabled via AVOption 'strict_mime_boundary' > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 75 > +++-- > 1 file changed, 72 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index b9093ea..b89f128 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -20,6 +20,7 @@ > */ > > #include "libavutil/avstring.h" > +#include "libavutil/opt.h" > > #include "avformat.h" > #include "internal.h" > @@ -28,9 +29,11 @@ > > > typedef struct MPJPEGDemuxContext { > +const AVClass *class; > char* boundary; > char* searchstr; > int searchstr_len; > +int strict_mime_boundary; > } MPJPEGDemuxContext; > > > @@ -245,6 +248,43 @@ static int parse_multipart_header(AVIOContext *pb, > } > > > +static char* mpjpeg_get_boundary(AVIOContext* pb) > +{ > +uint8_t *mime_type = NULL; > +uint8_t *start; > +uint8_t *end; > +uint8_t *res = NULL; > +int len; > + > +/* get MIME type, and skip to the first parameter */ > +av_opt_get(pb, "mime_type", AV_OPT_SEARCH_CHILDREN, &mime_type); > +start = mime_type; > +while (start != NULL && *start != '\0') { > +start = strchr(start, ';'); > +if (start) > +start = start+1; > + > +while (av_isspace(*start)) > +start++; > + > +if (!av_strncasecmp(start, "boundary=", 9)) { > +start += 9; > + > +end = strchr(start, ';'); > +if (end) > +len = end - start - 1; > +else > +len = strlen(start); > +res = av_strndup(start, len); > +break; > +} > +} > + > +av_freep(&mime_type); > +return res; > +} > + > + > static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) > { > int size; > @@ -252,8 +292,18 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > > MPJPEGDemuxContext *mpjpeg = s->priv_data; > if (mpjpeg->boundary == NULL) { > -mpjpeg->boundary = av_strdup("--"); > -mpjpeg->searchstr = av_strdup("\r\n--"); > +uint8_t* boundary = NULL; > +if (mpjpeg->strict_mime_boundary) { > +boundary = mpjpeg_get_boundary(s->pb); > +} > +if (boundary != NULL) { > +mpjpeg->boundary = boundary; > +mpjpeg->searchstr = av_malloc(4+strlen(boundary)+1); > +sprintf( mpjpeg->searchstr, "\r\n%s\r\n", boundary ); please use snprintf() > +} else { > +mpjpeg->boundary = av_strdup("--"); > +mpjpeg->searchstr = av_strdup("\r\n--"); > +} > mpjpeg->searchstr_len = strlen(mpjpeg->searchstr); > if (!mpjpeg->boundary || !mpjpeg->searchstr) { > av_freep(&mpjpeg->boundary); > @@ -315,6 +365,22 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > return ret; > } > > +#define OFFSET(x) offsetof(MPJPEGDemuxContext, x) > + > +#define DEC AV_OPT_FLAG_DECODING_PARAM > +const AVOption mpjpeg_options[] = { > +{ "strict_mime_boundary", "require MIME boundaries match", > OFFSET(strict_mime_boundary), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, DEC }, > +{ NULL } > +}; > + > + > +static const AVClass ff_mpjpeg_demuxer_class = { there should be no ff_ as its static > +.class_name = "MPJPEG demuxer", > +.item_name = av_default_item_name, > +.option = mpjpeg_options, > +.version= LIBAVUTIL_VERSION_INT, > +}; > + > AVInputFormat ff_mpjpeg_demuxer = { > .name = "mpjpeg", > .long_name = NULL_IF_CONFIG_SMALL("MIME multipart JPEG"), > @@ -324,5 +390,8 @@ AVInputFormat ff_mpjpeg_demuxer = { > .read_probe= mpjpeg_read_probe, > .read_header = mpjpeg_read_header, > .read_packet = mpjpeg_read_packet, > -.read_close= mpjpeg_read_close > +.read_close= mpjpeg_read_close, > +.priv_class= &ff_mpjpeg_demuxer_class > }; > + > + > -- > 2.4.9 (Apple Git-60) > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting prob
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On November 24, 2015 at 6:06:36 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Tue, Nov 24, 2015 at 03:01:28PM -0500, Alex Agranovsky wrote: [...] > From 2c253d7978a6c9c2dc701d393eb5b9d68e831c98 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:07:34 -0500 > Subject: [PATCH 2/2] If available, use the actual boundary in HTTP response's > Content-Type header, to separate MIME parts in mpjpeg stream > > This code is disabled by default so not to break some fragile endpoints > sending invalid MIME, but can be enabled via AVOption 'strict_mime_boundary' > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 75 > +++-- > 1 file changed, 72 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index b9093ea..b89f128 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -20,6 +20,7 @@ > */ > > #include "libavutil/avstring.h" > +#include "libavutil/opt.h" > > #include "avformat.h" > #include "internal.h" > @@ -28,9 +29,11 @@ > > > typedef struct MPJPEGDemuxContext { > + const AVClass *class; > char* boundary; > char* searchstr; > int searchstr_len; > + int strict_mime_boundary; > } MPJPEGDemuxContext; > > > @@ -245,6 +248,43 @@ static int parse_multipart_header(AVIOContext *pb, > } > > > +static char* mpjpeg_get_boundary(AVIOContext* pb) > +{ > + uint8_t *mime_type = NULL; > + uint8_t *start; > + uint8_t *end; > + uint8_t *res = NULL; > + int len; > + > + /* get MIME type, and skip to the first parameter */ > + av_opt_get(pb, "mime_type", AV_OPT_SEARCH_CHILDREN, &mime_type); > + start = mime_type; > + while (start != NULL && *start != '\0') { > + start = strchr(start, ';'); > + if (start) > + start = start+1; > + > + while (av_isspace(*start)) > + start++; > + > + if (!av_strncasecmp(start, "boundary=", 9)) { > + start += 9; > + > + end = strchr(start, ';'); > + if (end) > + len = end - start - 1; > + else > + len = strlen(start); > + res = av_strndup(start, len); > + break; > + } > + } > + > + av_freep(&mime_type); > + return res; > +} > + > + > static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) > { > int size; > @@ -252,8 +292,18 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > > MPJPEGDemuxContext *mpjpeg = s->priv_data; > if (mpjpeg->boundary == NULL) { > - mpjpeg->boundary = av_strdup("--"); > - mpjpeg->searchstr = av_strdup("\r\n--"); > + uint8_t* boundary = NULL; > + if (mpjpeg->strict_mime_boundary) { > + boundary = mpjpeg_get_boundary(s->pb); > + } > + if (boundary != NULL) { > + mpjpeg->boundary = boundary; > + mpjpeg->searchstr = av_malloc(4+strlen(boundary)+1); > + sprintf( mpjpeg->searchstr, "\r\n%s\r\n", boundary ); please use snprintf() > + } else { > + mpjpeg->boundary = av_strdup("--"); > + mpjpeg->searchstr = av_strdup("\r\n--"); > + } > mpjpeg->searchstr_len = strlen(mpjpeg->searchstr); > if (!mpjpeg->boundary || !mpjpeg->searchstr) { > av_freep(&mpjpeg->boundary); > @@ -315,6 +365,22 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > return ret; > } > > +#define OFFSET(x) offsetof(MPJPEGDemuxContext, x) > + > +#define DEC AV_OPT_FLAG_DECODING_PARAM > +const AVOption mpjpeg_options[] = { > + { "strict_mime_boundary", "require MIME boundaries match", > OFFSET(strict_mime_boundary), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, DEC }, > + { NULL } > +}; > + > + > +static const AVClass ff_mpjpeg_demuxer_class = { there should be no ff_ as its static > + .class_name = "MPJPEG demuxer", > + .item_name = av_default_item_name, > + .option = mpjpeg_options, > + .version = LIBAVUTIL_VERSION_INT, > +}; > + > AVInputFormat ff_mpjpeg_demuxer = { > .name = "mpjpeg", > .long_name = NULL_IF_CONFIG_SMALL("MIME multipart JPEG"), > @@ -324,5 +390,8 @@ AVInputFormat ff_mpjpeg_demuxer = { > .read_probe = mpjpeg_read_probe, > .read_header = mpjpeg_read_header, > .read_packet = mpjpeg_read_packet, > - .read_close = mpjpeg_read_close > + .read_close = mpjpeg_read_close, > + .priv_class = &ff_mpjpeg_demuxer_class > }; > + > + > -- > 2.4.9 (Apple Git-60) > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Please find the patches attached. First one is unc
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On November 25, 2015 at 10:35:33 AM, Alex Agranovsky (a...@sighthound.com) wrote: On November 24, 2015 at 6:06:36 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Tue, Nov 24, 2015 at 03:01:28PM -0500, Alex Agranovsky wrote: [...] > From 2c253d7978a6c9c2dc701d393eb5b9d68e831c98 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:07:34 -0500 > Subject: [PATCH 2/2] If available, use the actual boundary in HTTP response's > Content-Type header, to separate MIME parts in mpjpeg stream > > This code is disabled by default so not to break some fragile endpoints > sending invalid MIME, but can be enabled via AVOption 'strict_mime_boundary' > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 75 > +++-- > 1 file changed, 72 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index b9093ea..b89f128 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -20,6 +20,7 @@ > */ > > #include "libavutil/avstring.h" > +#include "libavutil/opt.h" > > #include "avformat.h" > #include "internal.h" > @@ -28,9 +29,11 @@ > > > typedef struct MPJPEGDemuxContext { > + const AVClass *class; > char* boundary; > char* searchstr; > int searchstr_len; > + int strict_mime_boundary; > } MPJPEGDemuxContext; > > > @@ -245,6 +248,43 @@ static int parse_multipart_header(AVIOContext *pb, > } > > > +static char* mpjpeg_get_boundary(AVIOContext* pb) > +{ > + uint8_t *mime_type = NULL; > + uint8_t *start; > + uint8_t *end; > + uint8_t *res = NULL; > + int len; > + > + /* get MIME type, and skip to the first parameter */ > + av_opt_get(pb, "mime_type", AV_OPT_SEARCH_CHILDREN, &mime_type); > + start = mime_type; > + while (start != NULL && *start != '\0') { > + start = strchr(start, ';'); > + if (start) > + start = start+1; > + > + while (av_isspace(*start)) > + start++; > + > + if (!av_strncasecmp(start, "boundary=", 9)) { > + start += 9; > + > + end = strchr(start, ';'); > + if (end) > + len = end - start - 1; > + else > + len = strlen(start); > + res = av_strndup(start, len); > + break; > + } > + } > + > + av_freep(&mime_type); > + return res; > +} > + > + > static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) > { > int size; > @@ -252,8 +292,18 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > > MPJPEGDemuxContext *mpjpeg = s->priv_data; > if (mpjpeg->boundary == NULL) { > - mpjpeg->boundary = av_strdup("--"); > - mpjpeg->searchstr = av_strdup("\r\n--"); > + uint8_t* boundary = NULL; > + if (mpjpeg->strict_mime_boundary) { > + boundary = mpjpeg_get_boundary(s->pb); > + } > + if (boundary != NULL) { > + mpjpeg->boundary = boundary; > + mpjpeg->searchstr = av_malloc(4+strlen(boundary)+1); > + sprintf( mpjpeg->searchstr, "\r\n%s\r\n", boundary ); please use snprintf() > + } else { > + mpjpeg->boundary = av_strdup("--"); > + mpjpeg->searchstr = av_strdup("\r\n--"); > + } > mpjpeg->searchstr_len = strlen(mpjpeg->searchstr); > if (!mpjpeg->boundary || !mpjpeg->searchstr) { > av_freep(&mpjpeg->boundary); > @@ -315,6 +365,22 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > return ret; > } > > +#define OFFSET(x) offsetof(MPJPEGDemuxContext, x) > + > +#define DEC AV_OPT_FLAG_DECODING_PARAM > +const AVOption mpjpeg_options[] = { > + { "strict_mime_boundary", "require MIME boundaries match", > OFFSET(strict_mime_boundary), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, DEC }, > + { NULL } > +}; > + > + > +static const AVClass ff_mpjpeg_demuxer_class = { there should be no ff_ as its static > + .class_name = "MPJPEG demuxer", > + .item_name = av_default_item_name, > + .option = mpjpeg_options, > + .version = LIBAVUTIL_VERSION_INT, > +}; > + > AVInputFormat ff_mpjpeg_demuxer = { > .name = "mpjpeg", > .long_name = NULL_IF_CONFIG_SMALL("MIME multipart JPEG"), > @@ -324,5 +390,8 @@ AVInputFormat ff_mpjpeg_demuxer = { > .read_probe = mpjpeg_read_probe, > .read_header = mpjpeg_read_header, > .read_packet = mpjpeg_read_packet, > - .read_close = mpjpeg_read_close > + .read_close = mpjpeg_read_close, > + .priv_class = &ff_mpjpeg_demuxer_class > }; > + > + > -- > 2.4.9 (Apple Git-60) > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Fri, 27 Nov 2015 15:14:54 -0500 Alex Agranovsky wrote: > > Hi - are there any additional corrections I need to address on this set of > patches, or is it good to go at this point? Sorry, forgot about it, will look tomorrow. Also, the quoting in your replies is completely messed up. I can't tell what is text from others and what you have written. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Wed, 25 Nov 2015 10:35:31 -0500 Alex Agranovsky wrote: > From 70a6e1b0f3d47698bf49c3c766d5472646bff71a Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:06:14 -0500 > Subject: [PATCH 1/2] Allow mpjpeg demuxer to process MIME parts which do not > include Content-Length header. Commit messages should start with a prefix (like "avformat/mpjpeg: "), and then maybe up to 60 characters of summary. If more text is needed, it should be part of the commit body (i.e. the part after the subject line). (Same for the second patch.) > > Fixes ticket 5023 > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 164 > +--- > 1 file changed, 126 insertions(+), 38 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index 2749a48..b9093ea 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -23,22 +23,15 @@ > > #include "avformat.h" > #include "internal.h" > +#include "avio_internal.h" > > -static int get_line(AVIOContext *pb, char *line, int line_size) > -{ > -int i = ff_get_line(pb, line, line_size); > > -if (i > 1 && line[i - 2] == '\r') > -line[i - 2] = '\0'; > - > -if (pb->error) > -return pb->error; > - > -if (pb->eof_reached) > -return AVERROR_EOF; > > -return 0; > -} > +typedef struct MPJPEGDemuxContext { > +char* boundary; > +char* searchstr; > +int searchstr_len; > +} MPJPEGDemuxContext; > > > static void trim_right(char* p) > @@ -47,12 +40,28 @@ static void trim_right(char* p) > if (!p || !*p) > return; > end = p + strlen(p) - 1; > -while (end != p && av_isspace(*end)) { > +while (end >= p && av_isspace(*end)) { I still think that this change is theoretically unclean and invalid, because end will point before the memory location at one point. Maybe someone else can give a second opinion whether it's invalid or allowed by the C standard, and whether it's ok to ignore it. (Besides writing a valid trim shouldn't be hard.) > *end = '\0'; > end--; > } > } > > +static int get_line(AVIOContext *pb, char *line, int line_size) > +{ > +ff_get_line(pb, line, line_size); > + > +if (pb->error) > +return pb->error; > + > +if (pb->eof_reached) > +return AVERROR_EOF; > + > +trim_right(line); > +return 0; > +} > + > + > + > static int split_tag_value(char **tag, char **value, char *line) > { > char *p = line; > @@ -86,12 +95,24 @@ static int split_tag_value(char **tag, char **value, char > *line) > return 0; > } > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); > +static int parse_multipart_header(AVIOContext *pb, > +int* size, > +const char* expected_boundary, > +void *log_ctx); > + > +static int mpjpeg_read_close(AVFormatContext *s) > +{ > +MPJPEGDemuxContext *mpjpeg = s->priv_data; > +av_freep(&mpjpeg->boundary); > +av_freep(&mpjpeg->searchstr); > +return 0; > +} > > static int mpjpeg_read_probe(AVProbeData *p) > { > AVIOContext *pb; > int ret = 0; > +int size = 0; > > if (p->buf_size < 2 || p->buf[0] != '-' || p->buf[1] != '-') > return 0; > @@ -100,7 +121,7 @@ static int mpjpeg_read_probe(AVProbeData *p) > if (!pb) > return 0; > > -ret = (parse_multipart_header(pb, NULL)>0)?AVPROBE_SCORE_MAX:0; > +ret = (parse_multipart_header(pb, &size, "--", NULL) > 0) ? > AVPROBE_SCORE_MAX : 0; > > av_free(pb); > > @@ -110,14 +131,15 @@ static int mpjpeg_read_probe(AVProbeData *p) > static int mpjpeg_read_header(AVFormatContext *s) > { > AVStream *st; > -char boundary[70 + 2 + 1]; > +char boundary[70 + 2 + 1] = {0}; > int64_t pos = avio_tell(s->pb); > int ret; > > - > -ret = get_line(s->pb, boundary, sizeof(boundary)); > -if (ret < 0) > -return ret; > +do { > +ret = get_line(s->pb, boundary, sizeof(boundary)); > +if (ret < 0) > +return ret; > +} while (!boundary[0]); > > if (strncmp(boundary, "--", 2)) > return AVERROR_INVALIDDATA; > @@ -147,11 +169,16 @@ static int parse_content_length(const char *value) > return val; > } > > -static int parse_multipart_header(AVIOContext *pb, void *log_ctx) > +static int parse_multipart_header(AVIOContext *pb, > +int* size, > +const char* expected_boundary, > +void *log_ctx) > { > char line[128]; > int found_content_type = 0; > -int ret, size = -1; > +int ret; > + > +*size = -1; > > // get the CRLF as empty string > ret = get_line(pb, line, sizeof(line)); > @@ -161,14 +188,23 @@ static int parse_multipart_header(AVIOContext *pb, voi
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Wed, 25 Nov 2015 10:35:31 -0500 Alex Agranovsky wrote: > From 4797590e11267993c3883e5037619625e1f1dadf Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Tue, 24 Nov 2015 00:07:34 -0500 > Subject: [PATCH 2/2] If available, use the actual boundary in HTTP response's > Content-Type header, to separate MIME parts in mpjpeg stream > > This code is disabled by default so not to regress endpoints sending invalid > MIME, but can be enabled via AVOption 'strict_mime_boundary' > > Signed-off-by: Alex Agranovsky > --- > libavformat/mpjpegdec.c | 76 > +++-- > 1 file changed, 73 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index b9093ea..c0f011d 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -20,6 +20,7 @@ > */ > > #include "libavutil/avstring.h" > +#include "libavutil/opt.h" > > #include "avformat.h" > #include "internal.h" > @@ -28,9 +29,11 @@ > > > typedef struct MPJPEGDemuxContext { > +const AVClass *class; > char* boundary; > char* searchstr; > int searchstr_len; > +int strict_mime_boundary; > } MPJPEGDemuxContext; > > > @@ -245,6 +248,43 @@ static int parse_multipart_header(AVIOContext *pb, > } > > > +static char* mpjpeg_get_boundary(AVIOContext* pb) > +{ > +uint8_t *mime_type = NULL; > +uint8_t *start; > +uint8_t *end; > +uint8_t *res = NULL; > +int len; > + > +/* get MIME type, and skip to the first parameter */ > +av_opt_get(pb, "mime_type", AV_OPT_SEARCH_CHILDREN, &mime_type); > +start = mime_type; > +while (start != NULL && *start != '\0') { > +start = strchr(start, ';'); > +if (start) > +start = start+1; > + > +while (av_isspace(*start)) > +start++; > + > +if (!av_strncasecmp(start, "boundary=", 9)) { > +start += 9; (Probably could use av_strstart() here, but you don't need to change it if you don't want to.) > + > +end = strchr(start, ';'); > +if (end) > +len = end - start - 1; > +else > +len = strlen(start); > +res = av_strndup(start, len); > +break; > +} > +} > + > +av_freep(&mime_type); > +return res; > +} > + > + > static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) > { > int size; > @@ -252,8 +292,19 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > > MPJPEGDemuxContext *mpjpeg = s->priv_data; > if (mpjpeg->boundary == NULL) { > -mpjpeg->boundary = av_strdup("--"); > -mpjpeg->searchstr = av_strdup("\r\n--"); > +uint8_t* boundary = NULL; > +if (mpjpeg->strict_mime_boundary) { > +boundary = mpjpeg_get_boundary(s->pb); > +} > +if (boundary != NULL) { > +size_t bufsize = 4+strlen(boundary)+1; > +mpjpeg->boundary = boundary; > +mpjpeg->searchstr = av_malloc(bufsize); > +snprintf( mpjpeg->searchstr, bufsize, "\r\n%s\r\n", boundary ); av_asprintf() would be much more convenient. > +} else { > +mpjpeg->boundary = av_strdup("--"); > +mpjpeg->searchstr = av_strdup("\r\n--"); > +} > mpjpeg->searchstr_len = strlen(mpjpeg->searchstr); > if (!mpjpeg->boundary || !mpjpeg->searchstr) { > av_freep(&mpjpeg->boundary); > @@ -315,6 +366,22 @@ static int mpjpeg_read_packet(AVFormatContext *s, > AVPacket *pkt) > return ret; > } > > +#define OFFSET(x) offsetof(MPJPEGDemuxContext, x) > + > +#define DEC AV_OPT_FLAG_DECODING_PARAM > +const AVOption mpjpeg_options[] = { > +{ "strict_mime_boundary", "require MIME boundaries match", > OFFSET(strict_mime_boundary), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, DEC }, Not sure, but maybe it'd be good to have a better explanation for this option in doc/demuxers.texi. > +{ NULL } > +}; > + > + > +static const AVClass mpjpeg_demuxer_class = { > +.class_name = "MPJPEG demuxer", > +.item_name = av_default_item_name, > +.option = mpjpeg_options, > +.version= LIBAVUTIL_VERSION_INT, > +}; > + > AVInputFormat ff_mpjpeg_demuxer = { > .name = "mpjpeg", > .long_name = NULL_IF_CONFIG_SMALL("MIME multipart JPEG"), > @@ -324,5 +391,8 @@ AVInputFormat ff_mpjpeg_demuxer = { > .read_probe= mpjpeg_read_probe, > .read_header = mpjpeg_read_header, > .read_packet = mpjpeg_read_packet, > -.read_close= mpjpeg_read_close > +.read_close= mpjpeg_read_close, > +.priv_class= &mpjpeg_demuxer_class > }; > + > + Rest looks good to me. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Sun, Nov 29, 2015 at 1:00 PM, wm4 wrote: > On Wed, 25 Nov 2015 10:35:31 -0500 > Alex Agranovsky wrote: > >> From 70a6e1b0f3d47698bf49c3c766d5472646bff71a Mon Sep 17 00:00:00 2001 >> From: Alex Agranovsky >> Date: Tue, 24 Nov 2015 00:06:14 -0500 >> Subject: [PATCH 1/2] Allow mpjpeg demuxer to process MIME parts which do not >> include Content-Length header. > > Commit messages should start with a prefix (like "avformat/mpjpeg: "), > and then maybe up to 60 characters of summary. If more text is needed, > it should be part of the commit body (i.e. the part after the subject > line). > > (Same for the second patch.) > >> >> Fixes ticket 5023 >> >> Signed-off-by: Alex Agranovsky >> --- >> libavformat/mpjpegdec.c | 164 >> +--- >> 1 file changed, 126 insertions(+), 38 deletions(-) >> >> diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c >> index 2749a48..b9093ea 100644 >> --- a/libavformat/mpjpegdec.c >> +++ b/libavformat/mpjpegdec.c >> @@ -23,22 +23,15 @@ >> >> #include "avformat.h" >> #include "internal.h" >> +#include "avio_internal.h" >> >> -static int get_line(AVIOContext *pb, char *line, int line_size) >> -{ >> -int i = ff_get_line(pb, line, line_size); >> >> -if (i > 1 && line[i - 2] == '\r') >> -line[i - 2] = '\0'; >> - >> -if (pb->error) >> -return pb->error; >> - >> -if (pb->eof_reached) >> -return AVERROR_EOF; >> >> -return 0; >> -} >> +typedef struct MPJPEGDemuxContext { >> +char* boundary; >> +char* searchstr; >> +int searchstr_len; >> +} MPJPEGDemuxContext; >> >> >> static void trim_right(char* p) >> @@ -47,12 +40,28 @@ static void trim_right(char* p) >> if (!p || !*p) >> return; >> end = p + strlen(p) - 1; >> -while (end != p && av_isspace(*end)) { >> +while (end >= p && av_isspace(*end)) { > > I still think that this change is theoretically unclean and invalid, > because end will point before the memory location at one point. Maybe > someone else can give a second opinion whether it's invalid or allowed > by the C standard, and whether it's ok to ignore it. (Besides writing a > valid trim shouldn't be hard.) Don't know what you are referring to here, but dereferencing is clearly invalid. However, in order to allow common loop idioms, pointer arithmetic one element beyond a array (memory) range is valid: https://stackoverflow.com/questions/8133804/negative-array-index-in-c. [...] ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
Le nonidi 9 frimaire, an CCXXIV, Ganesh Ajjanagadde a écrit : > >> end = p + strlen(p) - 1; > Don't know what you are referring to here, but dereferencing is > clearly invalid. However, in order to allow common loop idioms, > pointer arithmetic one element beyond a array (memory) range is valid: Of course. But one element BEFORE the beginning of an array is invalid. In other words p + sizeof(p) is valid (assuming p is char[]), but p - 1 is not. In this instance, if p is a 0-terminated string, p + strlen(p) is always valid, even without the provision you mention, because the 0 is part of the object. But p + strlen(p) - 1 is not, if strlen(p) can be 0. Note that dereferencing or not is not relevant: some architectures have special registers for pointers that would cause traps just loading an invalid pointer (think: p at offset 0 of segment S, p-1 at offset max of segment S-1 -> load segment descriptor). FFmpeg probably does not run on any such architecture, though. But still, this is bad style. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/29/15 1:00 PM, wm4 wrote: On Wed, 25 Nov 2015 10:35:31 -0500 Alex Agranovsky wrote: From 4797590e11267993c3883e5037619625e1f1dadf Mon Sep 17 00:00:00 2001 From: Alex Agranovsky Date: Tue, 24 Nov 2015 00:07:34 -0500 Subject: [PATCH 2/2] If available, use the actual boundary in HTTP response's Content-Type header, to separate MIME parts in mpjpeg stream This code is disabled by default so not to regress endpoints sending invalid MIME, but can be enabled via AVOption 'strict_mime_boundary' Signed-off-by: Alex Agranovsky --- libavformat/mpjpegdec.c | 76 +++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index b9093ea..c0f011d 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -20,6 +20,7 @@ */ #include "libavutil/avstring.h" +#include "libavutil/opt.h" #include "avformat.h" #include "internal.h" @@ -28,9 +29,11 @@ typedef struct MPJPEGDemuxContext { +const AVClass *class; char* boundary; char* searchstr; int searchstr_len; +int strict_mime_boundary; } MPJPEGDemuxContext; @@ -245,6 +248,43 @@ static int parse_multipart_header(AVIOContext *pb, } +static char* mpjpeg_get_boundary(AVIOContext* pb) +{ +uint8_t *mime_type = NULL; +uint8_t *start; +uint8_t *end; +uint8_t *res = NULL; +int len; + +/* get MIME type, and skip to the first parameter */ +av_opt_get(pb, "mime_type", AV_OPT_SEARCH_CHILDREN, &mime_type); +start = mime_type; +while (start != NULL && *start != '\0') { +start = strchr(start, ';'); +if (start) +start = start+1; + +while (av_isspace(*start)) +start++; + +if (!av_strncasecmp(start, "boundary=", 9)) { +start += 9; (Probably could use av_strstart() here, but you don't need to change it if you don't want to.) + +end = strchr(start, ';'); +if (end) +len = end - start - 1; +else +len = strlen(start); +res = av_strndup(start, len); +break; +} +} + +av_freep(&mime_type); +return res; +} + + static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) { int size; @@ -252,8 +292,19 @@ static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) MPJPEGDemuxContext *mpjpeg = s->priv_data; if (mpjpeg->boundary == NULL) { -mpjpeg->boundary = av_strdup("--"); -mpjpeg->searchstr = av_strdup("\r\n--"); +uint8_t* boundary = NULL; +if (mpjpeg->strict_mime_boundary) { +boundary = mpjpeg_get_boundary(s->pb); +} +if (boundary != NULL) { +size_t bufsize = 4+strlen(boundary)+1; +mpjpeg->boundary = boundary; +mpjpeg->searchstr = av_malloc(bufsize); +snprintf( mpjpeg->searchstr, bufsize, "\r\n%s\r\n", boundary ); av_asprintf() would be much more convenient. +} else { +mpjpeg->boundary = av_strdup("--"); +mpjpeg->searchstr = av_strdup("\r\n--"); +} mpjpeg->searchstr_len = strlen(mpjpeg->searchstr); if (!mpjpeg->boundary || !mpjpeg->searchstr) { av_freep(&mpjpeg->boundary); @@ -315,6 +366,22 @@ static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt) return ret; } +#define OFFSET(x) offsetof(MPJPEGDemuxContext, x) + +#define DEC AV_OPT_FLAG_DECODING_PARAM +const AVOption mpjpeg_options[] = { +{ "strict_mime_boundary", "require MIME boundaries match", OFFSET(strict_mime_boundary), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 1, DEC }, Not sure, but maybe it'd be good to have a better explanation for this option in doc/demuxers.texi. +{ NULL } +}; + + +static const AVClass mpjpeg_demuxer_class = { +.class_name = "MPJPEG demuxer", +.item_name = av_default_item_name, +.option = mpjpeg_options, +.version= LIBAVUTIL_VERSION_INT, +}; + AVInputFormat ff_mpjpeg_demuxer = { .name = "mpjpeg", .long_name = NULL_IF_CONFIG_SMALL("MIME multipart JPEG"), @@ -324,5 +391,8 @@ AVInputFormat ff_mpjpeg_demuxer = { .read_probe= mpjpeg_read_probe, .read_header = mpjpeg_read_header, .read_packet = mpjpeg_read_packet, -.read_close= mpjpeg_read_close +.read_close= mpjpeg_read_close, +.priv_class= &mpjpeg_demuxer_class }; + + Rest looks good to me. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Please see the updated patches attached. The trimming loop that was subject of the discussion had been rewritten to use indices rather than pointer arithmetics. Best, - Alex From 7d9fdaee79f1cd671aa62799b
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/29/15 1:16 PM, Nicolas George wrote: Le nonidi 9 frimaire, an CCXXIV, Ganesh Ajjanagadde a écrit : end = p + strlen(p) - 1; Don't know what you are referring to here, but dereferencing is clearly invalid. However, in order to allow common loop idioms, pointer arithmetic one element beyond a array (memory) range is valid: Of course. But one element BEFORE the beginning of an array is invalid. In other words p + sizeof(p) is valid (assuming p is char[]), but p - 1 is not. In this instance, if p is a 0-terminated string, p + strlen(p) is always valid, even without the provision you mention, because the 0 is part of the object. But p + strlen(p) - 1 is not, if strlen(p) can be 0. Note that dereferencing or not is not relevant: some architectures have special registers for pointers that would cause traps just loading an invalid pointer (think: p at offset 0 of segment S, p-1 at offset max of segment S-1 -> load segment descriptor). FFmpeg probably does not run on any such architecture, though. But still, this is bad style. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Ended up reworking these few lines to operate on indices, rather than doing pointer airthmetics. Should be the same thing, functionally. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
Le nonidi 9 frimaire, an CCXXIV, Alexander Agranovsky a écrit : > Please see the updated patches attached. The trimming loop that was subject > of the discussion had been rewritten to use indices rather than pointer > arithmetics. This kind of drastic change was not necessary, you can do the same with pointers. IMHO, the best way of dealing with that situation is this: when dealing with the end of the string, have the pointer point AFTER the end of the string, i.e.: char *p = string + strlen(string); // not -1 if (p > string && av_isspace(p[-1])) *(--p) = 0; > +char* boundary; Here and in all new code, please use "char *var" instead of "char* var" for consistency. There is a good reason for that: "char* a, b" means that a is a pointer and b is not, grouping the pointer mark with the type is misleading. > +"Expected boundary '%s' not found, instead found a line of > %lu bytes\n", > +expected_boundary, > +strlen(line)); "%lu" is not correct for size_t. The correct type would be %zu, but it is possible that we have to use another construct to avoid bugs from microsoft libraries, see other instances in the code for examples. > -size = parse_content_length(value); > -if (size < 0) > -return size; > +*size = parse_content_length(value); Did you remove the check on purpose? > +if (!av_strncasecmp(start, "boundary=", 9)) { > +start += 9; It has already be pointed out: av_stristart() to avoid the duplicated magic number. Can not comment on the functional aspect, sorry. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Mon, 30 Nov 2015 08:41:42 +0100 Nicolas George wrote: > > +"Expected boundary '%s' not found, instead found a line of > > %lu bytes\n", > > +expected_boundary, > > +strlen(line)); > > "%lu" is not correct for size_t. The correct type would be %zu, but it is > possible that we have to use another construct to avoid bugs from microsoft > libraries, see other instances in the code for examples. It's used in some portable code (e.g. http.c), so it's probably fine. > > -size = parse_content_length(value); > > -if (size < 0) > > -return size; > > +*size = parse_content_length(value); > > Did you remove the check on purpose? He probably did, but now that I look at this again, it seems a bit shady. This code parses the Content-length header, and if parsing it fails, it will now try the boundary instead. But doesn't this header always require a numeric value if it's present at all? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/30/15 2:41 AM, Nicolas George wrote: Le nonidi 9 frimaire, an CCXXIV, Alexander Agranovsky a écrit : Please see the updated patches attached. The trimming loop that was subject of the discussion had been rewritten to use indices rather than pointer arithmetics. This kind of drastic change was not necessary, you can do the same with pointers. IMHO, the best way of dealing with that situation is this: when dealing with the end of the string, have the pointer point AFTER the end of the string, i.e.: char *p = string + strlen(string); // not -1 if (p > string && av_isspace(p[-1])) *(--p) = 0; That works too. +char* boundary; Here and in all new code, please use "char *var" instead of "char* var" for consistency. There is a good reason for that: "char* a, b" means that a is a pointer and b is not, grouping the pointer mark with the type is misleading. Without getting into a religious debate, not my favorite part of the style ;) Will change the code to match the style of existing, though. +"Expected boundary '%s' not found, instead found a line of %lu bytes\n", +expected_boundary, +strlen(line)); "%lu" is not correct for size_t. The correct type would be %zu, but it is possible that we have to use another construct to avoid bugs from microsoft libraries, see other instances in the code for examples. As pointed later in the thread, lu is used elsewhere. And yes, MS makes it interesting in this case. -size = parse_content_length(value); -if (size < 0) -return size; +*size = parse_content_length(value); Did you remove the check on purpose? Yes. Invalid Content-Length, just as no Content-Length at all, should not prevent us from processing the part. +if (!av_strncasecmp(start, "boundary=", 9)) { +start += 9; It has already be pointed out: av_stristart() to avoid the duplicated magic number. I've pondered the change, but with if (!av_stristart(start, "boundary=")) { start += 9; '9' seems a bit random, and with if (!av_stristart(start, "boundary=")) { start += strlen("boundary="); we end up with unnecessarily taking strlen at least twice. Again, not critical, but IMHO as written it is the best compromise between readability and efficiency. Thanks for your comments! Can not comment on the functional aspect, sorry. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Mon, 30 Nov 2015 07:27:14 -0500 Alexander Agranovsky wrote: > On 11/30/15 2:41 AM, Nicolas George wrote: > > Le nonidi 9 frimaire, an CCXXIV, Alexander Agranovsky a écrit : > >> Please see the updated patches attached. The trimming loop that was subject > >> of the discussion had been rewritten to use indices rather than pointer > >> arithmetics. > > This kind of drastic change was not necessary, you can do the same with > > pointers. IMHO, the best way of dealing with that situation is this: when > > dealing with the end of the string, have the pointer point AFTER the end of > > the string, i.e.: > > > > char *p = string + strlen(string); // not -1 > > if (p > string && av_isspace(p[-1])) > > *(--p) = 0; > That works too. > > > > >> +char* boundary; > > Here and in all new code, please use "char *var" instead of "char* var" for > > consistency. There is a good reason for that: "char* a, b" means that a is a > > pointer and b is not, grouping the pointer mark with the type is > > misleading. > Without getting into a religious debate, not my favorite part of the > style ;) > Will change the code to match the style of existing, though. > > >> +"Expected boundary '%s' not found, instead found a line > >> of %lu bytes\n", > >> +expected_boundary, > >> +strlen(line)); > > "%lu" is not correct for size_t. The correct type would be %zu, but it is > > possible that we have to use another construct to avoid bugs from microsoft > > libraries, see other instances in the code for examples. > > As pointed later in the thread, lu is used elsewhere. And yes, MS makes > it interesting in this case. No, I meant %zu. lu really isn't safe. It will fail on 64 bit Windows, for one. (But what Nicolas meant was that %zu could fail on Windows because Windows is stuck in the past, but we work that around if necessary - I think.) > >> -size = parse_content_length(value); > >> -if (size < 0) > >> -return size; > >> +*size = parse_content_length(value); > > Did you remove the check on purpose? > > Yes. Invalid Content-Length, just as no Content-Length at all, should > not prevent us from processing the part. Probably not a good idea to ignore when the server sends us definitely broken data. I'd prefer failure or at least an error message. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/30/15 6:37 AM, wm4 wrote: On Mon, 30 Nov 2015 08:41:42 +0100 Nicolas George wrote: +"Expected boundary '%s' not found, instead found a line of %lu bytes\n", +expected_boundary, +strlen(line)); "%lu" is not correct for size_t. The correct type would be %zu, but it is possible that we have to use another construct to avoid bugs from microsoft libraries, see other instances in the code for examples. It's used in some portable code (e.g. http.c), so it's probably fine. -size = parse_content_length(value); -if (size < 0) -return size; +*size = parse_content_length(value); Did you remove the check on purpose? He probably did, but now that I look at this again, it seems a bit shady. This code parses the Content-length header, and if parsing it fails, it will now try the boundary instead. But doesn't this header always require a numeric value if it's present at all? It surely does. However, seeing creative ways in which existing endpoints choose to break MIME, I'd rather not introduce a restriction where we don't have to. Take the boundary for example -- the code before my changes (and after as well, at least by default) only looks for "CRLF--". I would've found it unfathomable and unacceptable, if I didn't recently see a trace from a popular consumer camera, where one boundary was declared, and another was used. I'm a huge proponent of standards *compliance*, but when standards *enforcement* will result in a regression report, even if the other side is at fault, choosing to be a bit more flexible may be the way to go. In this particular case I don't see a downside, do you? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/30/15 7:34 AM, wm4 wrote: On Mon, 30 Nov 2015 07:27:14 -0500 Alexander Agranovsky wrote: On 11/30/15 2:41 AM, Nicolas George wrote: Le nonidi 9 frimaire, an CCXXIV, Alexander Agranovsky a écrit : Please see the updated patches attached. The trimming loop that was subject of the discussion had been rewritten to use indices rather than pointer arithmetics. This kind of drastic change was not necessary, you can do the same with pointers. IMHO, the best way of dealing with that situation is this: when dealing with the end of the string, have the pointer point AFTER the end of the string, i.e.: char *p = string + strlen(string); // not -1 if (p > string && av_isspace(p[-1])) *(--p) = 0; That works too. +char* boundary; Here and in all new code, please use "char *var" instead of "char* var" for consistency. There is a good reason for that: "char* a, b" means that a is a pointer and b is not, grouping the pointer mark with the type is misleading. Without getting into a religious debate, not my favorite part of the style ;) Will change the code to match the style of existing, though. +"Expected boundary '%s' not found, instead found a line of %lu bytes\n", +expected_boundary, +strlen(line)); "%lu" is not correct for size_t. The correct type would be %zu, but it is possible that we have to use another construct to avoid bugs from microsoft libraries, see other instances in the code for examples. As pointed later in the thread, lu is used elsewhere. And yes, MS makes it interesting in this case. No, I meant %zu. lu really isn't safe. It will fail on 64 bit Windows, for one. (But what Nicolas meant was that %zu could fail on Windows because Windows is stuck in the past, but we work that around if necessary - I think.) -size = parse_content_length(value); -if (size < 0) -return size; +*size = parse_content_length(value); Did you remove the check on purpose? Yes. Invalid Content-Length, just as no Content-Length at all, should not prevent us from processing the part. Probably not a good idea to ignore when the server sends us definitely broken data. I'd prefer failure or at least an error message. I'll add a warning, how about that? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/30/15 7:34 AM, wm4 wrote: On Mon, 30 Nov 2015 07:27:14 -0500 Alexander Agranovsky wrote: On 11/30/15 2:41 AM, Nicolas George wrote: Le nonidi 9 frimaire, an CCXXIV, Alexander Agranovsky a écrit : Please see the updated patches attached. The trimming loop that was subject of the discussion had been rewritten to use indices rather than pointer arithmetics. This kind of drastic change was not necessary, you can do the same with pointers. IMHO, the best way of dealing with that situation is this: when dealing with the end of the string, have the pointer point AFTER the end of the string, i.e.: char *p = string + strlen(string); // not -1 if (p > string && av_isspace(p[-1])) *(--p) = 0; That works too. +char* boundary; Here and in all new code, please use "char *var" instead of "char* var" for consistency. There is a good reason for that: "char* a, b" means that a is a pointer and b is not, grouping the pointer mark with the type is misleading. Without getting into a religious debate, not my favorite part of the style ;) Will change the code to match the style of existing, though. +"Expected boundary '%s' not found, instead found a line of %lu bytes\n", +expected_boundary, +strlen(line)); "%lu" is not correct for size_t. The correct type would be %zu, but it is possible that we have to use another construct to avoid bugs from microsoft libraries, see other instances in the code for examples. As pointed later in the thread, lu is used elsewhere. And yes, MS makes it interesting in this case. No, I meant %zu. lu really isn't safe. It will fail on 64 bit Windows, for one. (But what Nicolas meant was that %zu could fail on Windows because Windows is stuck in the past, but we work that around if necessary - I think.) -size = parse_content_length(value); -if (size < 0) -return size; +*size = parse_content_length(value); Did you remove the check on purpose? Yes. Invalid Content-Length, just as no Content-Length at all, should not prevent us from processing the part. Probably not a good idea to ignore when the server sends us definitely broken data. I'd prefer failure or at least an error message. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel And one more round ... From 3edc50fc36e6abffaa7ae39d1047537cd17aad62 Mon Sep 17 00:00:00 2001 From: Alex Agranovsky Date: Sun, 29 Nov 2015 18:36:20 -0500 Subject: [PATCH 1/2] avformat/mpjpeg: allow processing of MIME parts without Content-Length header Fixes ticket 5023 Signed-off-by: Alex Agranovsky --- libavformat/mpjpegdec.c | 168 +++- 1 file changed, 125 insertions(+), 43 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 2749a48..9d5700a 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -23,13 +23,30 @@ #include "avformat.h" #include "internal.h" +#include "avio_internal.h" -static int get_line(AVIOContext *pb, char *line, int line_size) + + +typedef struct MPJPEGDemuxContext { +char *boundary; +char *searchstr; +int searchstr_len; +} MPJPEGDemuxContext; + + +static void trim_right(char *p) { -int i = ff_get_line(pb, line, line_size); +if (!p || !*p) +return; + +char *end = p + strlen(p); +while (end > p && av_isspace(*(end-1))) +*(--end) = '\0'; +} -if (i > 1 && line[i - 2] == '\r') -line[i - 2] = '\0'; +static int get_line(AVIOContext *pb, char *line, int line_size) +{ +ff_get_line(pb, line, line_size); if (pb->error) return pb->error; @@ -37,21 +54,11 @@ static int get_line(AVIOContext *pb, char *line, int line_size) if (pb->eof_reached) return AVERROR_EOF; +trim_right(line); return 0; } -static void trim_right(char* p) -{ -char *end; -if (!p || !*p) -return; -end = p + strlen(p) - 1; -while (end != p && av_isspace(*end)) { -*end = '\0'; -end--; -} -} static int split_tag_value(char **tag, char **value, char *line) { @@ -86,12 +93,24 @@ static int split_tag_value(char **tag, char **value, char *line) return 0; } -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); +static int parse_multipart_header(AVIOContext *pb, +int* size, +const char* expected_boundary, +void *log_ctx); + +static int mpjpeg_read_close(AVFormatContext *s) +{ +MPJPEGDemuxContext *mpjpeg = s->priv_data; +av_freep(&mpjpeg->boundary); +av_freep(&mpjpeg->searchstr); +return 0; +} static int mpjpeg_read_probe(AVProbeData *p) { AVIOContext *pb; int ret = 0; +int size = 0; if (p->buf_si
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
Le decadi 10 frimaire, an CCXXIV, Alexander Agranovsky a écrit : > Without getting into a religious debate This is the reason I gave practical argument. > As pointed later in the thread, lu is used elsewhere. And yes, MS makes it > interesting in this case. wm4 explained that point. Really, long and short should only ever be used by libc headers to implement intXX_t. > I've pondered the change, but with > if (!av_stristart(start, "boundary=")) { > start += 9; > '9' seems a bit random, and with This is the reason for the third argument of av_stristart(): if (av_stristart(start, "boundary=", &start)) { // no need to add 9, av_stristart() does it } Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/30/15 10:24 AM, Nicolas George wrote: Le decadi 10 frimaire, an CCXXIV, Alexander Agranovsky a écrit : Without getting into a religious debate This is the reason I gave practical argument. As pointed later in the thread, lu is used elsewhere. And yes, MS makes it interesting in this case. wm4 explained that point. Really, long and short should only ever be used by libc headers to implement intXX_t. I've pondered the change, but with if (!av_stristart(start, "boundary=")) { start += 9; '9' seems a bit random, and with This is the reason for the third argument of av_stristart(): if (av_stristart(start, "boundary=", &start)) { // no need to add 9, av_stristart() does it } Ah, missed that. Thanks -- attaching new iteration with this change. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel From 3edc50fc36e6abffaa7ae39d1047537cd17aad62 Mon Sep 17 00:00:00 2001 From: Alex Agranovsky Date: Sun, 29 Nov 2015 18:36:20 -0500 Subject: [PATCH 1/2] avformat/mpjpeg: allow processing of MIME parts without Content-Length header Fixes ticket 5023 Signed-off-by: Alex Agranovsky --- libavformat/mpjpegdec.c | 168 +++- 1 file changed, 125 insertions(+), 43 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 2749a48..9d5700a 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -23,13 +23,30 @@ #include "avformat.h" #include "internal.h" +#include "avio_internal.h" -static int get_line(AVIOContext *pb, char *line, int line_size) + + +typedef struct MPJPEGDemuxContext { +char *boundary; +char *searchstr; +int searchstr_len; +} MPJPEGDemuxContext; + + +static void trim_right(char *p) { -int i = ff_get_line(pb, line, line_size); +if (!p || !*p) +return; + +char *end = p + strlen(p); +while (end > p && av_isspace(*(end-1))) +*(--end) = '\0'; +} -if (i > 1 && line[i - 2] == '\r') -line[i - 2] = '\0'; +static int get_line(AVIOContext *pb, char *line, int line_size) +{ +ff_get_line(pb, line, line_size); if (pb->error) return pb->error; @@ -37,21 +54,11 @@ static int get_line(AVIOContext *pb, char *line, int line_size) if (pb->eof_reached) return AVERROR_EOF; +trim_right(line); return 0; } -static void trim_right(char* p) -{ -char *end; -if (!p || !*p) -return; -end = p + strlen(p) - 1; -while (end != p && av_isspace(*end)) { -*end = '\0'; -end--; -} -} static int split_tag_value(char **tag, char **value, char *line) { @@ -86,12 +93,24 @@ static int split_tag_value(char **tag, char **value, char *line) return 0; } -static int parse_multipart_header(AVIOContext *pb, void *log_ctx); +static int parse_multipart_header(AVIOContext *pb, +int* size, +const char* expected_boundary, +void *log_ctx); + +static int mpjpeg_read_close(AVFormatContext *s) +{ +MPJPEGDemuxContext *mpjpeg = s->priv_data; +av_freep(&mpjpeg->boundary); +av_freep(&mpjpeg->searchstr); +return 0; +} static int mpjpeg_read_probe(AVProbeData *p) { AVIOContext *pb; int ret = 0; +int size = 0; if (p->buf_size < 2 || p->buf[0] != '-' || p->buf[1] != '-') return 0; @@ -100,7 +119,7 @@ static int mpjpeg_read_probe(AVProbeData *p) if (!pb) return 0; -ret = (parse_multipart_header(pb, NULL)>0)?AVPROBE_SCORE_MAX:0; +ret = (parse_multipart_header(pb, &size, "--", NULL) > 0) ? AVPROBE_SCORE_MAX : 0; av_free(pb); @@ -110,14 +129,15 @@ static int mpjpeg_read_probe(AVProbeData *p) static int mpjpeg_read_header(AVFormatContext *s) { AVStream *st; -char boundary[70 + 2 + 1]; +char boundary[70 + 2 + 1] = {0}; int64_t pos = avio_tell(s->pb); int ret; - -ret = get_line(s->pb, boundary, sizeof(boundary)); -if (ret < 0) -return ret; +do { +ret = get_line(s->pb, boundary, sizeof(boundary)); +if (ret < 0) +return ret; +} while (!boundary[0]); if (strncmp(boundary, "--", 2)) return AVERROR_INVALIDDATA; @@ -147,11 +167,16 @@ static int parse_content_length(const char *value) return val; } -static int parse_multipart_header(AVIOContext *pb, void *log_ctx) +static int parse_multipart_header(AVIOContext *pb, +int* size, +const char* expected_boundary, +void *log_ctx) { char line[128]; int found_content_type = 0; -int ret, size = -1; +int ret; + +*size = -1; // get the CRLF as empty string ret = get_line(pb, line, sizeof(line)); @@ -161,14 +186,21 @
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On 11/30/15 10:35 AM, Alexander Agranovsky wrote: On 11/30/15 10:24 AM, Nicolas George wrote: Le decadi 10 frimaire, an CCXXIV, Alexander Agranovsky a écrit : Without getting into a religious debate This is the reason I gave practical argument. As pointed later in the thread, lu is used elsewhere. And yes, MS makes it interesting in this case. wm4 explained that point. Really, long and short should only ever be used by libc headers to implement intXX_t. I've pondered the change, but with if (!av_stristart(start, "boundary=")) { start += 9; '9' seems a bit random, and with This is the reason for the third argument of av_stristart(): if (av_stristart(start, "boundary=", &start)) { // no need to add 9, av_stristart() does it } Ah, missed that. Thanks -- attaching new iteration with this change. Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel Hi guys -- where do we stand with this? Are there any additional comments? - A. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Wed, 2 Dec 2015 09:28:40 -0500 Alexander Agranovsky wrote: > Hi guys -- where do we stand with this? Are there any additional comments? > > - A. Looks fine now. The trim_right() still looks a bit awkward IMHO, but it should be correct (whoever really cares can send a follow-up patch to change it). I'll apply this tonight, I guess. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
On Wed, 2 Dec 2015 09:28:40 -0500 Alexander Agranovsky wrote: > Hi guys -- where do we stand with this? Are there any additional comments? > > - A. Pushed both patches. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel