[FFmpeg-devel] [FFMPEG] [PATCH] cavs encoder

Diego Biurrun diego
Wed Nov 18 15:39:10 CET 2009


On Tue, Nov 17, 2009 at 05:33:47PM +0800, zhihang wang wrote:
> 
> On Tue, Nov 17, 2009 at 2:24 AM, Diego Biurrun <diego at biurrun.de> wrote:
> > > +static int (*me)(AVSContext *h, enum cavs_block blk_sz, int blk_idx,
> > > +                 int ref, cavs_vector* pmv, cavs_vector* mv);
> > > +static int (*bi_me)(AVSContext *h, enum cavs_block blk_sz, int blk_idx,
> > > +                 int ref, cavs_vector* pmv, cavs_vector* mv);
> >
> > Again, please avoid forward declarations.
> >
> *Sorry. because these are two function pointers, I don't know how to use
> them without forward declarations Would you like to give me some ideas?*

Ignore my comment, I misread.

> > > +        if ((level > CAVS_MAX_LEVEL)||(run > CAVS_MAX_RUN)
> > > +                ||((level_code = enc->rlcode[run][level]) < 0)) {
> >
> > alignment
> 
> *Sorry. I don't know the alignment rule.  Is the following is ok?*
> > +        if ((level > CAVS_MAX_LEVEL)||(run > CAVS_MAX_RUN)
> > +             ||((level_code = enc->rlcode[run][level]) < 0))  {

No.  You are missing some spaces and the columns should align at the
first character after the '('.  Here is how it should look like:

    if ((level > CAVS_MAX_LEVEL) || (run > CAVS_MAX_RUN)
        || ((level_code = enc->rlcode[run][level]) < 0)) {

> > > +static void partition_copy_c(uint8_t *src, int src_stride, uint8_t *dst,
> > int dst_stride, int w, int h)
> >
> > Please break lines over 80 characters where easily possible.
> 
> *Do I need to break ALL lines over 80 characters?*

No.  But break those where it is sensible and easily possible, like the
above example.

Diego



More information about the ffmpeg-devel mailing list