Diego, i post it on [GCI] yesterday 18 min before deadline, but it`s no
problem to fix it all... Please reply me to know what to do, i think to get
one more file for cleaning if there`s available on GCI, because i want to
learn about clean writing a code but first need to close the previous one,
thx :)

Поздрави, Асен Леков
Gmail: [email protected]
Hotmail & MSN: [email protected]
GSM: +359 898 66 13 13



On Wed, Nov 30, 2011 at 11:06 AM, Diego Biurrun <[email protected]> wrote:

> On Wed, Nov 30, 2011 at 12:13:17AM +0200, Asen Lekov wrote:
> >
> > --- a/libavformat/nutdec.c
> > +++ b/libavformat/nutdec.c
> > @@ -33,59 +33,73 @@
> >
> > -    if(maxlen)
> > -        string[FFMIN(len, maxlen-1)]= 0;
> > +    if (maxlen)
> > +        string[FFMIN(len, maxlen-1)] = 0;
>
> Please give the operators some room to breathe.
>
> >  #ifdef TRACE
> > -static inline uint64_t get_v_trace(AVIOContext *bc, char *file, char
> *func, int line){
> > +static inline uint64_t get_v_trace(AVIOContext *bc, char *file, char
> *func, int line)
> > +{
>
> This long line could easily be broken, more below.
>
> > @@ -113,17 +129,22 @@ static int get_packetheader(NUTContext *nut,
> AVIOContext *bc, int calculate_chec
> >
> > -    if(pos >= 0)
> > -        avio_seek(bc, pos, SEEK_SET); //note, this may fail if the
> stream is not seekable, but that should not matter, as in this case we
> simply start where we currently are
> > +    if (pos >= 0)
> > +        avio_seek(bc, pos, SEEK_SET);
> > +/*
> > +  note, this may fail if the stream is not seekable, but that should
> not matter,
> > +  as in this case we simply start where we currently are
> > +*/
>
> This is wrong, now it looks as though the comment applies to the code
> below instead of the avio_seek.
>
> > -    while(!bc->eof_reached){
> > -        state= (state<<8) | avio_r8(bc);
> > -        if((state>>56) != 'N')
> > +    while (!bc->eof_reached) {
> > +        state = (state<<8) | avio_r8(bc);
> > +        if ((state>>56) != 'N')
>
> Please give the operators some room to breathe, more below.
>
> > @@ -173,82 +196,104 @@ static int nut_probe(AVProbeData *p){
> > +        if (tmp_fields > 3)
> > +            tmp_size = ffio_read_varlen(bc);
> > +        else tmp_size = 0;
>
> Break the lines after "else", more below.
>
> > @@ -263,64 +308,69 @@ static int decode_main_header(NUTContext *nut){
> >
> > -        for(i=1; i<nut->header_count; i++){
> > -            GET_V(nut->header_len[i], tmp>0 && tmp<256);
> > +
> > +        for(i = 1; i < nut->header_count; i++) {
> > +            GET_V(nut->header_len[i], tmp > 0 && tmp < 256);
>
> for (
>
> >      st = s->streams[stream_id];
> > +
> >      if (!st)
> >          return AVERROR(ENOMEM);
>
> Don't add this empty line.
>
> > +    switch (class) {
> >          case 0:
>
> case statements should be indented at the same level as the switch in K&R.
>
> > +    if (skip_reserved(bc, end) || ffio_get_checksum(bc)){
>
> space before '{'
>
> > -static void set_disposition_bits(AVFormatContext* avf, char* value, int
> stream_id){
> > +static void set_disposition_bits(AVFormatContext* avf, char* value, int
> stream_id)
> > +{
> >      int flag = 0, i;
> > -    for (i=0; ff_nut_dispositions[i].flag; ++i) {
> > +
> > +    for (i = 0; ff_nut_dispositions[i].flag; ++i) {
> >          if (!strcmp(ff_nut_dispositions[i].str, value))
> >              flag = ff_nut_dispositions[i].flag;
> >      }
> > +
> >      if (!flag)
> >          av_log(avf, AV_LOG_INFO, "unknown disposition type '%s'\n",
> value);
> > +
> >      for (i = 0; i < avf->nb_streams; ++i)
> >          if (stream_id == i || stream_id == -1)
> >              avf->streams[i]->disposition |= flag;
> >  }
>
> Here I don't mind, but in general I'm a bit sceptical about adding
> all those empty lines.
>
> > @@ -400,52 +459,53 @@ static int decode_info_header(NUTContext *nut){
> > +        } else if (value == -3) {
> > +            type = "s";
> > +            value = get_s(bc);
> > +        } else if (value == -4) {
> > +            type = "t";
> > +            value = ffio_read_varlen(bc);
>
> Vertically align the '='.
>
>
> That's enough for now, please doublecheck the rest yourself.
>
> Diego
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel
>
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to