[FFmpeg-devel] [PATCH] 1/6 Remove incorrect APE tag size adjustment based on flags

[Kostya]

On Tue, Aug 11, 2009 at 12:11:31AM +0300, Matti Hamalainen wrote:
> On Mon, 10 Aug 2009, Kostya wrote:
> 
> > On Mon, Aug 10, 2009 at 12:41:24PM +0300, Matti Hamalainen wrote:
> > > On Mon, 10 Aug 2009, Matti Hamalainen wrote:
> > > 
> > > > Remove incorrect tag size adjustment based on flags, the size already
> > > > includes the whole tag, including footer.
> > > 
> > > Urgh, with the patch this time too .. this is embarrassing.
> > 
> > Can you provide a sample to test this all?
> 
> Actually the Musepack sample files already in MPlayer sample archive are 
> are applicable here. However, as they are MPC files, the other patches
> need to be applied too for testing.
> 
> http://samples.mplayerhq.hu/A-codecs/musepack/
> 
> At least "03-Take A Chance On Me.mpc" contains a v1.x APE tag, which has
> "contains a header" bit set. The current (incorrect) code would in this 
> case seek to (file_size - tag_size - 2*APE_TAG_FOOTER_BYTES), which
> is the first byte of the tag header "APETAGEX" string. But the tag item 
> reading code expects the position to be on the first tag item.
> 
> http://wiki.hydrogenaudio.org/index.php?title=APE_Tags_Header                                                        
> states that "Tag size in bytes including footer and all tag items                                                          
> excluding the header to be as compatible as possible with APE Tags 1.000".                                                 

I prefer verifying against MAC source code but they seem to agree on
that (along other things). 

> There are also some APE files, which can be used to verify that this
> change (or all of the patches together) introduce no regressions as
> far as I am aware. Links to APE samples:
> 
> http://samples.mplayerhq.hu/A-codecs/lossless/luckynight.ape
> http://samples.mplayerhq.hu/monkeyaudio/sh3.ape
 
Yep, using these to verify.

Oh, and this patch was applied.