[FFmpeg-devel] ffplay and the return values

[Marton Balint]

On Sun, 8 Apr 2012, Bernd Krüger-Knauber wrote:

> Sorry,
>
> it's my first contribution to a bigger project.
>
> Here is diff -u output
>
> --- ffplay.c.org	2012-04-07 22:02:21 +0000
> +++ ffplay.c	2012-04-08 14:40:13 +0000
> @@ -84,6 +84,21 @@
>  /* NOTE: the size must be big enough to compensate the hardware audio buffersize size */
>  #define SAMPLE_ARRAY_SIZE (2 * 65536)
> 
> +enum ExitValues { 
> +	NormalExit,
> +	UserExit,
> +	SDLVideoModeErrorExit,
> +	ImageToBigErrorExit,

please mind the spelling: ImageTooBig

> +	LockMgrErorExit,

spelling: Error

> +	VideoStateErrorExit,
> +	ConversionContextInitErrorExit,
> +	OptionArgumentErrorExit,
> +	NoInputfileErrorExit,

should be InputFile

> +	SDLInitErrorExit,
> +	InputfileErrorExit,

same here

> +	SigTermHandlerExit
> +};
> +
>  static int sws_flags = SWS_BICUBIC;
>
>  typedef struct PacketQueue {
> @@ -908,7 +923,7 @@
>      av_free(is);
>  }
> 
> -static void do_exit(VideoState *is)
> +static void do_exit(VideoState *is, int ExitValue)

please use exit_value for variable name instead, CamelCase is not used for 
variables in the current code.

>  {
>      if (is) {
>          stream_close(is);
> @@ -923,12 +938,12 @@
>          printf("\n");
>      SDL_Quit();
>      av_log(NULL, AV_LOG_QUIET, "%s", "");
> -    exit(0);
> +    exit(ExitValue);
>  }
>
>  static void sigterm_handler(int sig)
>  {
> -    exit(123);
> +    exit(SigTermHandlerExit);
>  }
>
>  static int video_open(VideoState *is, int force_set_video_mode)
> @@ -964,7 +979,7 @@
>      screen = SDL_SetVideoMode(w, h, 0, flags);
>      if (!screen) {
>          fprintf(stderr, "SDL: could not set video mode - exiting\n");
> -        do_exit(is);
> +        do_exit(is, SDLVideoModeErrorExit);
>      }
>      if (!window_title)
>          window_title = input_filename;
> @@ -1327,7 +1342,7 @@
>          fprintf(stderr, "Error: the video system does not support an image\n"
>                          "size of %dx%d pixels. Try using -lowres or -vf \"scale=w:h\"\n"
>                          "to reduce the image size.\n", vp->width, vp->height );
> -        do_exit(is);
> +        do_exit(is, ImageToBigErrorExit);
>      }
>
>      SDL_LockMutex(is->pictq_mutex);
> @@ -1445,7 +1460,7 @@
>              PIX_FMT_YUV420P, sws_flags, NULL, NULL, NULL);
>          if (is->img_convert_ctx == NULL) {
>              fprintf(stderr, "Cannot initialize the conversion context\n");
> -            exit(1);
> +            exit(ConversionContextInitErrorExit);
>          }
>          sws_scale(is->img_convert_ctx, src_frame->data, src_frame->linesize,
>                    0, vp->height, pict.data, pict.linesize);
> @@ -2826,13 +2841,13 @@
>          switch (event.type) {
>          case SDL_KEYDOWN:
>              if (exit_on_keydown) {
> -                do_exit(cur_stream);
> +                do_exit(cur_stream, UserExit);
>                  break;
>              }
>              switch (event.key.keysym.sym) {
>              case SDLK_ESCAPE:
>              case SDLK_q:
> -                do_exit(cur_stream);
> +                do_exit(cur_stream, UserExit);
>                  break;
>              case SDLK_f:
>                  toggle_full_screen(cur_stream);
> @@ -2904,7 +2919,7 @@
>              break;
>          case SDL_MOUSEBUTTONDOWN:
>              if (exit_on_mousedown) {
> -                do_exit(cur_stream);
> +                do_exit(cur_stream, UserExit);
>                  break;
>              }
>          case SDL_MOUSEMOTION:
> @@ -2947,8 +2962,10 @@
>                  cur_stream->force_refresh = 1;
>              break;
>          case SDL_QUIT:
> +        		do_exit(cur_stream, UserExit);

indentation seems messed up

> +            break;
>          case FF_QUIT_EVENT:
> -            do_exit(cur_stream);
> +            do_exit(cur_stream, NormalExit);
>              break;
>          case FF_ALLOC_EVENT:
>              video_open(event.user.data1, 0);
> @@ -3008,7 +3025,7 @@
>          av_sync_type = AV_SYNC_EXTERNAL_CLOCK;
>      else {
>          fprintf(stderr, "Unknown value for %s: %s\n", opt, arg);
> -        exit(1);
> +        exit(OptionArgumentErrorExit);
>      }
>      return 0;
>  }
> @@ -3039,7 +3056,7 @@
>      if (input_filename) {
>          fprintf(stderr, "Argument '%s' provided as input filename, but '%s' was already specified.\n",
>                  filename, input_filename);
> -        exit_program(1);
> +        exit_program(InputfileErrorExit);
>      }
>      if (!strcmp(filename, "-"))
>          filename = "pipe:";
> @@ -3194,7 +3211,7 @@
>          show_usage();
>          fprintf(stderr, "An input file must be specified\n");
>          fprintf(stderr, "Use -h to get full help or, even better, run 'man %s'\n", program_name);
> -        exit(1);
> +        exit(NoInputfileErrorExit);
>      }
>
>      if (display_disable) {
> @@ -3209,7 +3226,7 @@
>      if (SDL_Init (flags)) {
>          fprintf(stderr, "Could not initialize SDL - %s\n", SDL_GetError());
>          fprintf(stderr, "(Did you set the DISPLAY variable?)\n");
> -        exit(1);
> +        exit(SDLInitErrorExit);
>      }
>
>      if (!display_disable) {
> @@ -3226,7 +3243,7 @@
>
>      if (av_lockmgr_register(lockmgr)) {
>          fprintf(stderr, "Could not initialize lock manager!\n");
> -        do_exit(NULL);
> +        do_exit(NULL, LockMgrErorExit);
>      }
>
>      av_init_packet(&flush_pkt);
> @@ -3235,7 +3252,7 @@
>      is = stream_open(input_filename, file_iformat);
>      if (!is) {
>          fprintf(stderr, "Failed to initialize VideoState!\n");
> -        do_exit(NULL);
> +        do_exit(NULL, VideoStateErrorExit);
>      }
>
>      event_loop(is);

Otherwise it looks fine.

Thanks,
Marton