[MPlayer-dev-eng] [PATCH] EOSD/ASS code factorization

[Nicolas George]

Le quintidi 5 fructidor, an CCXVIII, Diego Biurrun a écrit :
> no author?
> K&R style for new files please, i.e. place a space after if/for/while.
> Indentation is off.

Changed.

> > +static void eosd_ass_uninit(mp_eosd_source_t *src)
> > +{
> > +    (void)src;
> > +    ass_renderer_done(ass_renderer);
> Maybe I'm talking nonsense because I dunno the surrounding code,
> but why the cast?  And src looks unused..

src is required to fit the prototype for the ->uninit field, but not needed
by the code of the function. Some other uninit callbacks may need it.
"(void)src" is a way I picked up a few years ago to make that fact explicit
both to the reader and the compiler (it silences gcc warning without
altering the code).

If you do not like it, I can remove it altogether, or replace it by
something more explicit:

	(void)src; /* required by the prototype, unused */

> Break this line please.

Changed. That was from the old code.

> The _t namespace is reserved for POSIX, please don't pollute it further.
> Same, what's so bad about writing 'struct'?

There was already a typedef eosd_res_t, it seemed consistent that way.
Changed all.

> > +static void overlay_update(mp_eosd_source_t *src, const mp_eosd_settings_t *res,
> > +    double ts);
> ugly forward declaration

Apart from the indentation (fixed), what is ugly about it? overlay_update
and overlay_source are mutually recursive. To avoid the forward declaration,
it would be necessary to init overlay_source->update in overlay_add.

> Indentation is off.

Changed.

> > +    ASS_Image *img, **prev;
> > +    (void)src;
> > +    (void)res;
> > +    (void)ts;
> Why the casts?

Same as above: to make it explicit they are unused.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-eosd-01-strtol-20100822-1213.diff
Type: text/x-diff
Size: 656 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100822/ed95b4b5/attachment-0004.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-eosd-02-factor-20100822-1213.diff
Type: text/x-diff
Size: 11835 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100822/ed95b4b5/attachment-0005.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-eosd-03-dynamic-20100822-1213.diff
Type: text/x-diff
Size: 9888 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100822/ed95b4b5/attachment-0006.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-eosd-04-command-20100822-1213.diff
Type: text/x-diff
Size: 5026 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100822/ed95b4b5/attachment-0007.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100822/ed95b4b5/attachment-0001.pgp>