[FFmpeg-devel] XVMC Deathmatch

[Diego Biurrun]

On Sun, Feb 15, 2009 at 01:01:36AM +0100, Diego Biurrun wrote:
> On Sun, Feb 15, 2009 at 01:56:49AM +0200, Ivan Kalvachev wrote:
> > On 2/15/09, Diego Biurrun <diego at biurrun.de> wrote:
> > > 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...
> > 
> > Well, don't tell me I didn't had reasons to hold your
> > plans on making it public header.
> 
> You held that plan back for many years.  Without this deathmatch, you
> would not have made those changes to MPlayer.
> 
> > > 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?
> > 
> > You were the one who insisted on installing it.
> > Deal with the consequences.
> 
> I take that as a "I don't care", which I will now interpret to mean I
> have a free ticket to go ahead.

Note to Michael: I know this breaks ABI and API.  I'm operating under the
assumption that the API is still in flux and the only user is MPlayer,
which has already been updated.

So don't go subtracting points from me :)

Diego