Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
Michael Niedermayer niedermayer.cc> writes: > > New patch attached that fixes ticket #5151. > > LGTM Patch applied. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
Hi! I guess that attached patch fixes ticket #5151. It is the user's responsibility to know if the input stream is suitable for the bitstream filter or not. Please comment, Carl Eugen diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c index 68640db..b29039e 100644 --- a/libavcodec/mjpeg2jpeg_bsf.c +++ b/libavcodec/mjpeg2jpeg_bsf.c @@ -31,6 +31,7 @@ #include "avcodec.h" #include "jpegtables.h" +#include "mjpeg.h" static const uint8_t jpeg_header[] = { 0xff, 0xd8, // SOI @@ -88,11 +89,11 @@ static int mjpeg2jpeg_filter(AVBitStreamFilterContext *bsfc, av_log(avctx, AV_LOG_ERROR, "input is truncated\n"); return AVERROR_INVALIDDATA; } -if (memcmp("AVI1", buf + 6, 4)) { -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); -return AVERROR_INVALIDDATA; +if (buf[2] == 0xff && buf[3] == APP0) { +input_skip = (buf[4] << 8) + buf[5] + 4; +} else { +input_skip = 2; } -input_skip = (buf[4] << 8) + buf[5] + 4; if (buf_size < input_skip) { av_log(avctx, AV_LOG_ERROR, "input is truncated\n"); return AVERROR_INVALIDDATA; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tuesday 12 January 2016 02:16:52 pm Michael Niedermayer wrote: > On Tue, Jan 12, 2016 at 09:58:53AM +0100, Carl Eugen Hoyos wrote: > > -if (memcmp("AVI1", buf + 6, 4)) { > > -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); > > -return AVERROR_INVALIDDATA; > > +if (buf[2] == 0xff && buf[3] == APP0) { > > +input_skip = (buf[4] << 8) + buf[5] + 4; > > +} else { > > +input_skip = 2; > > shouldnt the first 2 bytes that are being skiped be checked ? I don't know (possibly) but it seems unrelated to this patch: They are not checked now. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tue, Jan 12, 2016 at 02:41:12PM +0100, Carl Eugen Hoyos wrote: > On Tuesday 12 January 2016 02:28:28 pm Michael Niedermayer wrote: > > On Tue, Jan 12, 2016 at 02:19:53PM +0100, Carl Eugen Hoyos wrote: > > > On Tuesday 12 January 2016 02:16:52 pm Michael Niedermayer wrote: > > > > On Tue, Jan 12, 2016 at 09:58:53AM +0100, Carl Eugen Hoyos wrote: > > > > > -if (memcmp("AVI1", buf + 6, 4)) { > > > > > -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); > > > > > -return AVERROR_INVALIDDATA; > > > > > +if (buf[2] == 0xff && buf[3] == APP0) { > > > > > +input_skip = (buf[4] << 8) + buf[5] + 4; > > > > > +} else { > > > > > +input_skip = 2; > > > > > > > > shouldnt the first 2 bytes that are being skiped be checked ? > > > > > > I don't know (possibly) but it seems unrelated to this patch: > > > They are not checked now. > > > > true > > > > still before the patch 4 bytes are checked, afterwards none > > these 4 bytes sort of imply that the previous bytes arent arbitrary > > > > if the 2 bytes are different from what is expected then the code > > would potentially generate invalid output, or do i miss some check > > elsewhere that would prevent that ? > > New patch attached. > > Please comment, Carl Eugen > mjpeg2jpeg_bsf.c |5 + > 1 file changed, 5 insertions(+) > a261f4350cbfeefc9c011cfc93fc39e5c4f7fe7c patchmjpeg2jpgffd8.diff > diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tue, Jan 12, 2016 at 02:19:53PM +0100, Carl Eugen Hoyos wrote: > On Tuesday 12 January 2016 02:16:52 pm Michael Niedermayer wrote: > > On Tue, Jan 12, 2016 at 09:58:53AM +0100, Carl Eugen Hoyos wrote: > > > > -if (memcmp("AVI1", buf + 6, 4)) { > > > -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); > > > -return AVERROR_INVALIDDATA; > > > +if (buf[2] == 0xff && buf[3] == APP0) { > > > +input_skip = (buf[4] << 8) + buf[5] + 4; > > > +} else { > > > +input_skip = 2; > > > > shouldnt the first 2 bytes that are being skiped be checked ? > > I don't know (possibly) but it seems unrelated to this patch: > They are not checked now. true still before the patch 4 bytes are checked, afterwards none these 4 bytes sort of imply that the previous bytes arent arbitrary if the 2 bytes are different from what is expected then the code would potentially generate invalid output, or do i miss some check elsewhere that would prevent that ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Everything should be made as simple as possible, but not simpler. -- Albert Einstein signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tue, Jan 12, 2016 at 09:58:53AM +0100, Carl Eugen Hoyos wrote: > Hi! > > I guess that attached patch fixes ticket #5151. > It is the user's responsibility to know if the input stream > is suitable for the bitstream filter or not. > > Please comment, Carl Eugen > mjpeg2jpeg_bsf.c |9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > 6d36c24e50d0ba2484b959c5f315de919c57ae5f patchmjpeg2jpg.diff > diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c > index 68640db..b29039e 100644 > --- a/libavcodec/mjpeg2jpeg_bsf.c > +++ b/libavcodec/mjpeg2jpeg_bsf.c > @@ -31,6 +31,7 @@ > > #include "avcodec.h" > #include "jpegtables.h" > +#include "mjpeg.h" > > static const uint8_t jpeg_header[] = { > 0xff, 0xd8, // SOI > @@ -88,11 +89,11 @@ static int mjpeg2jpeg_filter(AVBitStreamFilterContext > *bsfc, > av_log(avctx, AV_LOG_ERROR, "input is truncated\n"); > return AVERROR_INVALIDDATA; > } > -if (memcmp("AVI1", buf + 6, 4)) { > -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); > -return AVERROR_INVALIDDATA; > +if (buf[2] == 0xff && buf[3] == APP0) { > +input_skip = (buf[4] << 8) + buf[5] + 4; > +} else { > +input_skip = 2; shouldnt the first 2 bytes that are being skiped be checked ? [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Rewriting code that is poorly written but fully understood is good. Rewriting code that one doesnt understand is a sign that one is less smart then the original author, trying to rewrite it will not make it better. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tuesday 12 January 2016 02:28:28 pm Michael Niedermayer wrote: > On Tue, Jan 12, 2016 at 02:19:53PM +0100, Carl Eugen Hoyos wrote: > > On Tuesday 12 January 2016 02:16:52 pm Michael Niedermayer wrote: > > > On Tue, Jan 12, 2016 at 09:58:53AM +0100, Carl Eugen Hoyos wrote: > > > > -if (memcmp("AVI1", buf + 6, 4)) { > > > > -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); > > > > -return AVERROR_INVALIDDATA; > > > > +if (buf[2] == 0xff && buf[3] == APP0) { > > > > +input_skip = (buf[4] << 8) + buf[5] + 4; > > > > +} else { > > > > +input_skip = 2; > > > > > > shouldnt the first 2 bytes that are being skiped be checked ? > > > > I don't know (possibly) but it seems unrelated to this patch: > > They are not checked now. > > true > > still before the patch 4 bytes are checked, afterwards none > these 4 bytes sort of imply that the previous bytes arent arbitrary > > if the 2 bytes are different from what is expected then the code > would potentially generate invalid output, or do i miss some check > elsewhere that would prevent that ? New patch attached. Please comment, Carl Eugen diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c index 68640db..71f0154 100644 --- a/libavcodec/mjpeg2jpeg_bsf.c +++ b/libavcodec/mjpeg2jpeg_bsf.c @@ -28,6 +28,7 @@ #include "libavutil/error.h" #include "libavutil/mem.h" +#include "libavutil/intreadwrite.h" #include "avcodec.h" #include "jpegtables.h" @@ -88,6 +89,10 @@ static int mjpeg2jpeg_filter(AVBitStreamFilterContext *bsfc, av_log(avctx, AV_LOG_ERROR, "input is truncated\n"); return AVERROR_INVALIDDATA; } +if (AV_RB16(buf) != 0xffd8) { +av_log(avctx, AV_LOG_ERROR, "input is not MJPEG\n"); +return AVERROR_INVALIDDATA; +} if (memcmp("AVI1", buf + 6, 4)) { av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); return AVERROR_INVALIDDATA; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tuesday 12 January 2016 03:19:24 pm Michael Niedermayer wrote: > > mjpeg2jpeg_bsf.c |5 + > > 1 file changed, 5 insertions(+) > > a261f4350cbfeefc9c011cfc93fc39e5c4f7fe7c patchmjpeg2jpgffd8.diff > > diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c > > LGTM Patch applied. New patch attached that fixes ticket #5151. Thank you, Carl Eugen diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c index 71f0154..92dc3ca 100644 --- a/libavcodec/mjpeg2jpeg_bsf.c +++ b/libavcodec/mjpeg2jpeg_bsf.c @@ -32,6 +32,7 @@ #include "avcodec.h" #include "jpegtables.h" +#include "mjpeg.h" static const uint8_t jpeg_header[] = { 0xff, 0xd8, // SOI @@ -93,11 +94,11 @@ static int mjpeg2jpeg_filter(AVBitStreamFilterContext *bsfc, av_log(avctx, AV_LOG_ERROR, "input is not MJPEG\n"); return AVERROR_INVALIDDATA; } -if (memcmp("AVI1", buf + 6, 4)) { -av_log(avctx, AV_LOG_ERROR, "input is not MJPEG/AVI1\n"); -return AVERROR_INVALIDDATA; +if (buf[2] == 0xff && buf[3] == APP0) { +input_skip = (buf[4] << 8) + buf[5] + 4; +} else { +input_skip = 2; } -input_skip = (buf[4] << 8) + buf[5] + 4; if (buf_size < input_skip) { av_log(avctx, AV_LOG_ERROR, "input is truncated\n"); return AVERROR_INVALIDDATA; ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/mjpeg2jpeg: Accept more mjpeg streams as input
On Tue, Jan 12, 2016 at 04:18:35PM +0100, Carl Eugen Hoyos wrote: > On Tuesday 12 January 2016 03:19:24 pm Michael Niedermayer wrote: > > > mjpeg2jpeg_bsf.c |5 + > > > 1 file changed, 5 insertions(+) > > > a261f4350cbfeefc9c011cfc93fc39e5c4f7fe7c patchmjpeg2jpgffd8.diff > > > diff --git a/libavcodec/mjpeg2jpeg_bsf.c b/libavcodec/mjpeg2jpeg_bsf.c > > > > LGTM > > Patch applied. > > New patch attached that fixes ticket #5151. LGTM thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Never trust a computer, one day, it may think you are the virus. -- Compn signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel