[FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
wm4
nfxjfg at googlemail.com
Sun Nov 29 19:00:20 CET 2015
On Wed, 25 Nov 2015 10:35:31 -0500
Alex Agranovsky <alex at sighthound.com> wrote:
> From 70a6e1b0f3d47698bf49c3c766d5472646bff71a Mon Sep 17 00:00:00 2001
> From: Alex Agranovsky <alex at sighthound.com>
> 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 <alex at sighthound.com>
> ---
> 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, 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)) {
> + 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));
> + }
> +
> return AVERROR_INVALIDDATA;
> + }
>
> while (!pb->eof_reached) {
> char *tag, *value;
> @@ -201,32 +237,82 @@ static int parse_multipart_header(AVIOContext *pb, void *log_ctx)
> } 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 (!found_content_type || size < 0) {
> - return AVERROR_INVALIDDATA;
> - }
> -
> - return size;
> + return (found_content_type) ? 0 : AVERROR_INVALIDDATA;
The ( ) could be removed.
> }
>
> +
> 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--");
> + mpjpeg->searchstr_len = strlen(mpjpeg->searchstr);
Seems like you forgot to remove this line.
> + 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 */
> +
> + 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);
> +
> +
> + int remaining = 0, len;
Declaration after statements (move to the top of the block).
> +
> + 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 +320,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
> };
Other than these, looks ok.
More information about the ffmpeg-devel
mailing list