[MPlayer-dev-eng] [PATCH] Get rid of the "main" label

[Reimar Döffinger]

On Wed, Jan 12, 2011 at 10:39:29PM +0100, Clément Bœsch wrote:
> On Sat, Jan 08, 2011 at 10:40:30PM +0100, Reimar Döffinger wrote:
> > On 8 Jan 2011, at 18:50, Clément Bœsch <ubitux at gmail.com> wrote:
> > > On Sat, Jan 08, 2011 at 05:41:38PM +0100, Reimar Döffinger wrote:
> > >> On 25 Dec 2010, at 17:29, Clément Bœsch <ubitux at gmail.com> wrote:
> > >>> I'd like a feedback on this patch: there is still a "main" label in
> > >>> mplayer.c. This is patch to get rid of it.
> > >> 
> > >> If you only want to get rid of it, just use a if(sh_video) block there
> > >> and leave everything else unchanged. I don't like changing more code
> > >> than necessary at least not if there seems to be no good reason to do
> > >> it...
> > > 
> > > It's not enough since there is two goto main. I would have done it like
> > > you said in case of just one.
> > 
> > Ok, but that's still no reason to move the code so far around.
> > Also the code is confusing, part of the confusion is due to a completely pointless check ofmthe reinit result, the only thing that does is making a crash possible.
> > I think the code should be
> > If (video) reinit...
> > If (video) previous code
> > Else if (!audio) goto next
> 
> Mmh ok I see :)
> 
> Well I'll commit this version in a few days then. Thanks. Just a question
> though: is the crash you were refering to would be a case where reinit
> returns false but video context is still set?

No, I mean the only case where not checking reinit result makes any difference
in behaviour is if it would return true but still set sh_video to NULL,
which would crash with the current code. So that means that we definitely
will not lose anything by changing it.

> -if(!mpctx->sh_video) goto main; // audio-only
> -
> -if(!reinit_video_chain()) {
> -  if(!mpctx->sh_video){
> -    if(!mpctx->sh_audio) goto goto_next_file;
> -    goto main; // exit_player(MSGTR_Exit_error);
> -  }
> -}
> -
> -   if(vo_flags & 0x08 && vo_spudec)
> -      spudec_set_hw_spu(vo_spudec,mpctx->video_out);
> +  if (mpctx->sh_video)
> +      reinit_video_chain();
>  
> +  if (mpctx->sh_video) {
> +      if (vo_flags & 0x08 && vo_spudec)
> +          spudec_set_hw_spu(vo_spudec, mpctx->video_out);
>  #ifdef CONFIG_FREETYPE
> -   force_load_font = 1;
> +      force_load_font = 1;
>  #endif
> +  } else if (!mpctx->sh_audio)
> +      goto goto_next_file;

While it's small enough to be readable/reviewable okish anyway,
I'd prefer it if you applied reindenting the unchanged parts
separately.
Otherwise ok.