[FFmpeg-devel] XVMC Deathmatch

[Diego Biurrun]

On Sun, Feb 15, 2009 at 12:29:18AM +0100, Michael Niedermayer wrote:
> On Sat, Feb 14, 2009 at 11:40:35PM +0100, Diego Biurrun wrote:
> > On Sat, Feb 14, 2009 at 11:30:02PM +0100, Michael Niedermayer wrote:
> > > On Sat, Feb 14, 2009 at 10:38:39PM +0100, Diego Biurrun wrote:
> > > > On Sat, Feb 14, 2009 at 04:53:01PM +0100, Michael Niedermayer wrote:
> > > > > 
> > > > > >    //these are not changed by the decoder!
> > > > > >    int  magic;
> > > > > 
> > > > > 3 points
> > > > > find and get rid of or rename fields to "ununsed123" all unused fields in this
> > > > > struct, dont break ABI!
> > > > 
> > > > All fields are used either in FFmpeg or MPlayer.
> > > > 
> > > > The following are unused in FFmpeg:
> > > > 
> > > >   int             mc_type;
> > > >   int             chroma_format;
> > > >   unsigned int    display_flags;
> > > >   int             state;
> > > >   void*           p_osd_target_surface_render;
> > > > 
> > > > But they are all used in libvo/vo_xvmc.c in MPlayer as can be seen by
> > > > trying to compile against an xvmc.h without those fields:
> > > > 
> > > > libvo/vo_xvmc.c: In function 'xvmc_draw_image':
> > > > libvo/vo_xvmc.c:378: error: 'struct xvmc_render_state' has no member named 'state'
> > > > libvo/vo_xvmc.c: In function 'config':
> > > > libvo/vo_xvmc.c:536: error: 'struct xvmc_render_state' has no member named 'mc_type'
> > > > libvo/vo_xvmc.c:538: error: 'struct xvmc_render_state' has no member named 'chroma_format'
> > > > libvo/vo_xvmc.c: In function 'draw_osd':
> > > > libvo/vo_xvmc.c:899: error: 'struct xvmc_render_state' has no member named 'display_flags'
> > > > libvo/vo_xvmc.c:899: error: 'struct xvmc_render_state' has no member named 'display_flags'
> > > > libvo/vo_xvmc.c:902: error: 'struct xvmc_render_state' has no member named 'state'
> > > > libvo/vo_xvmc.c:903: error: 'struct xvmc_render_state' has no member named 'state'
> > > > libvo/vo_xvmc.c:904: error: 'struct xvmc_render_state' has no member named 'p_osd_target_surface_render'
> > > > [...]
> > > > 
> > > > Does research give points? :)
> > > 
> > > no, you have to remove or rename these fields to unused* to get the points.
> > 
> > Well, I did do 
> > 
> >    int             mc_type;
> > 
> > -->
> > 
> >    int             unused1;
> > 
> > and FFmpeg compiles fine afterwards, but when I put that xvmc.h in my
> > MPlayer tree, I get the above compilation failure.
> > 
> > Thus my conclusion is that the field is not, as you claim, unused.
> > Am I missing something?
> > 
> > > am i allowed to give tips on how to solve such things?
> > 
> > This will likely result in more fixes :)
> 
> well
> first as was said the header was not public thus no rename in the header
> really could break an external APP, removial of a field of course could
> so renaming these to unused123 and placing them under #if no_major_bump_yet
> should not break anything (in theory)

I had all the fields I mentioned above under

#if LIBAVCODEC_VERSION_MAJOR < 53

> yes it means mplayer has to be updated before the next major bump and
> before it could use our installed header instead of its private copy
> 
> but thats mplayers problem to say it harshly ...

Well...
Ivan designed this incestuously.  It's kind of hard to tell if this is a
public or a private header...

I suggest that we assume it is becoming a public header just now and thus
now that Ivan updated MPlayer to not use them anymore, the following two
fields could be safely be removed:

   int mc_type;
   int chroma_format;

OK to remove them?

Diego