[FFmpeg-devel] [PATCH] avfilter/drawtext: allow to format pts with strftime
-- Alex Agranovsky 0001-avfilter-drawtext-allow-to-format-pts-with-strftime.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/drawtext: allow to format pts with strftime
Thanks for your comments. I’m attaching the amended patch, hopefully it addresses all of them. Please let me know if something else is out of order. - Alex On October 9, 2015 at 2:23:42 AM, Nicolas George (geo...@nsup.org) wrote: Le septidi 17 vendémiaire, an CCXXIV, Alex Agranovsky a écrit : > From a51f49007381701df18309c73083749413df3cb8 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Thu, 8 Oct 2015 15:54:59 -0400 > Subject: [PATCH] avfilter/drawtext: allow to format pts with strftime > > --- > libavfilter/vf_drawtext.c | 5 + > 1 file changed, 5 insertions(+) Thanks for the patch. Unfortunately, there are a few issues to be fixed. First: please update the docs too. Next: see below. > > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c > index 9fd9461..5c4a7fa 100644 > --- a/libavfilter/vf_drawtext.c > +++ b/libavfilter/vf_drawtext.c > @@ -824,6 +824,12 @@ static int func_pts(AVFilterContext *ctx, AVBPrint *bp, > (int)(ms / 1000) % 60, > (int)ms % 1000); > } > + } else if (!strcmp(fmt, "strftime")) { > + struct tm ltime; There is a tab here. > + int64_t ms = (int64_t)pts; This is not the correct type: since it is passed as a pointer, it must be time_t, nothing else. > + const char *fmt = (argc >= 3) ? argv[2] : "%Y-%m-%d %H:%M:%S"; Did you test this? As far as I know, at this point it is not possible to have argc >= 3. > + localtime_r(&ms, <ime); Are you sure about that? pts is only occasionally a wall-clock timestamp. And if it is, the default formats leaves a timestamp without a time zone, that is Evil. And maybe the user wants UTC. > + av_bprint_strftime(bp, fmt, <ime); > } else { > av_log(ctx, AV_LOG_ERROR, "Invalid format '%s'\n", fmt); > return AVERROR(EINVAL); Regards, -- Nicolas George ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 0001-avfilter-drawtext-allow-to-format-pts-with-strftime.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avfilter/drawtext: allow to format pts with strftime
Hi — are there still outstanding issues with the patch, or is it good to go? Thanks, - Alex On October 9, 2015 at 8:57:22 AM, Alex Agranovsky (a...@sighthound.com) wrote: Thanks for your comments. I’m attaching the amended patch, hopefully it addresses all of them. Please let me know if something else is out of order. - Alex On October 9, 2015 at 2:23:42 AM, Nicolas George (geo...@nsup.org) wrote: Le septidi 17 vendémiaire, an CCXXIV, Alex Agranovsky a écrit : > From a51f49007381701df18309c73083749413df3cb8 Mon Sep 17 00:00:00 2001 > From: Alex Agranovsky > Date: Thu, 8 Oct 2015 15:54:59 -0400 > Subject: [PATCH] avfilter/drawtext: allow to format pts with strftime > > --- > libavfilter/vf_drawtext.c | 5 + > 1 file changed, 5 insertions(+) Thanks for the patch. Unfortunately, there are a few issues to be fixed. First: please update the docs too. Next: see below. > > diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c > index 9fd9461..5c4a7fa 100644 > --- a/libavfilter/vf_drawtext.c > +++ b/libavfilter/vf_drawtext.c > @@ -824,6 +824,12 @@ static int func_pts(AVFilterContext *ctx, AVBPrint *bp, > (int)(ms / 1000) % 60, > (int)ms % 1000); > } > + } else if (!strcmp(fmt, "strftime")) { > + struct tm ltime; There is a tab here. > + int64_t ms = (int64_t)pts; This is not the correct type: since it is passed as a pointer, it must be time_t, nothing else. > + const char *fmt = (argc >= 3) ? argv[2] : "%Y-%m-%d %H:%M:%S"; Did you test this? As far as I know, at this point it is not possible to have argc >= 3. > + localtime_r(&ms, <ime); Are you sure about that? pts is only occasionally a wall-clock timestamp. And if it is, the default formats leaves a timestamp without a time zone, that is Evil. And maybe the user wants UTC. > + av_bprint_strftime(bp, fmt, <ime); > } else { > av_log(ctx, AV_LOG_ERROR, "Invalid format '%s'\n", fmt); > return AVERROR(EINVAL); Regards, -- Nicolas George ___ 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] avfilter/drawtext: allow to format pts with strftime
On October 10, 2015 at 1:10:25 PM, Nicolas George (geo...@nsup.org) wrote: Thanks, I was about to apply, after fixing myself very minor details (trailing spaces, >80 chars lines in the doc and H:M:S -> HH:MM::SS), but I noticed that your patch is missing the "Signed-Off" tag; we have not been very careful with it, but for legal reasons it would be better to have it, and I can not add it myself. Please see the modified patch attached.From 0d759b996718de1987741906480534d2e0f739ff Mon Sep 17 00:00:00 2001 From: Alex Agranovsky Date: Sat, 10 Oct 2015 14:52:34 -0400 Subject: [PATCH] avfilter/drawtext: allow to format pts with strftime Signed-off-by: Alex Agranovsky --- doc/filters.texi | 8 +++- libavfilter/vf_drawtext.c | 12 +++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index 9ab7d43..bd90ac8 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -5398,14 +5398,20 @@ A 1 character description of the current picture type. @item pts The timestamp of the current frame. -It can take up to two arguments. +It can take up to three arguments. The first argument is the format of the timestamp; it defaults to @code{flt} for seconds as a decimal number with microsecond accuracy; @code{hms} stands for a formatted @var{[-]HH:MM:SS.mmm} timestamp with millisecond accuracy. +@code{gmtime} stands for the timestamp of the frame formatted as UTC time; +@code{localtime} stands for the timestamp of the frame formatted as +local time zone time. The second argument is an offset added to the timestamp. +If the format is set to @code{localtime} or @code{gmtime}, +a third argument may be supplied: a strftime() format string. +By default, @var{-MM-DD HH:MM:SS} format will be used. @end table @subsection Examples diff --git a/libavfilter/vf_drawtext.c b/libavfilter/vf_drawtext.c index 9fd9461..8e21693 100644 --- a/libavfilter/vf_drawtext.c +++ b/libavfilter/vf_drawtext.c @@ -824,6 +824,16 @@ static int func_pts(AVFilterContext *ctx, AVBPrint *bp, (int)(ms / 1000) % 60, (int)ms % 1000); } +} else if (!strcmp(fmt, "localtime") || + !strcmp(fmt, "gmtime")) { +struct tm tm; +time_t ms = (time_t)pts; +const char *timefmt = argc >= 3 ? argv[2] : "%Y-%m-%d %H:%M:%S"; +if (!strcmp(fmt, "localtime")) +localtime_r(&ms, &tm); +else +gmtime_r(&ms, &tm); +av_bprint_strftime(bp, timefmt, &tm); } else { av_log(ctx, AV_LOG_ERROR, "Invalid format '%s'\n", fmt); return AVERROR(EINVAL); @@ -958,7 +968,7 @@ static const struct drawtext_function { { "expr_int_format", 2, 3, 0, func_eval_expr_int_format }, { "eif", 2, 3, 0, func_eval_expr_int_format }, { "pict_type", 0, 0, 0, func_pict_type }, -{ "pts", 0, 2, 0, func_pts }, +{ "pts", 0, 3, 0, func_pts }, { "gmtime",0, 1, 'G', func_strftime }, { "localtime", 0, 1, 'L', func_strftime }, { "frame_num", 0, 0, 0, func_frame_num }, -- 2.3.8 (Apple Git-58) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[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) ) ) { +
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. > -
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; > >
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) >
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 = &
[FFmpeg-devel] Proposed patches to mpjpeg demux
Please look at the attached patches. They accomplish the following: 1) change to format.c: allows the demux to be matched by Content-Type, even if the incoming Content-Type header contains parameters, such as 'multipart/x-mixed-replace;boundary=ffserver’. Currently the comparison seems to occur verbatim, thus preventing the selection of mpjpegdec demux, even when running against MJPEG served by ffserver. P.S. I’m not sure if parameters are ever needed to be included for this comparison, but considering we’re dealing with probing, seems unlikely 2) changes to mpjpegdec.c: - allow the empty line after multipart MIME headers to be parsed without generating an error - if get_line encounters EOF, don’t fail right away (exit the loop and check if the relevant headers had been processed instead) - trim whitespace from both name and value extracted from a header, allowing for more flexibility for the incoming data -- Alex Agranovsky Sighthound, Inc www.sighthound.com ffmpeg3.patch Description: Binary data ffmpeg4.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Proposed patches to mpjpeg demux
Quick update: format.c patch needs to be amended, to include a NULL check. -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 10, 2015 at 12:04:55 PM, Alex Agranovsky (a...@sighthound.com) wrote: Please look at the attached patches. They accomplish the following: 1) change to format.c: allows the demux to be matched by Content-Type, even if the incoming Content-Type header contains parameters, such as 'multipart/x-mixed-replace;boundary=ffserver’. Currently the comparison seems to occur verbatim, thus preventing the selection of mpjpegdec demux, even when running against MJPEG served by ffserver. P.S. I’m not sure if parameters are ever needed to be included for this comparison, but considering we’re dealing with probing, seems unlikely 2) changes to mpjpegdec.c: - allow the empty line after multipart MIME headers to be parsed without generating an error - if get_line encounters EOF, don’t fail right away (exit the loop and check if the relevant headers had been processed instead) - trim whitespace from both name and value extracted from a header, allowing for more flexibility for the incoming data -- Alex Agranovsky Sighthound, Inc www.sighthound.com ffmpeg4.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] Proposed patches to mpjpeg demux
Michael, thanks! Is the other change (mpjpegdec.c) still in review, or had it been overlooked due to not being included in the amended patch? -Best. - Alex -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 11, 2015 at 6:43:31 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Thu, Sep 10, 2015 at 05:23:09PM -0400, Alex Agranovsky wrote: > Quick update: format.c patch needs to be amended, to include a NULL check. > > > > -- > Alex Agranovsky > Sighthound, Inc > www.sighthound.com > > On September 10, 2015 at 12:04:55 PM, Alex Agranovsky (a...@sighthound.com) > wrote: > > Please look at the attached patches. They accomplish the following: > > 1) change to format.c: allows the demux to be matched by Content-Type, even > if the incoming Content-Type header contains parameters, such as > 'multipart/x-mixed-replace;boundary=ffserver’. Currently the comparison seems > to occur verbatim, thus preventing the selection of mpjpegdec demux, even > when running against MJPEG served by ffserver. > P.S. I’m not sure if parameters are ever needed to be included for this > comparison, but considering we’re dealing with probing, seems unlikely applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes ___ 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] Proposed patches to mpjpeg demux
Sure thing — will split and resubmit. -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 12, 2015 at 2:37:52 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Sat, Sep 12, 2015 at 02:06:28PM -0400, Alex Agranovsky wrote: > Michael, thanks! > > Is the other change (mpjpegdec.c) still in review, or had it been overlooked > due to not being included in the amended patch? i didnt overlook them but they include several changes that could be argued to be independant so they need to be split into seperate patches / commits. can you do that split ? thanks > > -Best. > - Alex > > -- > Alex Agranovsky > Sighthound, Inc > www.sighthound.com > > On September 11, 2015 at 6:43:31 PM, Michael Niedermayer (michae...@gmx.at) > wrote: > > On Thu, Sep 10, 2015 at 05:23:09PM -0400, Alex Agranovsky wrote: > > Quick update: format.c patch needs to be amended, to include a NULL check. > > > > > > > > -- > > Alex Agranovsky > > Sighthound, Inc > > www.sighthound.com > > > > On September 10, 2015 at 12:04:55 PM, Alex Agranovsky (a...@sighthound.com) > > wrote: > > > > Please look at the attached patches. They accomplish the following: > > > > > 1) change to format.c: allows the demux to be matched by Content-Type, even > > if the incoming Content-Type header contains parameters, such as > > 'multipart/x-mixed-replace;boundary=ffserver’. Currently the comparison > > seems to occur verbatim, thus preventing the selection of mpjpegdec demux, > > even when running against MJPEG served by ffserver. > > P.S. I’m not sure if parameters are ever needed to be included for this > > comparison, but considering we’re dealing with probing, seems unlikely > > applied > > thanks > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > No human being will ever know the Truth, for even if they happen to say it > by chance, they would not even known they had done so. -- Xenophanes > ___ > 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 -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB When the tyrant has disposed of foreign enemies by conquest or treaty, and there is nothing more to fear from them, then he is always stirring up some war or other, in order that the people may require a leader. -- Plato ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Allow mpjpeg headers parsing to succeed upon EOF, if required headers are present
libavformat/mpjpegdec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index b8281fc..8413ae7 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -165,7 +165,7 @@ static int parse_multipart_header(AVFormatContext *s) ret = get_line(s->pb, line, sizeof(line)); if (ret < 0) - return ret; + break; if (line[0] == '\0') break; -- Alex Agranovsky Sighthound, Inc www.sighthound.com ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mpjpeg: CRLF terminating a sequence of MIME headers should not cause an error
libavformat/mpjpegdec.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 8413ae7..c1ca508 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -43,11 +43,22 @@ static int get_line(AVIOContext *pb, char *line, int line_size) static int split_tag_value(char **tag, char **value, char *line) { char *p = line; + int foundData = 0; - while (*p != '\0' && *p != ':') + *tag = NULL; + *value = NULL; + + + while (*p != '\0' && *p != ':') { + if (!av_isspace(*p)) { + foundData = 1; + } p++; - if (*p != ':') - return AVERROR_INVALIDDATA; + } + if (*p != ':') { + /* don't fail the parser if dealing with an empty line */ + return foundData?AVERROR_INVALIDDATA:0; + } *p = '\0'; *tag = line; @@ -166,12 +177,15 @@ static int parse_multipart_header(AVFormatContext *s) ret = get_line(s->pb, line, sizeof(line)); if (ret < 0) break; + /* CRLF terminates a sequence of MIME headers */ + if (value==NULL || tag==NULL) + break; if (line[0] == '\0') break; ret = split_tag_value(&tag, &value, line); - if (ret < 0) + if (ret < 0 || tag == NULL || value == NULL) return ret; if (!av_strcasecmp(tag, "Content-type")) { -- Alex Agranovsky Sighthound, Inc www.sighthound.com ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpjpeg: CRLF terminating a sequence of MIME headers should not cause an error
Amended: one of the new blocks ended up in a wrong location. libavformat/mpjpegdec.c | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 8413ae7..1661b9d 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -43,11 +43,20 @@ static int get_line(AVIOContext *pb, char *line, int line_size) static int split_tag_value(char **tag, char **value, char *line) { char *p = line; + int foundData = 0; - while (*p != '\0' && *p != ':') + *tag = NULL; + *value = NULL; + + + while (*p != '\0' && *p != ':') { + if (!av_isspace(*p)) { + foundData = 1; + } p++; + } if (*p != ':') - return AVERROR_INVALIDDATA; + return foundData?AVERROR_INVALIDDATA:0; *p = '\0'; *tag = line; @@ -67,7 +76,7 @@ static int check_content_type(char *line) char *tag, *value; int ret = split_tag_value(&tag, &value, line); - if (ret < 0) + if (ret < 0 || tag == NULL || value == NULL) return ret; if (av_strcasecmp(tag, "Content-type") || @@ -173,7 +182,9 @@ static int parse_multipart_header(AVFormatContext *s) ret = split_tag_value(&tag, &value, line); if (ret < 0) return ret; - + if (value==NULL || tag==NULL) + break; + if (!av_strcasecmp(tag, "Content-type")) { if (av_strcasecmp(value, "image/jpeg")) { av_log(s, AV_LOG_ERROR, -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 12, 2015 at 4:50:54 PM, Alex Agranovsky (a...@sighthound.com) wrote: libavformat/mpjpegdec.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 8413ae7..c1ca508 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -43,11 +43,22 @@ static int get_line(AVIOContext *pb, char *line, int line_size) static int split_tag_value(char **tag, char **value, char *line) { char *p = line; + int foundData = 0; - while (*p != '\0' && *p != ':') + *tag = NULL; + *value = NULL; + + + while (*p != '\0' && *p != ':') { + if (!av_isspace(*p)) { + foundData = 1; + } p++; - if (*p != ':') - return AVERROR_INVALIDDATA; + } + if (*p != ':') { + /* don't fail the parser if dealing with an empty line */ + return foundData?AVERROR_INVALIDDATA:0; + } *p = '\0'; *tag = line; @@ -166,12 +177,15 @@ static int parse_multipart_header(AVFormatContext *s) ret = get_line(s->pb, line, sizeof(line)); if (ret < 0) break; + /* CRLF terminates a sequence of MIME headers */ + if (value==NULL || tag==NULL) + break; if (line[0] == '\0') break; ret = split_tag_value(&tag, &value, line); - if (ret < 0) + if (ret < 0 || tag == NULL || value == NULL) return ret; if (!av_strcasecmp(tag, "Content-type")) { -- Alex Agranovsky Sighthound, Inc www.sighthound.com ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mpjpeg: trim header name/value of MIME headers while probing
libavformat/mpjpegdec.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c index 1661b9d..009bbaf 100644 --- a/libavformat/mpjpegdec.c +++ b/libavformat/mpjpegdec.c @@ -40,6 +40,19 @@ static int get_line(AVIOContext *pb, char *line, int line_size) 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) { char *p = line; @@ -60,6 +73,7 @@ static int split_tag_value(char **tag, char **value, char *line) *p = '\0'; *tag = line; + trim_right(*tag); p++; @@ -67,6 +81,7 @@ static int split_tag_value(char **tag, char **value, char *line) p++; *value = p; + trim_right(*value); return 0; } -- Alex Agranovsky Sighthound, Inc www.sighthound.com ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg headers parsing to succeed upon EOF, if required headers are present
Modified patch is attached. -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 12, 2015 at 5:37:51 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Sat, Sep 12, 2015 at 04:33:17PM -0400, Alex Agranovsky wrote: > libavformat/mpjpegdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index b8281fc..8413ae7 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -165,7 +165,7 @@ static int parse_multipart_header(AVFormatContext *s) > > ret = get_line(s->pb, line, sizeof(line)); > if (ret < 0) > - return ret; > + break; Applying: Allow mpjpeg headers parsing to succeed upon EOF, if required headers are present fatal: corrupt patch at line 7 also for strict correctness this should probably only break for AVERROR_EOF [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel patch1.diff Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpjpeg: CRLF terminating a sequence of MIME headers should not cause an error
You’re correct, sir. New patch is attached. -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 12, 2015 at 5:54:23 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Sat, Sep 12, 2015 at 05:05:04PM -0400, Alex Agranovsky wrote: > Amended: one of the new blocks ended up in a wrong location. > > libavformat/mpjpegdec.c | 19 +++ > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index 8413ae7..1661b9d 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -43,11 +43,20 @@ static int get_line(AVIOContext *pb, char *line, int > line_size) > static int split_tag_value(char **tag, char **value, char *line) > { > char *p = line; > + int foundData = 0; > > - while (*p != '\0' && *p != ':') > + *tag = NULL; > + *value = NULL; > + > + > + while (*p != '\0' && *p != ':') { > + if (!av_isspace(*p)) { > + foundData = 1; > + } > p++; > + } > if (*p != ':') > - return AVERROR_INVALIDDATA; > + return foundData?AVERROR_INVALIDDATA:0; > > *p = '\0'; > *tag = line; > @@ -67,7 +76,7 @@ static int check_content_type(char *line) > char *tag, *value; > int ret = split_tag_value(&tag, &value, line); > > - if (ret < 0) > + if (ret < 0 || tag == NULL || value == NULL) > return ret; doesnt this result in a probe score of AVPROBE_SCORE_MAX for an empty line, or am i misreading the code ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel patch2b.diff Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpjpeg: trim header name/value of MIME headers while probing
Attached. -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 12, 2015 at 5:41:56 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Sat, Sep 12, 2015 at 05:10:06PM -0400, Alex Agranovsky wrote: > libavformat/mpjpegdec.c | 15 +++ > 1 file changed, 15 insertions(+) patching file libavformat/mpjpegdec.c patch: malformed patch at line 6: return 0; it seems the patches have been corrupted somewhere in the mail processing, can you send the as attachment ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Concerning the gods, I have no means of knowing whether they exist or not or of what sort they may be, because of the obscurity of the subject, and the brevity of human life -- Protagoras ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel patch3.diff Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpjpeg: CRLF terminating a sequence of MIME headers should not cause an error
This is as intended: stopping at the empty line is exactly what we should do in this case, as it separates headers from the content. If the two headers we’re looking for hadn’t been encountered prior to the empty line, error should be returned. This is based on http://tools.ietf.org/html/rfc2046: body-part := MIME-part-headers [CRLF *OCTET] ; Lines in a body-part must not start ; with the specified dash-boundary and ; the delimiter must not appear anywhere ; in the body part. Note that the ; semantics of a body-part differ from ; the semantics of a message, as ; described in the text. -- Alex Agranovsky Sighthound, Inc www.sighthound.com On September 12, 2015 at 8:14:01 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Sat, Sep 12, 2015 at 07:04:26PM -0400, Alex Agranovsky wrote: > You’re correct, sir. New patch is attached. > > > -- > Alex Agranovsky > Sighthound, Inc > www.sighthound.com > > On September 12, 2015 at 5:54:23 PM, Michael Niedermayer (michae...@gmx.at) > wrote: > > On Sat, Sep 12, 2015 at 05:05:04PM -0400, Alex Agranovsky wrote: > > Amended: one of the new blocks ended up in a wrong location. > > > > libavformat/mpjpegdec.c | 19 +++ > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > > index 8413ae7..1661b9d 100644 > > --- a/libavformat/mpjpegdec.c > > +++ b/libavformat/mpjpegdec.c > > @@ -43,11 +43,20 @@ static int get_line(AVIOContext *pb, char *line, int > > line_size) > > static int split_tag_value(char **tag, char **value, char *line) > > { > > char *p = line; > > + int foundData = 0; > > > > - while (*p != '\0' && *p != ':') > > + *tag = NULL; > > + *value = NULL; > > + > > + > > + while (*p != '\0' && *p != ':') { > > + if (!av_isspace(*p)) { > > + foundData = 1; > > + } > > p++; > > + } > > if (*p != ':') > > - return AVERROR_INVALIDDATA; > > + return foundData?AVERROR_INVALIDDATA:0; > > > > *p = '\0'; > > *tag = line; > > @@ -67,7 +76,7 @@ static int check_content_type(char *line) > > char *tag, *value; > > int ret = split_tag_value(&tag, &value, line); > > > > - if (ret < 0) > > + if (ret < 0 || tag == NULL || value == NULL) > > return ret; > > doesnt this result in a probe score of AVPROBE_SCORE_MAX for an > empty line, or am i misreading the code ? > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Good people do not need laws to tell them to act responsibly, while bad > people will find a way around the laws. -- Plato > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > mpjpegdec.c | 19 --- > 1 file changed, 16 insertions(+), 3 deletions(-) > 7f04b810839ce200528855c2c9b50aaf45028045 patch2b.diff > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index e4d76f3..8c91bc4 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -43,11 +43,20 @@ static int get_line(AVIOContext *pb, char *line, int > line_size) > static int split_tag_value(char **tag, char **value, char *line) > { > char *p = line; > + int foundData = 0; > > - while (*p != '\0' && *p != ':') > + *tag = NULL; > + *value = NULL; > + > + > + while (*p != '\0' && *p != ':') { > + if (!av_isspace(*p)) { > + foundData = 1; > + } > p++; > + } > if (*p != ':') > - return AVERROR_INVALIDDATA; > + return foundData?AVERROR_INVALIDDATA:0; > > *p = '\0'; > *tag = line; > @@ -70,7 +79,9 @@ static int check_content_type(char *line) > if (ret < 0) > return ret; > > - if (av_strcasecmp(tag, "Content-type") || > + if ( tag == NULL || > + value == NULL || > + av_strcasecmp(tag, "Content-type") || > av_strcasecmp(value, "image/jpeg")) > return AVERROR_INVALIDDATA; > > @@ -176,6 +187,8 @@ static int parse_multipa
Re: [FFmpeg-devel] [PATCH] Allow mpjpeg headers parsing to succeed upon EOF, if required headers are present
My bad — sorry about that. On September 12, 2015 at 7:08:34 PM, Michael Niedermayer (michae...@gmx.at) wrote: On Sat, Sep 12, 2015 at 06:49:41PM -0400, Alex Agranovsky wrote: > Modified patch is attached. > > -- > Alex Agranovsky > Sighthound, Inc > www.sighthound.com > > On September 12, 2015 at 5:37:51 PM, Michael Niedermayer (michae...@gmx.at) > wrote: > > On Sat, Sep 12, 2015 at 04:33:17PM -0400, Alex Agranovsky wrote: > > libavformat/mpjpegdec.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > > index b8281fc..8413ae7 100644 > > --- a/libavformat/mpjpegdec.c > > +++ b/libavformat/mpjpegdec.c > > @@ -165,7 +165,7 @@ static int parse_multipart_header(AVFormatContext *s) > > > > ret = get_line(s->pb, line, sizeof(line)); > > if (ret < 0) > > - return ret; > > + break; > > Applying: Allow mpjpeg headers parsing to succeed upon EOF, if required > headers are present > fatal: corrupt patch at line 7 > > also for strict correctness this should probably only break for AVERROR_EOF > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Dictatorship naturally arises out of democracy, and the most aggravated > form of tyranny and slavery out of the most extreme liberty. -- Plato > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > mpjpegdec.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > 95570908ce1b8c6a97e45954d4f5a801a525d22c patch1.diff > diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c > index b8281fc..e4d76f3 100644 > --- a/libavformat/mpjpegdec.c > +++ b/libavformat/mpjpegdec.c > @@ -154,8 +154,11 @@ static int parse_multipart_header(AVFormatContext *s) > int ret, size = -1; > > ret = get_line(s->pb, line, sizeof(line)); > - if (ret < 0) > + if (ret < 0) { > + if (ret==AVERROR_EOF) > + break; > return ret; > + } this is a different hunk and it fails to build: ffmpeg/libavformat/mpjpegdec.c: In function ‘parse_multipart_header’: ffmpeg/libavformat/mpjpegdec.c:159:13: error: break statement not within loop or switch [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel patch1c.diff Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] mpjpeg: probe should require same constraints as packet reader - both proper content-type and content-size must be present
0001-mpjpeg-probe-should-require-same-constraints-as-pack.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] mpjpeg: probe should require same constraints as packet reader - both proper content-type and content-size must be present
A slightly improved version … On September 13, 2015 at 4:56:21 PM, Alex Agranovsky (a...@sighthound.com) wrote: 0001-mpjpeg-probe-should-require-same-constraints-as-pack.patch Description: Binary data ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel