[MPlayer-dev-eng] [PATCH 5/5] Various rectifications uncrustify wasn't able to do.

Diego Biurrun diego at biurrun.de
Thu May 5 15:11:47 CEST 2011


On Thu, May 05, 2011 at 03:00:17PM +0200, Clément Bœsch wrote:
> On Sat, Apr 16, 2011 at 12:02:53AM +0200, Clément Bœsch wrote:
> > Due to uncrustify and A.I. limitations, humans are still needed
> > sometimes.
> 
> Patch updated. No more functionnal changes, and more cosmetics, vertical
> align & stuff. It follows the uncrustify patch.
> 
> --- a/mplayer.c
> +++ b/mplayer.c
> @@ -449,16 +449,11 @@ char *get_metadata(metadata_t type)
>  
>      case META_VIDEO_CODEC:
> -        if (sh_video->format == 0x10000001)
> -            return strdup("mpeg1");
> -        else if (sh_video->format == 0x10000002)
> -            return strdup("mpeg2");
> -        else if (sh_video->format == 0x10000004)
> -            return strdup("mpeg4");
> -        else if (sh_video->format == 0x10000005)
> -            return strdup("h264");
> -        else if (sh_video->format >= 0x20202020)
> -            return mp_asprintf("%.4s", (char *)&sh_video->format);
> +             if (sh_video->format == 0x10000001) return strdup("mpeg1");
> +        else if (sh_video->format == 0x10000002) return strdup("mpeg2");
> +        else if (sh_video->format == 0x10000004) return strdup("mpeg4");
> +        else if (sh_video->format == 0x10000005) return strdup("h264");
> +        else if (sh_video->format >= 0x20202020) return mp_asprintf("%.4s", (char *)&sh_video->format);
>          return mp_asprintf("0x%08X", sh_video->format);

I prefer the former.  I don't consider it a good idea to stray from the
automatic indentation (too much).

> @@ -479,26 +474,13 @@ char *get_metadata(metadata_t type)
>  
>      /* check for valid demuxer */
> -    case META_INFO_TITLE:
> -        return get_demuxer_info("Title");
> -
> -    case META_INFO_ARTIST:
> -        return get_demuxer_info("Artist");
> -
> -    case META_INFO_ALBUM:
> -        return get_demuxer_info("Album");
> -
> -    case META_INFO_YEAR:
> -        return get_demuxer_info("Year");
> -
> -    case META_INFO_COMMENT:
> -        return get_demuxer_info("Comment");
> -
> -    case META_INFO_TRACK:
> -        return get_demuxer_info("Track");
> -
> -    case META_INFO_GENRE:
> -        return get_demuxer_info("Genre");
> +    case META_INFO_TITLE:   return get_demuxer_info("Title");
> +    case META_INFO_ARTIST:  return get_demuxer_info("Artist");
> +    case META_INFO_ALBUM:   return get_demuxer_info("Album");
> +    case META_INFO_YEAR:    return get_demuxer_info("Year");
> +    case META_INFO_COMMENT: return get_demuxer_info("Comment");
> +    case META_INFO_TRACK:   return get_demuxer_info("Track");
> +    case META_INFO_GENRE:   return get_demuxer_info("Genre");

same

