On 11/30/15 7:34 AM, wm4 wrote:
On Mon, 30 Nov 2015 07:27:14 -0500
Alexander Agranovsky <a...@sighthound.com> 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 <a...@sighthound.com>
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 <a...@sighthound.com>
---
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 @@ static int parse_multipart_header(AVIOContext *pb, void
*log_ctx)
/* 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)
return ret;
}
- if (strncmp(line, "--", 2))
+ if (!av_strstart(line, expected_boundary, NULL)) {
+ av_log(log_ctx,
+ AV_LOG_ERROR,
+ "Expected boundary '%s' not found, instead found a line of %zu
bytes\n",
+ expected_boundary,
+ strlen(line));
+
return AVERROR_INVALIDDATA;
+ }
while (!pb->eof_reached) {
char *tag, *value;
@@ -191,42 +223,90 @@ static int parse_multipart_header(AVIOContext *pb, void
*log_ctx)
if (!av_strcasecmp(tag, "Content-type")) {
if (av_strcasecmp(value, "image/jpeg")) {
- if (log_ctx) {
- av_log(log_ctx, AV_LOG_ERROR,
+ av_log(log_ctx, AV_LOG_ERROR,
"Unexpected %s : %s\n",
tag, value);
- }
-
return AVERROR_INVALIDDATA;
} else
found_content_type = 1;
} else if (!av_strcasecmp(tag, "Content-Length")) {
- size = parse_content_length(value);
- if (size < 0)
- return size;
+ *size = parse_content_length(value);
+ if ( *size < 0 )
+ av_log(log_ctx, AV_LOG_WARNING,
+ "Invalid Content-Length value : %s\n",
+ value);
}
}
- if (!found_content_type || size < 0) {
- return AVERROR_INVALIDDATA;
- }
-
- return size;
+ return found_content_type ? 0 : AVERROR_INVALIDDATA;
}
+
static int mpjpeg_read_packet(AVFormatContext *s, AVPacket *pkt)
{
+ int size;
int ret;
- int size = parse_multipart_header(s->pb, s);
- if (size < 0)
- return size;
+ MPJPEGDemuxContext *mpjpeg = s->priv_data;
+ if (mpjpeg->boundary == NULL) {
+ mpjpeg->boundary = av_strdup("--");
+ mpjpeg->searchstr = av_strdup("\r\n--");
+ if (!mpjpeg->boundary || !mpjpeg->searchstr) {
+ av_freep(&mpjpeg->boundary);
+ av_freep(&mpjpeg->searchstr);
+ return AVERROR(ENOMEM);
+ }
+ mpjpeg->searchstr_len = strlen(mpjpeg->searchstr);
+ }
+
+ ret = parse_multipart_header(s->pb, &size, mpjpeg->boundary, s);
+
- ret = av_get_packet(s->pb, pkt, size);
if (ret < 0)
return ret;
- return 0;
+ if (size > 0) {
+ /* size has been provided to us in MIME header */
+ ret = av_get_packet(s->pb, pkt, size);
+ } else {
+ /* no size was given -- we read until the next boundary or end-of-file
*/
+ int remaining = 0, len;
+
+ const int read_chunk = 2048;
+ av_init_packet(pkt);
+ pkt->data = NULL;
+ pkt->size = 0;
+ pkt->pos = avio_tell(s->pb);
+
+ /* we may need to return as much as all we've read back to the buffer
*/
+ ffio_ensure_seekback(s->pb, read_chunk);
+
+ while ((ret = av_append_packet(s->pb, pkt, read_chunk - remaining)) >=
0) {
+ /* scan the new data */
+ len = ret + remaining;
+ char *start = pkt->data + pkt->size - len;
+ do {
+ if (!memcmp(start, mpjpeg->searchstr, mpjpeg->searchstr_len)) {
+ // got the boundary! rewind the stream
+ avio_seek(s->pb, -(len-2), SEEK_CUR);
+ pkt->size -= (len-2);
+ return pkt->size;
+ }
+ len--;
+ start++;
+ } while (len >= mpjpeg->searchstr_len);
+ remaining = len;
+ }
+
+ /* error or EOF occurred */
+ if (ret == AVERROR_EOF) {
+ ret = pkt->size > 0 ? pkt->size : AVERROR_EOF;
+ } else {
+ av_packet_unref(pkt);
+ }
+ }
+
+ return ret;
}
AVInputFormat ff_mpjpeg_demuxer = {
@@ -234,7 +314,9 @@ AVInputFormat ff_mpjpeg_demuxer = {
.long_name = NULL_IF_CONFIG_SMALL("MIME multipart JPEG"),
.mime_type = "multipart/x-mixed-replace",
.extensions = "mjpg",
+ .priv_data_size = sizeof(MPJPEGDemuxContext),
.read_probe = mpjpeg_read_probe,
.read_header = mpjpeg_read_header,
.read_packet = mpjpeg_read_packet,
+ .read_close = mpjpeg_read_close
};
--
2.4.9 (Apple Git-60)
From 737dc80c59c0ecca090901b72b460e99781b9a39 Mon Sep 17 00:00:00 2001
From: Alex Agranovsky <a...@sighthound.com>
Date: Sun, 29 Nov 2015 18:54:14 -0500
Subject: [PATCH 2/2] avformat/mpjpeg: utilize MIME boundary value to detect
start of new frame
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 <a...@sighthound.com>
---
doc/demuxers.texi | 15 ++++++++++
libavformat/mpjpegdec.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/doc/demuxers.texi b/doc/demuxers.texi
index 349b531..fb1e4fb 100644
--- a/doc/demuxers.texi
+++ b/doc/demuxers.texi
@@ -450,6 +450,21 @@ to 1 (-1 means automatic setting, 1 means enabled, 0 means
disabled). Default value is -1.
@end table
+@section mpjpeg
+
+MJPEG encapsulated in multi-part MIME demuxer.
+
+This demuxer allows reading of MJPEG, where each frame is represented as a
part of
+multipart/x-mixed-replace stream.
+@table @option
+
+@item strict_mime_boundary
+Default implementation applies a relaxed standard to multi-part MIME boundary
detection,
+to prevent regression with numerous existing endpoints not generating a proper
MIME
+MJPEG stream. Turning this option on by setting it to 1 will result in a
stricter check
+of the boundary value.
+@end table
+
@section rawvideo
Raw video demuxer.
diff --git a/libavformat/mpjpegdec.c b/libavformat/mpjpegdec.c
index 9d5700a..dd4ad9b 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;
@@ -242,6 +245,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;
@@ -249,8 +289,17 @@ 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_asprintf( "\r\n%s\r\n", boundary );
+ } else {
+ mpjpeg->boundary = av_strdup("--");
+ mpjpeg->searchstr = av_strdup("\r\n--");
+ }
if (!mpjpeg->boundary || !mpjpeg->searchstr) {
av_freep(&mpjpeg->boundary);
av_freep(&mpjpeg->searchstr);
@@ -309,6 +358,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 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"),
@@ -318,5 +383,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
};
+
+
--
2.4.9 (Apple Git-60)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
