[FFmpeg-devel] fate: Do not report side data size

[Michael Niedermayer]

On Wed, Mar 08, 2017 at 01:12:04PM +0100, wm4 wrote:
> On Wed, 8 Mar 2017 13:04:11 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> > On Wed, Mar 08, 2017 at 12:56:31PM +0100, wm4 wrote:
> > > On Wed, 8 Mar 2017 12:51:06 +0100
> > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > >   
> > > > On Wed, Mar 08, 2017 at 12:28:17PM +0100, Hendrik Leppkes wrote:  
> > > > > On Wed, Mar 8, 2017 at 1:17 AM, Vittorio Giovara
> > > > > <vittorio.giovara at gmail.com> wrote:    
> > > > > > This should address the mismatch between different archs    
> > > > 
> > > > iam not in favor of this solution
> > > > 
> > > >   
> > > > > 
> > > > > Removing the side_data_size from output should be fine, as its a
> > > > > implementation detail and as seen here can even vary between
> > > > > architecture or possibly even compiler.
> > > > > Maybe someone that uses that ffprobe output more often can comment?    
> > > > 
> > > > I use all kinds of stuff
> > > > if something is removed from ffprobes output it wont be tested anymore.
> > > > We should test more not less.
> > > > 
> > > >   
> > > > > 
> > > > > An alternative for fixing fate would be to use fixed size fields in
> > > > > the new sidedata, although the possibility of it breaking similarly
> > > > > again in the future then remains.    
> > > > 
> > > > I strongly prefer fixed size to be used in side data over platform
> > > > dependant fields. Not only does size become testable but theres also
> > > > a platform specific difference less in the interface which should
> > > > help bug reproducability between platforms
> > > > 
> > > > thanks
> > > > 
> > > > [...]  
> > >   
> > 
> > > So why don't let we fate test e.g. sizeof(AVPacket)? Makes as much
> > > sense.  
> > 
> > this is trolling
> 
> Really, I'm not sure if your first post in this thread isn't trolling.
> 
> > No change in our code could have sizeof(AVPacket) be "wrong"
> > but a
> > int side_data_size
> > can be wrong easily
> 
> Side datas are generally structs (well, some aren't, but most newer
> ones). How does it make sense to "test" the sizes of those structs?

the "int side_data_size" matches the stuct sizeof only if it was
set correctly. This field can be read from the main data of a AVPacket
and in that case can be anything its also possible a simple typo in
the code could set it incorrectly or linking to a old lib with a
struct with fewer fields could cause it to be incorrect.

Theres alot that can make the field wrong.
displaying the size in the output and thus testing it is easy and
usefull IMHO


> 
> If you want to do it right, then dump the side data contents with
> ffprobe or whatever.

Thats already done for some side data, but peple do not update the code
when new cases are added. Also until now the size was platform
independant in practice.

If the size printing is removed then other code should be added
to test for the size to match the correct value


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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170308/3c901dd3/attachment.sig>