[FFmpeg-devel] [PATCH] Incorrect ASF duration

[Michael Niedermayer]

On Sat, Apr 19, 2008 at 02:29:36PM +0200, Henrik Gulbrandsen wrote:
> On Fri, 2008-04-18 at 05:15 +0200, Michael Niedermayer wrote:
> > On Fri, Apr 18, 2008 at 01:05:52AM +0200, Henrik Gulbrandsen wrote:
> > > This is similar to the Ogg problem. The stream duration is not identical
> > > to the PTS of the last packet if the packet duration is non-negligible.
> > > 
> > > The attached examples show the result of
> > > 
> > >     ffmpeg -i video.mp4 -vcodec wmv2 -f asf video.wmv
> > >     ffplay -stats video.wmv > report.txt 2>&1
> > > 
> > > before and after the patch. As before, video.mp4 is exactly 24 seconds
> > > long, at 10 fps. Notice the "Duration: 00:00:23.90" for stats given with
> > > the current code base!
> > 
> > This shows that there is likely a bug in the asf demuxer or asf muxer,
> > what makes you belive it is the muxer that needs correction.
> 
> As I wrote, the stream duration is not identical to the PTS of the last
> packet if the packet duration is non-negligible. This is how the current
> muxer handles it, which indicates that correction is not a bad thing.
> 
> > What i see is a 50% chance of fixing a bug and a 50% chance of introducing
> > a new bug instead.
> 
> And a 50% chance of doing both at the same time :-)
> 
> There may of course still be an even number of mutually negating bugs
> left in the code, but I think this is a step in the right direction.
> However, the original patch did introduce an indexing bug, since I
> happened to miss that the duration variable was also used further down
> in the same function. An updated patch is attached.
> 
> > I suggest you use some "official" asf encoder and encode a video of known
> > length and then check what is written in the file.
> 
> I couldn't bring myself to read the EULA of an official encoder, but for
> what it's worth, both my phone and Windows Media Player gives the length
> of the WMV file created by the original code as 00:23. After my patch,
> the length is given as 00:24, so I think demuxer bugs are irrelevant...
> 
> > Also i suspect that the regression test checksums will change by such
> > a change to the muxer thus would need to be updated by your patch.
> 
> A separate patch with test changes is attached.

patches look ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080419/d010fbe9/attachment.pgp>