[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