Re: [FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
Michael Bradshaw gmail.com> writes: > > +return AVPROBE_SCORE_MAX / 4 + 1; > > A score of 26 seems low to me, but maybe that's just me. Increased the score and pushed. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
On Tue, Jan 12, 2016 at 08:33:47AM -0800, Michael Bradshaw wrote: > Overall it looks good. I thought it might overflow the buffer but with > AVPROBE_PADDING_SIZE it doesn't. > > On Tue, Jan 12, 2016 at 7:09 AM, Carl Eugen Hoyos wrote: > > diff --git a/libavformat/icodec.c b/libavformat/icodec.c > > index 22e2099..9cf3dca 100644 > > --- a/libavformat/icodec.c > > +++ b/libavformat/icodec.c > > @@ -27,6 +27,7 @@ > > #include "libavutil/intreadwrite.h" > > #include "libavcodec/bytestream.h" > > #include "libavcodec/bmp.h" > > +#include "libavcodec/png.h" > > #include "avformat.h" > > #include "internal.h" > > > > @@ -44,9 +45,30 @@ typedef struct { > > > > static int probe(AVProbeData *p) > > { > > -if (AV_RL16(p->buf) == 0 && AV_RL16(p->buf + 2) == 1 && AV_RL16(p->buf > > + 4)) > > -return AVPROBE_SCORE_MAX / 4; > > -return 0; > > +unsigned i, frames = AV_RL16(p->buf + 4); > > + > > +if (AV_RL16(p->buf) || AV_RL16(p->buf + 2) != 1 || !frames) > > +return 0; > > +for (i = 0; i < frames; i++) { > > +unsigned offset; > > +if (AV_RL16(p->buf + 10 + i * 16) & ~1) // color planes > > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > > +if (p->buf[13 + i * 16]) > > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > > +if (AV_RL32(p->buf + 14 + i * 16) < 40) // size > > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > > +offset = AV_RL32(p->buf + 18 + i * 16); > > +if (offset < 22) > > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > > +if (offset + 8 > p->buf_size) > > +return AVPROBE_SCORE_MAX / 4 + FFMIN(i, 1); > > +if (p->buf[offset] != 40 && AV_RB64(p->buf + offset) != PNGSIG) > > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > > +if (i * 16 + 6 > p->buf_size) > > +return AVPROBE_SCORE_MAX / 4; > > +} > > + > > +return AVPROBE_SCORE_MAX / 4 + 1; > > A score of 26 seems low to me, but maybe that's just me. same feeling either way LGTM [...] -- 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 signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
Overall it looks good. I thought it might overflow the buffer but with AVPROBE_PADDING_SIZE it doesn't. On Tue, Jan 12, 2016 at 7:09 AM, Carl Eugen Hoyos wrote: > diff --git a/libavformat/icodec.c b/libavformat/icodec.c > index 22e2099..9cf3dca 100644 > --- a/libavformat/icodec.c > +++ b/libavformat/icodec.c > @@ -27,6 +27,7 @@ > #include "libavutil/intreadwrite.h" > #include "libavcodec/bytestream.h" > #include "libavcodec/bmp.h" > +#include "libavcodec/png.h" > #include "avformat.h" > #include "internal.h" > > @@ -44,9 +45,30 @@ typedef struct { > > static int probe(AVProbeData *p) > { > -if (AV_RL16(p->buf) == 0 && AV_RL16(p->buf + 2) == 1 && AV_RL16(p->buf + > 4)) > -return AVPROBE_SCORE_MAX / 4; > -return 0; > +unsigned i, frames = AV_RL16(p->buf + 4); > + > +if (AV_RL16(p->buf) || AV_RL16(p->buf + 2) != 1 || !frames) > +return 0; > +for (i = 0; i < frames; i++) { > +unsigned offset; > +if (AV_RL16(p->buf + 10 + i * 16) & ~1) // color planes > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > +if (p->buf[13 + i * 16]) > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > +if (AV_RL32(p->buf + 14 + i * 16) < 40) // size > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > +offset = AV_RL32(p->buf + 18 + i * 16); > +if (offset < 22) > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > +if (offset + 8 > p->buf_size) > +return AVPROBE_SCORE_MAX / 4 + FFMIN(i, 1); > +if (p->buf[offset] != 40 && AV_RB64(p->buf + offset) != PNGSIG) > +return FFMIN(i, AVPROBE_SCORE_MAX / 4); > +if (i * 16 + 6 > p->buf_size) > +return AVPROBE_SCORE_MAX / 4; > +} > + > +return AVPROBE_SCORE_MAX / 4 + 1; A score of 26 seems low to me, but maybe that's just me. > } > > static int read_header(AVFormatContext *s) I checked all the various header bytes this would be checking and it all looks good. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
On Tuesday 12 January 2016 02:49:59 pm Michael Niedermayer wrote: > On Tue, Jan 12, 2016 at 01:49:37PM +0100, Carl Eugen Hoyos wrote: > > Hi! > > > > On Tuesday 12 January 2016 01:33:53 pm Carl Eugen Hoyos wrote: > > > Attached patch improves ico probing, previously mpeg-2 frames could be > > > detected. > > > > Wikipedia claims that planes can be 0, new patch attached. > > > > Carl Eugen > > > > icodec.c |8 ++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > 5587972d12000a679757c9b42947869eb1cee0f5 patchico.diff > > diff --git a/libavformat/icodec.c b/libavformat/icodec.c > > index 22e2099..4d6e6dc 100644 > > tools/probetest 256 4096 > testing size=1 > testing size=2 > testing size=4 > testing size=8 > Failure of ico probing code with score=26 type=1 p=EA6 size=8 Before the old patch, 32 bits were tested and return a value of 25, after the patch, 55 bits were tested and returned 26. Imo, this just shows that probetest cannot see every possible issue. Improved patch attached. Carl Eugen diff --git a/libavformat/icodec.c b/libavformat/icodec.c index 22e2099..9cf3dca 100644 --- a/libavformat/icodec.c +++ b/libavformat/icodec.c @@ -27,6 +27,7 @@ #include "libavutil/intreadwrite.h" #include "libavcodec/bytestream.h" #include "libavcodec/bmp.h" +#include "libavcodec/png.h" #include "avformat.h" #include "internal.h" @@ -44,9 +45,30 @@ typedef struct { static int probe(AVProbeData *p) { -if (AV_RL16(p->buf) == 0 && AV_RL16(p->buf + 2) == 1 && AV_RL16(p->buf + 4)) -return AVPROBE_SCORE_MAX / 4; -return 0; +unsigned i, frames = AV_RL16(p->buf + 4); + +if (AV_RL16(p->buf) || AV_RL16(p->buf + 2) != 1 || !frames) +return 0; +for (i = 0; i < frames; i++) { +unsigned offset; +if (AV_RL16(p->buf + 10 + i * 16) & ~1) +return FFMIN(i, AVPROBE_SCORE_MAX / 4); +if (p->buf[13 + i * 16]) +return FFMIN(i, AVPROBE_SCORE_MAX / 4); +if (AV_RL32(p->buf + 14 + i * 16) < 40) +return FFMIN(i, AVPROBE_SCORE_MAX / 4); +offset = AV_RL32(p->buf + 18 + i * 16); +if (offset < 22) +return FFMIN(i, AVPROBE_SCORE_MAX / 4); +if (offset + 8 > p->buf_size) +return AVPROBE_SCORE_MAX / 4 + FFMIN(i, 1); +if (p->buf[offset] != 40 && AV_RB64(p->buf + offset) != PNGSIG) +return FFMIN(i, AVPROBE_SCORE_MAX / 4); +if (i * 16 + 6 > p->buf_size) +return AVPROBE_SCORE_MAX / 4; +} + +return AVPROBE_SCORE_MAX / 4 + 1; } static int read_header(AVFormatContext *s) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
On Tue, Jan 12, 2016 at 01:49:37PM +0100, Carl Eugen Hoyos wrote: > Hi! > > On Tuesday 12 January 2016 01:33:53 pm Carl Eugen Hoyos wrote: > > > > Attached patch improves ico probing, previously mpeg-2 frames could be > > detected. > > Wikipedia claims that planes can be 0, new patch attached. > > Carl Eugen > icodec.c |8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > 5587972d12000a679757c9b42947869eb1cee0f5 patchico.diff > diff --git a/libavformat/icodec.c b/libavformat/icodec.c > index 22e2099..4d6e6dc 100644 tools/probetest 256 4096 testing size=1 testing size=2 testing size=4 testing size=8 Failure of ico probing code with score=26 type=1 p=EA6 size=8 testing size=16 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The misfortune of the wise is better than the prosperity of the fool. -- Epicurus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
Hi! On Tuesday 12 January 2016 01:33:53 pm Carl Eugen Hoyos wrote: > > Attached patch improves ico probing, previously mpeg-2 frames could be > detected. Wikipedia claims that planes can be 0, new patch attached. Carl Eugen diff --git a/libavformat/icodec.c b/libavformat/icodec.c index 22e2099..4d6e6dc 100644 --- a/libavformat/icodec.c +++ b/libavformat/icodec.c @@ -44,8 +44,12 @@ typedef struct { static int probe(AVProbeData *p) { -if (AV_RL16(p->buf) == 0 && AV_RL16(p->buf + 2) == 1 && AV_RL16(p->buf + 4)) -return AVPROBE_SCORE_MAX / 4; +if ( AV_RL16(p->buf) == 0 +&& AV_RL16(p->buf + 2) == 1 +&& AV_RL16(p->buf + 4) +&& !(AV_RL16(p->buf + 10) & ~1) +&& !p->buf[13]) +return AVPROBE_SCORE_MAX / 4 + 1; return 0; } ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavf/icodec: Improve probe function
Hi! Attached patch improves ico probing, previously mpeg-2 frames could be detected. Please comment, Carl Eugen diff --git a/libavformat/icodec.c b/libavformat/icodec.c index 22e2099..c3e87ea 100644 --- a/libavformat/icodec.c +++ b/libavformat/icodec.c @@ -44,8 +44,12 @@ typedef struct { static int probe(AVProbeData *p) { -if (AV_RL16(p->buf) == 0 && AV_RL16(p->buf + 2) == 1 && AV_RL16(p->buf + 4)) -return AVPROBE_SCORE_MAX / 4; +if ( AV_RL16(p->buf) == 0 +&& AV_RL16(p->buf + 2) == 1 +&& AV_RL16(p->buf + 4) +&& AV_RL16(p->buf + 10) == 1 +&& !p->buf[13]) +return AVPROBE_SCORE_MAX / 4 + 1; return 0; } ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel