avfilter/asubprocess: improved error handling

Signed-off-by: Stefan Westerfeld <stefan@space.twc.de>

libavfilter/af_asubprocess.c

@@ -10,14 +10,9 @@
 #include "filters.h"
 #include "internal.h"
 
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <unistd.h>
-
-// =============================================
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <sys/stat.h>
 
 #include <stdio.h>
 #include <unistd.h>
@@ -46,6 +41,20 @@ typedef struct SP
     bool  done;
 } SP;
 
+typedef struct AndioWMarkContext {
+    const AVClass *class;
+
+    const char   *command;
+    int           state;
+    unsigned int  chunk_size;
+    int           in_bit_depth;
+    int           out_bit_depth;
+    SP           *sp;
+    void         *sample_buffer;
+    int           eof;
+} ASubProcessContext;
+
+// =============================================
 static SPB *spb_new(void)
 {
     SPB *spb = malloc (sizeof (SPB));
@@ -56,9 +65,10 @@ static SPB *spb_new(void)
     return spb;
 }
 
-static SP *
-sp_new (const char *command)
+static int
+sp_new(AVFilterContext *ctx)
 {
+    ASubProcessContext *s = ctx->priv;
     SP *sp = calloc (1, sizeof (SP));
     int output_pipe[2];
     int input_pipe[2];
@@ -67,44 +77,80 @@ sp_new (const char *command)
 
     pok = pipe (output_pipe);
     if (pok != 0)
-        printf ("pipe() failed\n");
+    {
+        int ret = AVERROR(errno);
+        av_log(ctx, AV_LOG_ERROR, "Failed create subprocess output pipe: %s.\n", strerror(errno));
+        return ret;
+    }
     pok = pipe (input_pipe);
     if (pok != 0)
-        printf ("pipe() failed\n");
+    {
+        int ret = AVERROR(errno);
+        av_log(ctx, AV_LOG_ERROR, "Failed create subprocess input pipe: %s.\n", strerror(errno));
+        return ret;
+    }
     pid = fork();
     if (pid < 0)
-        printf ("fork() failed\n");
+    {
+        int ret = AVERROR(errno);
+        av_log(ctx, AV_LOG_ERROR, "Failed to fork() subprocess: %s.\n", strerror(errno));
+        return ret;
+    }
     if (pid == 0)
-      {
+    {
         if (dup2 (input_pipe[0], 0) < 0)
-            printf ("dup2() failed\n");
+        {
+            perror("asubprocess dup2() for stdin failed");
+            exit(EXIT_FAILURE);
+        }
         if (dup2 (output_pipe[1], 1) < 0)
-            printf ("dup2() failed\n");
+        {
+            perror("asubprocess dup2() for stdout failed");
+            exit(EXIT_FAILURE);
+        }
         close (output_pipe[1]);
         close (output_pipe[0]);
         close (input_pipe[1]);
         close (input_pipe[0]);
-        execl ("/bin/sh", "/bin/sh", "-c", command, NULL);
-      }
+        execl ("/bin/sh", "/bin/sh", "-c", s->command, NULL);
+        perror("asubprocess execl() failed");
+        exit(EXIT_FAILURE);
+    }
     sp->pid = pid;
 
     close (input_pipe[0]);
     close (output_pipe[1]);
+
+    /* make output pipe non blocking */
     i = fcntl(output_pipe[0], F_GETFL);
-    av_assert0(i != -1);
+    if (i < 0)
+    {
+        int ret = AVERROR(errno);
+        av_log(ctx, AV_LOG_ERROR, "Failed to get output pipe flags: %s.\n", strerror(errno));
+        return ret;
+    }
     i |= O_NONBLOCK;
+
+    /* make input pipe non blocking */
     fcntl(output_pipe[0], F_SETFL, i);
     i = fcntl(input_pipe[1], F_GETFL);
-    av_assert0(i != -1);
+    if (i < 0)
+    {
+        int ret = AVERROR(errno);
+        av_log(ctx, AV_LOG_ERROR, "Failed to get input pipe flags: %s.\n", strerror(errno));
+        return ret;
+    }
     i |= O_NONBLOCK;
     fcntl(input_pipe[1], F_SETFL, i);
+
     sp->input_pipe = input_pipe[1];
     sp->output_pipe = output_pipe[0];
-    return (sp);
+    s->sp = sp;
+    return 0;
 }
 
 static int
-sp_can_read (SP *sp)
+sp_can_read(SP *sp)
 {
     int can_read = 0;
     for (SPB *spb = sp->out_buffers; spb; spb = spb->next)
@@ -114,16 +160,14 @@ sp_can_read (SP *sp)
 }
 
 static void
-sp_read (SP *sp, char *buffer, int count)
+sp_read(SP *sp, char *buffer, int count)
 {
     SPB *spb = sp->out_buffers;
     while (spb)
     {
-        int n = spb->count - spb->offset;
-        if (n > count)
-            n = count;
+        int n = FFMIN(spb->count - spb->offset, count);
 
-        memcpy (buffer, spb->data + spb->offset, n);
+        memcpy(buffer, spb->data + spb->offset, n);
         spb->offset += n;
         count -= n;
         buffer += n;
@@ -131,7 +175,7 @@ sp_read (SP *sp, char *buffer, int count)
             return;
 
         sp->out_buffers = spb->next;
-        free (spb);
+        free(spb);
         spb = sp->out_buffers;
     }
 }
@@ -146,7 +190,10 @@ sp_write(AVFilterContext *ctx, SP *sp, char *buffer, int count)
         sp->input_pipe = -1;
     }
     if (count && sp->done)
+    {
+        av_log(ctx, AV_LOG_ERROR, "Subprocess terminated before all input data could be written.\n");
         return AVERROR_EXTERNAL;
+    }
     do
     {
         struct pollfd fds[2] = {
@@ -193,12 +240,21 @@ sp_write(AVFilterContext *ctx, SP *sp, char *buffer, int count)
             }
             else if (rc == 0)
             {
-                int status;
+                int status, exit_status;
                 waitpid(sp->pid, &status, 0);
-                printf("wstatus=%d\n", WEXITSTATUS (status));
+                exit_status = WEXITSTATUS(status);
                 sp->done = true;
+
+                if (exit_status != 0)
+                {
+                    av_log(ctx, AV_LOG_ERROR, "Subprocess failed with non-zero exit code (%d).\n", exit_status);
+                    return AVERROR_EXTERNAL;
+                }
                 if (count)
+                {
+                    av_log(ctx, AV_LOG_ERROR, "Subprocess terminated before all input data could be written.\n");
                     return AVERROR_EXTERNAL;
+                }
                 return 0;
             }
             else if (errno != EAGAIN && errno != EINTR)
@@ -228,21 +284,6 @@ enum {
     STATE_ERROR
 };
 
-typedef struct AndioWMarkContext {
-    const AVClass *class;
-
-    const char   *command;
-
-    int           fd;
-    int           state;
-    unsigned int  chunk_size;
-    int           in_bit_depth;
-    int           out_bit_depth;
-    SP           *sp;
-    void         *sample_buffer;
-    int           eof;
-} ASubProcessContext;
-
 #define OFFSET(x) offsetof(ASubProcessContext, x)
 #define A AV_OPT_FLAG_AUDIO_PARAM|AV_OPT_FLAG_FILTERING_PARAM
 
@@ -316,21 +357,23 @@ static int config_input(AVFilterLink *inlink)
 {
     AVFilterContext *ctx = inlink->dst;
     ASubProcessContext *s = ctx->priv;
-    s->sp = sp_new (s->command);
-    write_wav_header(ctx, s, inlink->ch_layout.nb_channels, inlink->sample_rate);
+    int ret;
 
-#if 0
-    int opts = s->transients|s->detector|s->phase|s->window|
-               s->smoothing|s->formant|s->opitch|s->channels|
-               RubberBandOptionProcessRealTime;
+    ret = sp_new(ctx);
+    if (ret != 0)
+        return ret;
+
+    ret = write_wav_header(ctx, s, inlink->ch_layout.nb_channels, inlink->sample_rate);
+    if (ret != 0)
+        return ret;
 
+#if 0
     if (s->rbs)
         rubberband_delete(s->rbs);
     s->rbs = rubberband_new(inlink->sample_rate, inlink->ch_layout.nb_channels, opts, 1. / s->tempo, s->pitch);
     if (!s->rbs)
         return AVERROR(ENOMEM);
 
-    s->nb_samples = rubberband_get_samples_required(s->rbs);
     s->first_pts = AV_NOPTS_VALUE;
 #endif
     s->state = STATE_EXPECT_RIFF;
@@ -521,11 +564,14 @@ static int activate(AVFilterContext *ctx)
         if (ret != 0)
             return ret;
     }
-    if (s->eof && !sp_done (s->sp))
+    if (s->eof && !sp_done(s->sp))
     {
-        while (!sp_done (s->sp))
-            sp_write (ctx, s->sp, 0, 0);
-        printf ("eof\n");
+        while (!sp_done(s->sp))
+        {
+            int ret = sp_write(ctx, s->sp, 0, 0);
+            if (ret != 0)
+                return ret;
+        }
     }
     ret = try_read_frame(s, outlink);
     if (ret != 0)