[FFmpeg-devel] XVMC Deathmatch

[Ivan Kalvachev]

On 2/15/09, Diego Biurrun <diego at biurrun.de> 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.

If it is installed, it is public header.
You have to keep compatibility.
This is what I mean.