> @@ -516,14 +498,14 @@ static void print_file_properties(const MPContext *mpctx, const char *filename)
>      if (mpctx->sh_video) {
>          /* Assume FOURCC if all bytes >= 0x20 (' ') */
>          if (mpctx->sh_video->format >= 0x20202020)
> -            mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_FORMAT=%.4s\n", (char *)&mpctx->sh_video->format);
> +            mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_FORMAT=%.4s\n",   (char *)&mpctx->sh_video->format);
>          else
>              mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_FORMAT=0x%08X\n", mpctx->sh_video->format);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_BITRATE=%d\n", mpctx->sh_video->i_bps * 8);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_WIDTH=%d\n", mpctx->sh_video->disp_w);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_HEIGHT=%d\n", mpctx->sh_video->disp_h);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_FPS=%5.3f\n", mpctx->sh_video->fps);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_VIDEO_ASPECT=%1.4f\n", mpctx->sh_video->aspect);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_VIDEO_BITRATE=%d\n",    mpctx->sh_video->i_bps * 8);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_VIDEO_WIDTH=%d\n",      mpctx->sh_video->disp_w);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_VIDEO_HEIGHT=%d\n",     mpctx->sh_video->disp_h);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_VIDEO_FPS=%5.3f\n",     mpctx->sh_video->fps);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_VIDEO_ASPECT=%1.4f\n",  mpctx->sh_video->aspect);
>          video_start_pts = ds_get_next_pts(mpctx->d_video);
>      }
>      if (mpctx->sh_audio) {
> @@ -531,10 +513,10 @@ static void print_file_properties(const MPContext *mpctx, const char *filename)
>          if (mpctx->sh_audio->format >= 0x20202020)
>              mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_FORMAT=%.4s\n", (char *)&mpctx->sh_audio->format);
>          else
> -            mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_FORMAT=%d\n", mpctx->sh_audio->format);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_BITRATE=%d\n", mpctx->sh_audio->i_bps * 8);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_RATE=%d\n", mpctx->sh_audio->samplerate);
> -        mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_NCH=%d\n", mpctx->sh_audio->channels);
> +            mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_FORMAT=%d\n",   mpctx->sh_audio->format);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_AUDIO_BITRATE=%d\n",  mpctx->sh_audio->i_bps * 8);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_AUDIO_RATE=%d\n",     mpctx->sh_audio->samplerate);
> +        mp_msg(MSGT_IDENTIFY, MSGL_INFO,     "ID_AUDIO_NCH=%d\n",      mpctx->sh_audio->channels);

I don't see the point.

> @@ -788,9 +769,8 @@ void exit_player(enum exit_reason how)
>  static void child_sighandler(int x)
>  {
>      pid_t pid;
> -    while ((pid = waitpid(-1, NULL, WNOHANG)) > 0) ;
> +    while ((pid = waitpid(-1, NULL, WNOHANG)) > 0);
>  }

This looks like a bug in uncrustify or the uncrustify configuration.

> @@ -1046,14 +1026,11 @@ static int playtree_add_playlist(play_tree_t *entry)
>      {
>          if (!entry) {
>              entry = mpctx->playtree_iter->tree;
> -            if (play_tree_iter_step(mpctx->playtree_iter, 1, 0) != PLAY_TREE_ITER_ENTRY) {
> +            if (play_tree_iter_step(mpctx->playtree_iter, 1, 0) != PLAY_TREE_ITER_ENTRY)
>                  return PT_NEXT_ENTRY;
> -            }
> -            if (mpctx->playtree_iter->tree == entry) { // Loop with a single file
> -                if (play_tree_iter_up_step(mpctx->playtree_iter, 1, 0) != PLAY_TREE_ITER_ENTRY) {
> +            if (mpctx->playtree_iter->tree == entry) // Loop with a single file
> +                if (play_tree_iter_up_step(mpctx->playtree_iter, 1, 0) != PLAY_TREE_ITER_ENTRY)
>                      return PT_NEXT_ENTRY;
> -                }
> -            }

I think uncrustify has an option for this.

> @@ -1927,9 +1903,9 @@ static mp_image_t *mp_dvdnav_copy_mpi(mp_image_t *to_mpi,
>      if (to_mpi &&
>          to_mpi->w == from_mpi->w &&
>          to_mpi->h == from_mpi->h &&
> -        to_mpi->imgfmt == from_mpi->imgfmt)
> +        to_mpi->imgfmt == from_mpi->imgfmt) {
>          mpi = to_mpi;
> -    else {
> +    } else {
>          if (to_mpi)
>              free_mp_image(to_mpi);
>          if (from_mpi->w == 0 || from_mpi->h == 0)
> @@ -2531,8 +2507,9 @@ static void pause_loop(void)
>          if (term_osd && !mpctx->sh_video) {
>              set_osd_msg(OSD_MSG_PAUSE, 1, 0, MSGTR_Paused);
>              update_osd_msg();
> -        } else
> +        } else {
>              mp_msg(MSGT_CPLAYER, MSGL_STATUS, "\n"MSGTR_Paused "\r");
> +        }
>          mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_PAUSED\n");
>      }
>  #ifdef CONFIG_GUI
> @@ -2574,9 +2551,10 @@ static void pause_loop(void)
>                      set_osd_msg(OSD_MSG_PAUSE, 1, 0, MSGTR_Paused " %d%%",
>                                  new_cache_fill);
>                      update_osd_msg();
> -                } else
> +                } else {
>                      mp_msg(MSGT_CPLAYER, MSGL_STATUS, MSGTR_Paused " %d%%\r",
>                             new_cache_fill);
> +                }

