| Message ID | 1539351665-18001-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
2018-10-12 15:41 GMT+02:00, Jun Zhao <mypopydev@gmail.com>: > case 1: > use the hexdump -C SMM0005.rcv get: > size skip (size - 4) > | | > V V > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > 00000010 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 > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > 00000010 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 with and without your patch, I get identical output from FFmpeg's decoder for both SSM0015.rcv and SMM0005.rcv. What exactly is your patch supposed to change? $ md5sum SMM0005.rcv abd0fa5ec90d44f7b4865c6930d65ff2 SMM0005.rcv $ md5sum SSM0015.rcv 21a0281f83b7f2e99a048b180dd3347f SSM0015.rcv The following does not change: $ ffmpeg -i SMM0005.rcv -f crc - ffmpeg version N-92147-gf85fa10 Copyright (c) 2000-2018 the FFmpeg developers built with gcc 6.4.0 (GCC) configuration: --enable-gpl --enable-gnutls --enable-libxml2 libavutil 56. 19.101 / 56. 19.101 libavcodec 58. 32.100 / 58. 32.100 libavformat 58. 18.104 / 58. 18.104 libavdevice 58. 4.105 / 58. 4.105 libavfilter 7. 33.100 / 7. 33.100 libswscale 5. 2.100 / 5. 2.100 libswresample 3. 2.100 / 3. 2.100 libpostproc 55. 2.100 / 55. 2.100 Input #0, vc1test, from 'SMM0005.rcv': Duration: 00:00:01.00, bitrate: 7472 kb/s Stream #0:0: Video: wmv3 (Main), yuv420p, 720x480, 24 fps, 24 tbr, 24 tbn, 24 tbc Stream mapping: Stream #0:0 -> #0:0 (wmv3 (native) -> rawvideo (native)) Press [q] to stop, [?] for help Output #0, crc, to 'pipe:': Metadata: encoder : Lavf58.18.104 Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p, 720x480, q=2-31, 99532 kb/s, 24 fps, 24 tbn, 24 tbc Metadata: encoder : Lavc58.32.100 rawvideo SMM0005.rcv: Input/output error Last message repeated 1 times CRC=0xe3b2671c frame= 24 fps=0.0 q=-0.0 Lsize= 0kB time=00:00:01.00 bitrate= 0.1kbits/s speed=14.8x video:12150kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown What exactly is your patch supposed to improve? Thank you, Carl Eugen (05 looks different here.)
On Fri, Oct 12, 2018 at 10:35 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2018-10-12 15:41 GMT+02:00, Jun Zhao <mypopydev@gmail.com>: > > case 1: > > use the hexdump -C SMM0005.rcv get: > > size skip (size - 4) > > | | > > V V > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > > 00000010 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 > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > > 00000010 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 with and without your patch, I get identical output from > FFmpeg's decoder for both SSM0015.rcv and SMM0005.rcv. > What exactly is your patch supposed to change? > > $ md5sum SMM0005.rcv > abd0fa5ec90d44f7b4865c6930d65ff2 SMM0005.rcv > $ md5sum SSM0015.rcv > 21a0281f83b7f2e99a048b180dd3347f SSM0015.rcv In my test clips, I get md5 like this $ md5sum SMM0005.rcv 39eb225dbfafacea3e3737deba789530 SMM0005.rcv $ md5sum SMM0015.rcv 78e45e50079e143e69f69d677c37ac3b SMM0015.rcv > > > The following does not change: > > $ ffmpeg -i SMM0005.rcv -f crc - > ffmpeg version N-92147-gf85fa10 Copyright (c) 2000-2018 the FFmpeg developers > built with gcc 6.4.0 (GCC) > configuration: --enable-gpl --enable-gnutls --enable-libxml2 > libavutil 56. 19.101 / 56. 19.101 > libavcodec 58. 32.100 / 58. 32.100 > libavformat 58. 18.104 / 58. 18.104 > libavdevice 58. 4.105 / 58. 4.105 > libavfilter 7. 33.100 / 7. 33.100 > libswscale 5. 2.100 / 5. 2.100 > libswresample 3. 2.100 / 3. 2.100 > libpostproc 55. 2.100 / 55. 2.100 > Input #0, vc1test, from 'SMM0005.rcv': > Duration: 00:00:01.00, bitrate: 7472 kb/s > Stream #0:0: Video: wmv3 (Main), yuv420p, 720x480, 24 fps, 24 tbr, > 24 tbn, 24 tbc > Stream mapping: > Stream #0:0 -> #0:0 (wmv3 (native) -> rawvideo (native)) > Press [q] to stop, [?] for help > Output #0, crc, to 'pipe:': > Metadata: > encoder : Lavf58.18.104 > Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p, > 720x480, q=2-31, 99532 kb/s, 24 fps, 24 tbn, 24 tbc > Metadata: > encoder : Lavc58.32.100 rawvideo > SMM0005.rcv: Input/output error > Last message repeated 1 times > CRC=0xe3b2671c > frame= 24 fps=0.0 q=-0.0 Lsize= 0kB time=00:00:01.00 bitrate= > 0.1kbits/s speed=14.8x > video:12150kB audio:0kB subtitle:0kB other streams:0kB global > headers:0kB muxing overhead: unknown > > What exactly is your patch supposed to improve? > > Thank you, Carl Eugen > > (05 looks different here.)
On Sat, Oct 13, 2018 at 12:55 AM mypopy@gmail.com <mypopy@gmail.com> wrote: > > On Fri, Oct 12, 2018 at 10:35 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > > > 2018-10-12 15:41 GMT+02:00, Jun Zhao <mypopydev@gmail.com>: > > > case 1: > > > use the hexdump -C SMM0005.rcv get: > > > size skip (size - 4) > > > | | > > > V V > > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > > > 00000010 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 > > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > > > 00000010 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 with and without your patch, I get identical output from > > FFmpeg's decoder for both SSM0015.rcv and SMM0005.rcv. > > What exactly is your patch supposed to change? > > > > $ md5sum SMM0005.rcv > > abd0fa5ec90d44f7b4865c6930d65ff2 SMM0005.rcv > > $ md5sum SSM0015.rcv > > 21a0281f83b7f2e99a048b180dd3347f SSM0015.rcv > > In my test clips, I get md5 like this > $ md5sum SMM0005.rcv > 39eb225dbfafacea3e3737deba789530 SMM0005.rcv > $ md5sum SMM0015.rcv > 78e45e50079e143e69f69d677c37ac3b SMM0015.rcv RCV format have a different version, you can google the rcv1 rcv2 for VC-1. (e,g: http://www.ti.com/lit/ml/sprs363a/sprs363a.pdf), I think we use a different RCV format for test clips SMM0005.rcv > > > > > > The following does not change: > > > > $ ffmpeg -i SMM0005.rcv -f crc - > > ffmpeg version N-92147-gf85fa10 Copyright (c) 2000-2018 the FFmpeg developers > > built with gcc 6.4.0 (GCC) > > configuration: --enable-gpl --enable-gnutls --enable-libxml2 > > libavutil 56. 19.101 / 56. 19.101 > > libavcodec 58. 32.100 / 58. 32.100 > > libavformat 58. 18.104 / 58. 18.104 > > libavdevice 58. 4.105 / 58. 4.105 > > libavfilter 7. 33.100 / 7. 33.100 > > libswscale 5. 2.100 / 5. 2.100 > > libswresample 3. 2.100 / 3. 2.100 > > libpostproc 55. 2.100 / 55. 2.100 > > Input #0, vc1test, from 'SMM0005.rcv': > > Duration: 00:00:01.00, bitrate: 7472 kb/s > > Stream #0:0: Video: wmv3 (Main), yuv420p, 720x480, 24 fps, 24 tbr, > > 24 tbn, 24 tbc > > Stream mapping: > > Stream #0:0 -> #0:0 (wmv3 (native) -> rawvideo (native)) > > Press [q] to stop, [?] for help > > Output #0, crc, to 'pipe:': > > Metadata: > > encoder : Lavf58.18.104 > > Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p, > > 720x480, q=2-31, 99532 kb/s, 24 fps, 24 tbn, 24 tbc > > Metadata: > > encoder : Lavc58.32.100 rawvideo > > SMM0005.rcv: Input/output error > > Last message repeated 1 times > > CRC=0xe3b2671c > > frame= 24 fps=0.0 q=-0.0 Lsize= 0kB time=00:00:01.00 bitrate= > > 0.1kbits/s speed=14.8x > > video:12150kB audio:0kB subtitle:0kB other streams:0kB global > > headers:0kB muxing overhead: unknown > > > > What exactly is your patch supposed to improve? > > > > Thank you, Carl Eugen > > > > (05 looks different here.)
> On Sat, Oct 13, 2018 at 12:55 AM mypopy at gmail.com <mypopy at gmail.com> wrote: >> >> On Fri, Oct 12, 2018 at 10:35 PM Carl Eugen Hoyos <ceffmpeg at gmail.com> > wrote: >> > >> > 2018-10-12 15:41 GMT+02:00, Jun Zhao <mypopydev at gmail.com>: >> > > case 1: >> > > use the hexdump -C SMM0005.rcv get: >> > > size skip (size - 4) >> > > | | >> > > V V >> > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 >> > > 00000010 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 >> > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 >> > > 00000010 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 with and without your patch, I get identical output from >> > FFmpeg's decoder for both SSM0015.rcv and SMM0005.rcv. >> > What exactly is your patch supposed to change? >> > >> > $ md5sum SMM0005.rcv >> > abd0fa5ec90d44f7b4865c6930d65ff2 SMM0005.rcv >> > $ md5sum SSM0015.rcv >> > 21a0281f83b7f2e99a048b180dd3347f SSM0015.rcv >> >> In my test clips, I get md5 like this >> $ md5sum SMM0005.rcv >> 39eb225dbfafacea3e3737deba789530 SMM0005.rcv >> $ md5sum SMM0015.rcv >> 78e45e50079e143e69f69d677c37ac3b SMM0015.rcv > RCV format have a different version, you can google the rcv1 rcv2 for VC-1. > (e,g: > http://www.ti.com/lit/ml/sprs363a/sprs363a.pdf), I think we use a different > RCV format for test clips SMM0005.rcv This has nothing to do with the version. Version is coded as the sixth bit in the initial byte, i.e. 0x85 = version 1, 0xc5 = version 2. VC-1(2006) is quite clear in Annex L that the second dword should be a 0x00000004. The reference decoder, however, is more lenient and gives more insight. The 0x00000004, and the 0x0000000c in the seventh dword, seem to be meant as a length indicator for the STRUCT_C and the STRUCT_B, respectively. This may be a leftover from older versions. I think it won't hurt to follow the reference decoder here and interpret these dwords as length indicators. The patch could be adjusted to do this for the 0x0000000c as well. Regards, Jerome
On Sat, Oct 13, 2018 at 4:51 PM Jerome Borsboom <jerome.borsboom@carpalis.nl> wrote: > > > On Sat, Oct 13, 2018 at 12:55 AM mypopy at gmail.com <mypopy at gmail.com> wrote: > >> > >> On Fri, Oct 12, 2018 at 10:35 PM Carl Eugen Hoyos <ceffmpeg at gmail.com> > > wrote: > >> > > >> > 2018-10-12 15:41 GMT+02:00, Jun Zhao <mypopydev at gmail.com>: > >> > > case 1: > >> > > use the hexdump -C SMM0005.rcv get: > >> > > size skip (size - 4) > >> > > | | > >> > > V V > >> > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > >> > > 00000010 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 > >> > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > >> > > 00000010 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 with and without your patch, I get identical output from > >> > FFmpeg's decoder for both SSM0015.rcv and SMM0005.rcv. > >> > What exactly is your patch supposed to change? > >> > > >> > $ md5sum SMM0005.rcv > >> > abd0fa5ec90d44f7b4865c6930d65ff2 SMM0005.rcv > >> > $ md5sum SSM0015.rcv > >> > 21a0281f83b7f2e99a048b180dd3347f SSM0015.rcv > >> > >> In my test clips, I get md5 like this > >> $ md5sum SMM0005.rcv > >> 39eb225dbfafacea3e3737deba789530 SMM0005.rcv > >> $ md5sum SMM0015.rcv > >> 78e45e50079e143e69f69d677c37ac3b SMM0015.rcv > > RCV format have a different version, you can google the rcv1 rcv2 for VC-1. > > (e,g: > > http://www.ti.com/lit/ml/sprs363a/sprs363a.pdf), I think we use a different > > RCV format for test clips SMM0005.rcv > > This has nothing to do with the version. Version is coded as the sixth > bit in the initial byte, i.e. 0x85 = version 1, 0xc5 = version 2. > > VC-1(2006) is quite clear in Annex L that the second dword should be a > 0x00000004. The reference decoder, however, is more lenient and gives > more insight. The 0x00000004, and the 0x0000000c in the seventh dword, > seem to be meant as a length indicator for the STRUCT_C and the > STRUCT_B, respectively. > > This may be a leftover from older versions. I think it won't hurt to > follow the reference decoder here and interpret these dwords as length > indicators. The patch could be adjusted to do this for the 0x0000000c as > well. > I agree with your, I think I get a older VC-1 test clip in this case. > > Regards, > Jerome
On Fri, Oct 12, 2018 at 09:41:04PM +0800, Jun Zhao wrote: > case 1: > use the hexdump -C SMM0005.rcv get: > size skip (size - 4) > | | > V V > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > 00000010 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 > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > 00000010 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: RP 228:2008 - SMPTE > Recommended Practice - VC-1 Decoder and Bitstream Conformance). > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > Signed-off-by: Yan, FengX <fengx.yan@intel.com> > --- > libavformat/vc1test.c | 11 +++++++++-- > 1 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c > index a801f4b..82e155a 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(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) > + > + size = AV_RL32(&p->buf[4]); > + if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[size+16]) != 0xC) this doesnt check size and could crash [...]
On Sun, Oct 14, 2018 at 3:07 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Fri, Oct 12, 2018 at 09:41:04PM +0800, Jun Zhao wrote: > > case 1: > > use the hexdump -C SMM0005.rcv get: > > size skip (size - 4) > > | | > > V V > > 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 > > 00000010 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 > > 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 > > 00000010 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: RP 228:2008 - SMPTE > > Recommended Practice - VC-1 Decoder and Bitstream Conformance). > > > > Signed-off-by: Jun Zhao <jun.zhao@intel.com> > > Signed-off-by: Yan, FengX <fengx.yan@intel.com> > > --- > > libavformat/vc1test.c | 11 +++++++++-- > > 1 files changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c > > index a801f4b..82e155a 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(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) > > + > > + size = AV_RL32(&p->buf[4]); > > + if (p->buf[3] != 0xC5 || AV_RL32(&p->buf[size+16]) != 0xC) > > this doesnt check size and could crash > Will add size check in V3 > > [...]
diff --git a/libavformat/vc1test.c b/libavformat/vc1test.c index a801f4b..82e155a 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(&p->buf[4]) != 4 || AV_RL32(&p->buf[20]) != 0xC) + + size = AV_RL32(&p->buf[4]); + if (p->buf[3] != 0xC5 || AV_RL32(&p->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)
case 1: use the hexdump -C SMM0005.rcv get: size skip (size - 4) | | V V 00000000 18 00 00 c5 05 00 00 00 4d f1 0a 11 00 e0 01 00 00000010 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 00000000 19 00 00 c5 04 00 00 00 41 f3 80 01 40 02 00 00 00000010 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: RP 228:2008 - SMPTE Recommended Practice - VC-1 Decoder and Bitstream Conformance). Signed-off-by: Jun Zhao <jun.zhao@intel.com> Signed-off-by: Yan, FengX <fengx.yan@intel.com> --- libavformat/vc1test.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)