Re: [FFmpeg-devel] [PATCH V3 1/2] lavf/vc1test: fix vc1test can't probe some RCV file.

2018-10-15 Thread myp...@gmail.com
On Mon, Oct 15, 2018 at 3:15 PM Jerome Borsboom
 wrote:
>
> > case 1:
> > use the hexdump -C SMM0005.rcv get:
> >  size  skip (size - 4)
> >   ||
> >   VV
> >   18 00 00 c5 05 00 00 00  4d f1 0a 11 00 e0 01 00
> > 0010  00 d0 02 00 00 0c 00 00  00 88 13 00 00 c0 65 52
> >  ^
> >|
> >size + 16
> > case 2:
> > same the command for SMM0015.rcv get:
> > size
> >   |
> >   V
> >   19 00 00 c5 04 00 00 00  41 f3 80 01 40 02 00 00
> > 0010  d0 02 00 00 0c 00 00 00  00 00 00 10 00 00 00 00
> >   ^
> > |
> >  size + 16
> >
> > There are different the RCV file format for VC-1, vc1test
> > just handle the case 2 now, this fix will support the case 1.
> > (Both of test clips come from: SMPTE Recommended Practice -
> > VC-1 Decoder and Bitstream Conformance). And I think I got
> > a older VC-1 test clip in the case 1.
> >
> > Reviewed-by: Carl Eugen Hoyos 
> > Reviewed-by: Jerome Borsboom 
> > Reviewed-by: Michael Niedermayer 
> > Signed-off-by: Jun Zhao 
> > Signed-off-by: Yan, FengX 
> > ---
> >  libavformat/vc1test.c |   11 +--
> >  1 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c
> > index a801f4b..e029ff4 100644
> > --- a/libavformat/vc1test.c
> > +++ b/libavformat/vc1test.c
> > @@ -34,9 +34,13 @@
> >
> >  static int vc1t_probe(AVProbeData *p)
> >  {
> > +int size;
> > +
> >  if (p->buf_size < 24)
> >  return 0;
> > -if (p->buf[3] != 0xC5 || AV_RL32(>buf[4]) != 4 ||
> AV_RL32(>buf[20]) != 0xC)
> > +
> > +size = AV_RL32(>buf[4]);
> > +if (p->buf[3] != 0xC5 || size < 4 || AV_RL32(>buf[size+16]) !=
> 0xC)
> >  return 0;
> >
> >  return AVPROBE_SCORE_EXTENSION;
> > @@ -48,9 +52,10 @@ static int vc1t_read_header(AVFormatContext *s)
> >  AVStream *st;
> >  int frames;
> >  uint32_t fps;
> > +int size;
> >
> >  frames = avio_rl24(pb);
> > -if(avio_r8(pb) != 0xC5 || avio_rl32(pb) != 4)
> > +if (avio_r8(pb) != 0xC5 || ((size = avio_rl32(pb)) < 4))
> >  return AVERROR_INVALIDDATA;
> >
> >  /* init video codec */
> > @@ -63,6 +68,8 @@ static int vc1t_read_header(AVFormatContext *s)
> >
> >  if (ff_get_extradata(s, st->codecpar, pb, VC1_EXTRADATA_SIZE) < 0)
> >  return AVERROR(ENOMEM);
> > +
> > +avio_skip(pb, size - 4);
> >  st->codecpar->height = avio_rl32(pb);
> >  st->codecpar->width = avio_rl32(pb);
> >  if(avio_rl32(pb) != 0xC)
> > --
> > 1.7.1
>
> You may still overread the buffer as the first check on buf_size only
> checks for at least 24 bytes. The following p->buf[size+16] may read
> beyond the end of the buffer.
>
I see, need to double-check the size with " size < 4 || size + 20 >
p->buf_size" in probe
> Regards,
> Jerome
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH V3 1/2] lavf/vc1test: fix vc1test can't probe some RCV file.