I see little point in doing this manually if uncrustify can achieve it.

> @@ -3306,19 +3280,15 @@ play_next_file:
>  
>      if (mpctx->stream->type == STREAMTYPE_BD) {
> -        if (audio_lang && audio_id == -1)
> -            audio_id = bd_aid_from_lang(mpctx->stream, audio_lang);
> -        if (dvdsub_lang && dvdsub_id == -1)
> -            dvdsub_id = bd_sid_from_lang(mpctx->stream, dvdsub_lang);
> +        if (audio_lang  && audio_id  == -1) audio_id  = bd_aid_from_lang(mpctx->stream, audio_lang);
> +        if (dvdsub_lang && dvdsub_id == -1) dvdsub_id = bd_sid_from_lang(mpctx->stream, dvdsub_lang);
>      }
>  
>  #ifdef CONFIG_DVDREAD
>      if (mpctx->stream->type == STREAMTYPE_DVD) {
>          current_module = "dvd lang->id";
> -        if (audio_lang && audio_id == -1)
> -            audio_id = dvd_aid_from_lang(mpctx->stream, audio_lang);
> -        if (dvdsub_lang && dvdsub_id == -1)
> -            dvdsub_id = dvd_sid_from_lang(mpctx->stream, dvdsub_lang);
> +        if (audio_lang  && audio_id  == -1) audio_id  = dvd_aid_from_lang(mpctx->stream, audio_lang);
> +        if (dvdsub_lang && dvdsub_id == -1) dvdsub_id = dvd_sid_from_lang(mpctx->stream, dvdsub_lang);

I'd skip this as well.

> @@ -3878,20 +3828,20 @@ goto_enable_cache:
>  
>  //============================ Auto QUALITY ============================
>  
> -/*Output quality adjustments:*/
> +                /* Output quality adjustments: */
>                  if (auto_quality > 0) {
>                      current_module = "autoq";
> -//  float total=0.000001f * (GetTimer()-aq_total_time);
> -//  if(output_quality<auto_quality && aq_sleep_time>0.05f*total)
> +                    //float total=0.000001f * (GetTimer()-aq_total_time);
> +                    //if(output_quality<auto_quality && aq_sleep_time>0.05f*total)
>                      if (output_quality < auto_quality && aq_sleep_time > 0)
>                          ++output_quality;
>                      else
> -//  if(output_quality>0 && aq_sleep_time<-0.05f*total)
> +                    //if(output_quality>0 && aq_sleep_time<-0.05f*total)
>                      if (output_quality > 1 && aq_sleep_time < 0)
>                          --output_quality;
>                      else if (output_quality > 0 && aq_sleep_time < -0.050f) // 50ms
>                          output_quality = 0;
> -//  printf("total: %8.6f  sleep: %8.6f  q: %d\n",(0.000001f*aq_total_time),aq_sleep_time,output_quality);
> +                    //printf("total: %8.6f  sleep: %8.6f  q: %d\n",(0.000001f*aq_total_time),aq_sleep_time,output_quality);
>                      set_video_quality(mpctx->sh_video, output_quality);

The style in the commented-out code is messed up :-p

Most of the rest like the comment adjustments etc. looks OK.

Diego


More information about the MPlayer-dev-eng mailing list