[FFmpeg-cvslog] r17314 - in trunk/libavcodec: mpegvideo_xvmc.c xvmc.h

[Reimar Döffinger]

On Sun, Feb 15, 2009 at 01:26:56PM +0200, Ivan Kalvachev wrote:
> On 2/15/09, Robert Swain <robert.swain at gmail.com> wrote:
> > 2009/2/15 Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> >> On Sun, Feb 15, 2009 at 09:42:11AM +0100, iive wrote:
> >>> Log:
> >>> Rename xvmc_render_state to something with pixfmt in its name.
> >>
> >> IMHO "something" describes it quite well.
> >>
> >>> -    struct xvmc_render_state *render = (struct
> >>> xvmc_render_state*)s->current_picture.data[2];
> >>> +    struct xvmc_pixfmt_render *render = (struct
> >>> xvmc_pixfmt_render*)s->current_picture.data[2];
> >>
> >> I can't see that that name would be any better, the struct neither is,
> >> nor describes it a PixelFormat Render (I am quite sure that "render" is
> >> not
> >> even an English word, at most it could be a renderer).
> >
> > 'Render' is usually a verb. 'Renderer' is a noun.
> >
> > I don't understand what the struct name 'xvmc_pixfmt_render' means. At
> > least before it told you that the struct contained state variables
> > relating to xvmc rendering. Now it's nonsense. I don't actually see
> > what that struct or that file (xvmc.h) has to do with pixel formats
> > either. It seems to contain variables relating to other stuff so I
> > don't see why it should be named with pixfmt in it. But I haven't been
> > paying much attention to XvMC.
> 
> I also prefer the old naming of struct and header file. After all I've
> made them up.
> But other people say otherwise.

Well, first that does not make "render" a proper noun (and it is used as
one IMO).
Secondly, while the previous name made some sense to the uninitiated,
I somewhat doubt that the obvious interpretation of
"state variables relating to xvmc rendering" is really correct. At most
part of it is state, and IIRC only the part that should not be there in
the first place (since FFmpeg does not use it) is state.
Lastly, ..._pixfmt was the name suggestion for the header, which makes
some sense because you need it whenever you use the xmvc pixfmt and it
describes (partially) what that pixfmt looks like.
However, that struct is only used as part of the pixfmt, and the word
"pixfmt" describes neither what the struct contains nor what its purpose
is, only what it is used for. By that logic, AVCodecContext would be
called AVDecoding.

Greetings,
Reimar D?ffinger