2018-10-15 Thread Jerome Borsboom
> case 1:
> use the hexdump -C SMM0005.rcv get:
>  size  skip (size - 4)
>   ||
>   VV
>   18 00 00 c5 05 00 00 00  4d f1 0a 11 00 e0 01 00
> 0010  00 d0 02 00 00 0c 00 00  00 88 13 00 00 c0 65 52
>  ^
>|
>size + 16
> case 2:
> same the command for SMM0015.rcv get:
> size
>   |
>   V
>   19 00 00 c5 04 00 00 00  41 f3 80 01 40 02 00 00
> 0010  d0 02 00 00 0c 00 00 00  00 00 00 10 00 00 00 00
>   ^
> |
>  size + 16
>
> There are different the RCV file format for VC-1, vc1test
> just handle the case 2 now, this fix will support the case 1.
> (Both of test clips come from: SMPTE Recommended Practice -
> VC-1 Decoder and Bitstream Conformance). And I think I got
> a older VC-1 test clip in the case 1.
>
> Reviewed-by: Carl Eugen Hoyos 
> Reviewed-by: Jerome Borsboom 
> Reviewed-by: Michael Niedermayer 
> Signed-off-by: Jun Zhao 
> Signed-off-by: Yan, FengX 
> ---
>  libavformat/vc1test.c |   11 +--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c
> index a801f4b..e029ff4 100644
> --- a/libavformat/vc1test.c
> +++ b/libavformat/vc1test.c
> @@ -34,9 +34,13 @@
>
>  static int vc1t_probe(AVProbeData *p)
>  {
> +int size;
> +
>  if (p->buf_size < 24)
>  return 0;
> -if (p->buf[3] != 0xC5 || AV_RL32(>buf[4]) != 4 ||
AV_RL32(>buf[20]) != 0xC)
> +
> +size = AV_RL32(>buf[4]);
> +if (p->buf[3] != 0xC5 || size < 4 || AV_RL32(>buf[size+16]) !=
0xC)
>  return 0;
>
>  return AVPROBE_SCORE_EXTENSION;
> @@ -48,9 +52,10 @@ static int vc1t_read_header(AVFormatContext *s)
>  AVStream *st;
>  int frames;
>  uint32_t fps;
> +int size;
>
>  frames = avio_rl24(pb);
> -if(avio_r8(pb) != 0xC5 || avio_rl32(pb) != 4)
> +if (avio_r8(pb) != 0xC5 || ((size = avio_rl32(pb)) < 4))
>  return AVERROR_INVALIDDATA;
>
>  /* init video codec */
> @@ -63,6 +68,8 @@ static int vc1t_read_header(AVFormatContext *s)
>
>  if (ff_get_extradata(s, st->codecpar, pb, VC1_EXTRADATA_SIZE) < 0)
>  return AVERROR(ENOMEM);
> +
> +avio_skip(pb, size - 4);
>  st->codecpar->height = avio_rl32(pb);
>  st->codecpar->width = avio_rl32(pb);
>  if(avio_rl32(pb) != 0xC)
> --
> 1.7.1

You may still overread the buffer as the first check on buf_size only
checks for at least 24 bytes. The following p->buf[size+16] may read
beyond the end of the buffer.

Regards,
Jerome
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH V3 1/2] lavf/vc1test: fix vc1test can't probe some RCV file.

2018-10-13 Thread Jun Zhao
case 1:
use the hexdump -C SMM0005.rcv get:
 size  skip (size - 4)
  ||
  VV
  18 00 00 c5 05 00 00 00  4d f1 0a 11 00 e0 01 00
0010  00 d0 02 00 00 0c 00 00  00 88 13 00 00 c0 65 52
 ^
 |
 size + 16
case 2:
same the command for SMM0015.rcv get:
size
  |
  V
  19 00 00 c5 04 00 00 00  41 f3 80 01 40 02 00 00
0010  d0 02 00 00 0c 00 00 00  00 00 00 10 00 00 00 00
  ^
  |
   size + 16

There are different the RCV file format for VC-1, vc1test
just handle the case 2 now, this fix will support the case 1.
(Both of test clips come from: SMPTE Recommended Practice -
VC-1 Decoder and Bitstream Conformance). And I think I got
a older VC-1 test clip in the case 1.

Reviewed-by: Carl Eugen Hoyos 
Reviewed-by: Jerome Borsboom 
Reviewed-by: Michael Niedermayer 
Signed-off-by: Jun Zhao 
Signed-off-by: Yan, FengX 
---
 libavformat/vc1test.c |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c
index a801f4b..e029ff4 100644
--- a/libavformat/vc1test.c
+++ b/libavformat/vc1test.c
@@ -34,9 +34,13 @@
 
 static int vc1t_probe(AVProbeData *p)
 {
+int size;
+
 if (p->buf_size < 24)
 return 0;
-if (p->buf[3] != 0xC5 || AV_RL32(>buf[4]) != 4 || AV_RL32(>buf[20]) 
!= 0xC)
+
+size = AV_RL32(>buf[4]);
+if (p->buf[3] != 0xC5 || size < 4 || AV_RL32(>buf[size+16]) != 0xC)
 return 0;
 
 return AVPROBE_SCORE_EXTENSION;
@@ -48,9 +52,10 @@ static int vc1t_read_header(AVFormatContext *s)
 AVStream *st;
 int frames;
 uint32_t fps;
+int size;
 
 frames = avio_rl24(pb);
-if(avio_r8(pb) != 0xC5 || avio_rl32(pb) != 4)
+if (avio_r8(pb) != 0xC5 || ((size = avio_rl32(pb)) < 4))
 return AVERROR_INVALIDDATA;
 
 /* init video codec */
@@ -63,6 +68,8 @@ static int vc1t_read_header(AVFormatContext *s)
 
 if (ff_get_extradata(s, st->codecpar, pb, VC1_EXTRADATA_SIZE) < 0)
 return AVERROR(ENOMEM);
+
+avio_skip(pb, size - 4);
 st->codecpar->height = avio_rl32(pb);
 st->codecpar->width = avio_rl32(pb);
 if(avio_rl32(pb) != 0xC)
-- 
1.7.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel