[FFmpeg-devel] XVMC Deathmatch

[Diego Biurrun]

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.

Diego