[FFmpeg-devel] [PATCH] Allow mpjpeg demuxer to process MIME parts which do not include Content-Length header.
alex at sighthound.com
Mon Nov 30 13:35:18 CET 2015
On 11/30/15 6:37 AM, wm4 wrote:
> On Mon, 30 Nov 2015 08:41:42 +0100
> Nicolas George <george at nsup.org> 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 at ffmpeg.org
More information about the ffmpeg-devel