[FFmpeg-cvslog] r25931 - trunk/libavformat/asfdec.c

[Reimar Döffinger]

On Wed, Dec 15, 2010 at 02:46:34PM +0100, Michael Niedermayer wrote:
> > > > > Did somebody change his mind ?
> > > > 
> > > > No. It's just that in this specific case it is a pain to do because the
> > > > demuxer needs to juggle the data around and that's not something you
> > > > can do if you have only half of it.
> > > > And I am too lazy to think about if and when you could still do
> > > > something reasonable.
> > > 
> > > then the end of the packet can be filled with zeros.
> > > we seem to be loosing significant portions of valid audio with this and i
> > > dont think this is good
> > 
> > I really don't like making up data, also the amount of audio lost is not
> > sufficient to actually hear the difference (well, for me).
> > Though I admit I really wouldn't mind a nicer solution, especially since
> > I think the descrambling code in some cases is a NOP.
> 
> if the descrambling is not a NOP random bytes in the middle of a packet are lost
> some audio codecs are designed to handle this, its annoying if the packet is
> droped IMHO

And I think it is annoying if we return bogus data without the slightest hint,
which AFAICT is the only alternative.

> > Problem is just it takes extra time to come up with a solution I'd consider
> > good and I don't have that much time I'd like to spend on this...
> 
> thats ok but i think then you should leave the code as it was if you lack time
> and not commit a hack

Sorry, but I consider that idiotic. You seriously consider it better to leave 
code around that returns unintialized data, gives no hint about the file being
broken and all with a fate test that depends on that unintialized data and thus
only work by pure chance than a "hack" where the only fault is that it is not optimal
because it drops a packet where the alternatives are only slightly better anyway?
But anyway I made a (not well tested, and I still do not think it is good to pad
with 0s for the scrambling case) patch to do it "properly", but it proved
to be a complete waste of time, because in this case scrambling is not used but
also our WMA decoder will not decode anything at all if it gets a only partial
packet.
Actually it will not even print a warning!
See this code in wmadec.c:
    if (buf_size < s->block_align)
        return 0;

Index: libavformat/asfdec.c
===================================================================
--- libavformat/asfdec.c        (revision 25943)
+++ libavformat/asfdec.c        (working copy)
@@ -953,12 +953,21 @@
 
         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
+                memset(asf_st->pkt.data + asf->packet_frag_offset + ret, 0,
+                       asf->packet_frag_size - ret);
+            } 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