[FFmpeg-cvslog] r26053 - in trunk: libavformat/asfdec.c tests/ref/fate/wmv8-drm

[Reimar Döffinger]

On Sat, Dec 18, 2010 at 08:32:41PM +0100, Vitor Sessak wrote:
> On 12/18/2010 06:19 PM, Reimar D?ffinger wrote:
> >On Sat, Dec 18, 2010 at 10:43:38AM -0500, Ronald S. Bultje wrote:
> >>On Sat, Dec 18, 2010 at 8:18 AM, reimar<subversion at mplayerhq.hu>  wrote:
> >>>Author: reimar
> >>>Date: Sat Dec 18 14:18:52 2010
> >>>New Revision: 26053
> >>>
> >>>Log:
> >>>Change ASF demuxer to return incomplete last packets.
> >>>Whether the behaviour for streams using scrambling makes sense
> >>>is unclear.
> >>>
> >>>Modified:
> >>>   trunk/libavformat/asfdec.c
> >>>   trunk/tests/ref/fate/wmv8-drm
> >>>
> >>>Modified: trunk/libavformat/asfdec.c
> >>>==============================================================================
> >>>--- trunk/libavformat/asfdec.c  Sat Dec 18 06:15:32 2010        (r26052)
> >>>+++ trunk/libavformat/asfdec.c  Sat Dec 18 14:18:52 2010        (r26053)
> >>>@@ -953,12 +953,24 @@ static int ff_asf_parse_packet(AVFormatC
> >>>
> >>>         ret = get_buffer(pb, asf_st->pkt.data + asf->packet_frag_offset,
> >>>                          asf->packet_frag_size);
> >>>-        if (ret != asf->packet_frag_size)
> >>>-            return ret>= 0 ? AVERROR_EOF : ret;
> >>>+        if (ret != asf->packet_frag_size) {
> >>>+            if (ret<  0 || asf->packet_frag_offset + ret == 0)
> >>>+                return ret<  0 ? ret : AVERROR_EOF;
> >>>+            if (asf_st->ds_span>  1) {
> >>>+                // scrambling, we can either drop it completely or fill the remainder
> >>>+                // TODO: should we fill the whole packet instead of just the current
> >>>+                // fragment?
> >>>+                memset(asf_st->pkt.data + asf->packet_frag_offset + ret, 0,
> >>>+                       asf->packet_frag_size - ret);
> >>>+                ret = asf->packet_frag_size;
> >>>+            } else
> >>>+                // no scrambling, so we can return partial packets
> >>>+                av_shrink_packet(&asf_st->pkt, asf->packet_frag_offset + ret);
> >>>+        }
> >>>         if (s->key&&  s->keylen == 20)
> >>>             ff_asfcrypt_dec(s->key, asf_st->pkt.data + asf->packet_frag_offset,
> >>>-                            asf->packet_frag_size);
> >>>-        asf_st->frag_offset += asf->packet_frag_size;
> >>>+                            ret);
> >>>+        asf_st->frag_offset += ret;
> >>>         /* test if whole packet is read */
> >>>         if (asf_st->frag_offset == asf_st->pkt.size) {
> >>>             //workaround for macroshit radio DVR-MS files
> >>>
> >>>Modified: trunk/tests/ref/fate/wmv8-drm
> >>>==============================================================================
> >>>--- trunk/tests/ref/fate/wmv8-drm       Sat Dec 18 06:15:32 2010        (r26052)
> >>>+++ trunk/tests/ref/fate/wmv8-drm       Sat Dec 18 14:18:52 2010        (r26053)
> >>>@@ -160,3 +160,4 @@
> >>>  0, 596250, 84480, 0xbce22331
> >>>  0, 600000, 84480, 0x020545d7
> >>>  0, 603750, 84480, 0x71869e48
> >>>+0, 607500, 84480, 0x5befc578
> >>
> >>Various fate platforms fail te wmv8-drm test now, e.g. x86-32 (but
> >>x86-64 works fine).
> >
> >This time unfortunately the issue is indeed that the VC-1 decoder reads outside
> >the packet size.
> >Below hack would fix it, but that is of course not ok.
> >My proposal would be to remove that fate test an only keep the one I added
> >(does not decode), unless someone comes up with a way to fix the VC-1 decoder...
> 
> This is not ideal, since the coverage of the VC-1 decoder is far
> from great in FATE already, and decoding one file less will only
> make it worse. Can't you re-cut the file to remove the incomplete
> frame?

Yes (though honestly it's not trivial at all IMO), but then we do not have
a test-case for neither the incomplete-last-frame nor that combined with
encryption case.
Why not add more VC-1 test-case if you think we do not have enough?
We have some samples, and it is still common enough on e.g. Blu-ray 
(as well as the encoder beeing freely available to any Windows user)
that it should be possible to get a few more if needed...
I think there should even be a "showcase" full of samples somewhere...