[PATCH weston] vaapi-recorder: Don't loop trying to write on out of space condition

2014-05-09 Thread Ander Conselvan de Oliveira
From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com

The error handling for the function that writes the encoded frame on
the disk was bogus, always assuming the buffer supplied to the encoder
was too small. That would cause a bigger buffer to be allocated and
another attempt to encode the frame was done. In the case of a failure
to write to disk (due to ENOSPC, for instance) that would cause an
endless loop.

---
Possibly related to
https://bugs.freedesktop.org/show_bug.cgi?id=69330
---
 src/compositor-drm.c | 27 +++
 src/vaapi-recorder.c | 42 +-
 src/vaapi-recorder.h |  2 +-
 3 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 5f59789..7d514e4 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -2558,6 +2558,18 @@ planes_binding(struct weston_seat *seat, uint32_t time, 
uint32_t key, void *data
 
 #ifdef BUILD_VAAPI_RECORDER
 static void
+recorder_destroy(struct drm_output *output)
+{
+   vaapi_recorder_destroy(output-recorder);
+   output-recorder = NULL;
+
+   output-base.disable_planes--;
+
+   wl_list_remove(output-recorder_frame_listener.link);
+   weston_log([libva recorder] done\n);
+}
+
+static void
 recorder_frame_notify(struct wl_listener *listener, void *data)
 {
struct drm_output *output;
@@ -2579,7 +2591,12 @@ recorder_frame_notify(struct wl_listener *listener, void 
*data)
return;
}
 
-   vaapi_recorder_frame(output-recorder, fd, output-current-stride);
+   ret = vaapi_recorder_frame(output-recorder, fd,
+  output-current-stride);
+   if (ret  0) {
+   weston_log([libva recorder] aborted: %m\n);
+   recorder_destroy(output);
+   }
 }
 
 static void *
@@ -2637,13 +2654,7 @@ recorder_binding(struct weston_seat *seat, uint32_t 
time, uint32_t key,
 
weston_log([libva recorder] initialized\n);
} else {
-   vaapi_recorder_destroy(output-recorder);
-   output-recorder = NULL;
-
-   output-base.disable_planes--;
-
-   wl_list_remove(output-recorder_frame_listener.link);
-   weston_log([libva recorder] done\n);
+   recorder_destroy(output);
}
 }
 #else
diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c
index 0095a42..921494d 100644
--- a/src/vaapi-recorder.c
+++ b/src/vaapi-recorder.c
@@ -50,6 +50,7 @@
 #include string.h
 #include unistd.h
 #include assert.h
+#include errno.h
 
 #include sys/types.h
 #include sys/stat.h
@@ -93,6 +94,7 @@ struct vaapi_recorder {
int width, height;
int frame_count;
 
+   int error;
int destroying;
pthread_t worker_thread;
pthread_mutex_t mutex;
@@ -761,7 +763,13 @@ encoder_create_output_buffer(struct vaapi_recorder *r)
return VA_INVALID_ID;
 }
 
-static int
+enum output_write_status {
+   OUTPUT_WRITE_SUCCESS,
+   OUTPUT_WRITE_OVERFLOW,
+   OUTPUT_WRITE_FATAL
+};
+
+static enum output_write_status
 encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf)
 {
VACodedBufferSegment *segment;
@@ -770,19 +778,22 @@ encoder_write_output(struct vaapi_recorder *r, VABufferID 
output_buf)
 
status = vaMapBuffer(r-va_dpy, output_buf, (void **) segment);
if (status != VA_STATUS_SUCCESS)
-   return -1;
+   return OUTPUT_WRITE_FATAL;
 
if (segment-status  VA_CODED_BUF_STATUS_SLICE_OVERFLOW_MASK) {
r-encoder.output_size *= 2;
vaUnmapBuffer(r-va_dpy, output_buf);
-   return -1;
+   return OUTPUT_WRITE_OVERFLOW;
}
 
count = write(r-output_fd, segment-buf, segment-size);
 
vaUnmapBuffer(r-va_dpy, output_buf);
 
-   return count;
+   if (count  0)
+   return OUTPUT_WRITE_FATAL;
+
+   return OUTPUT_WRITE_SUCCESS;
 }
 
 static void
@@ -792,9 +803,8 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input)
 
VABufferID buffers[8];
int count = 0;
-
-   int slice_type;
-   int ret, i;
+   int i, slice_type;
+   enum output_write_status ret;
 
if ((r-frame_count % r-encoder.intra_period) == 0)
slice_type = SLICE_TYPE_I;
@@ -829,7 +839,10 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID input)
output_buf = VA_INVALID_ID;
 
vaDestroyBuffer(r-va_dpy, buffers[--count]);
-   } while (ret  0);
+   } while (ret == OUTPUT_WRITE_OVERFLOW);
+
+   if (ret == OUTPUT_WRITE_FATAL)
+   r-error = errno;
 
for (i = 0; i  count; i++)
vaDestroyBuffer(r-va_dpy, buffers[i]);
@@ -1138,11 +1151,19 @@ worker_thread_function(void *data)
return NULL;
 }
 
-void
+int
 vaapi_recorder_frame(struct vaapi_recorder *r, int prime_fd, int 

Re: [PATCH weston] vaapi-recorder: Don't loop trying to write on out of space condition

2014-05-09 Thread Kristian Høgsberg
On Fri, May 09, 2014 at 03:57:38PM +0300, Ander Conselvan de Oliveira wrote:
 From: Ander Conselvan de Oliveira ander.conselvan.de.olive...@intel.com
 
 The error handling for the function that writes the encoded frame on
 the disk was bogus, always assuming the buffer supplied to the encoder
 was too small. That would cause a bigger buffer to be allocated and
 another attempt to encode the frame was done. In the case of a failure
 to write to disk (due to ENOSPC, for instance) that would cause an
 endless loop.

Thanks committed.  I added the bugzilla link to the commit message for
reference.

Kristian

 ---
 Possibly related to
 https://bugs.freedesktop.org/show_bug.cgi?id=69330
 ---
  src/compositor-drm.c | 27 +++
  src/vaapi-recorder.c | 42 +-
  src/vaapi-recorder.h |  2 +-
  3 files changed, 53 insertions(+), 18 deletions(-)
 
 diff --git a/src/compositor-drm.c b/src/compositor-drm.c
 index 5f59789..7d514e4 100644
 --- a/src/compositor-drm.c
 +++ b/src/compositor-drm.c
 @@ -2558,6 +2558,18 @@ planes_binding(struct weston_seat *seat, uint32_t 
 time, uint32_t key, void *data
  
  #ifdef BUILD_VAAPI_RECORDER
  static void
 +recorder_destroy(struct drm_output *output)
 +{
 + vaapi_recorder_destroy(output-recorder);
 + output-recorder = NULL;
 +
 + output-base.disable_planes--;
 +
 + wl_list_remove(output-recorder_frame_listener.link);
 + weston_log([libva recorder] done\n);
 +}
 +
 +static void
  recorder_frame_notify(struct wl_listener *listener, void *data)
  {
   struct drm_output *output;
 @@ -2579,7 +2591,12 @@ recorder_frame_notify(struct wl_listener *listener, 
 void *data)
   return;
   }
  
 - vaapi_recorder_frame(output-recorder, fd, output-current-stride);
 + ret = vaapi_recorder_frame(output-recorder, fd,
 +output-current-stride);
 + if (ret  0) {
 + weston_log([libva recorder] aborted: %m\n);
 + recorder_destroy(output);
 + }
  }
  
  static void *
 @@ -2637,13 +2654,7 @@ recorder_binding(struct weston_seat *seat, uint32_t 
 time, uint32_t key,
  
   weston_log([libva recorder] initialized\n);
   } else {
 - vaapi_recorder_destroy(output-recorder);
 - output-recorder = NULL;
 -
 - output-base.disable_planes--;
 -
 - wl_list_remove(output-recorder_frame_listener.link);
 - weston_log([libva recorder] done\n);
 + recorder_destroy(output);
   }
  }
  #else
 diff --git a/src/vaapi-recorder.c b/src/vaapi-recorder.c
 index 0095a42..921494d 100644
 --- a/src/vaapi-recorder.c
 +++ b/src/vaapi-recorder.c
 @@ -50,6 +50,7 @@
  #include string.h
  #include unistd.h
  #include assert.h
 +#include errno.h
  
  #include sys/types.h
  #include sys/stat.h
 @@ -93,6 +94,7 @@ struct vaapi_recorder {
   int width, height;
   int frame_count;
  
 + int error;
   int destroying;
   pthread_t worker_thread;
   pthread_mutex_t mutex;
 @@ -761,7 +763,13 @@ encoder_create_output_buffer(struct vaapi_recorder *r)
   return VA_INVALID_ID;
  }
  
 -static int
 +enum output_write_status {
 + OUTPUT_WRITE_SUCCESS,
 + OUTPUT_WRITE_OVERFLOW,
 + OUTPUT_WRITE_FATAL
 +};
 +
 +static enum output_write_status
  encoder_write_output(struct vaapi_recorder *r, VABufferID output_buf)
  {
   VACodedBufferSegment *segment;
 @@ -770,19 +778,22 @@ encoder_write_output(struct vaapi_recorder *r, 
 VABufferID output_buf)
  
   status = vaMapBuffer(r-va_dpy, output_buf, (void **) segment);
   if (status != VA_STATUS_SUCCESS)
 - return -1;
 + return OUTPUT_WRITE_FATAL;
  
   if (segment-status  VA_CODED_BUF_STATUS_SLICE_OVERFLOW_MASK) {
   r-encoder.output_size *= 2;
   vaUnmapBuffer(r-va_dpy, output_buf);
 - return -1;
 + return OUTPUT_WRITE_OVERFLOW;
   }
  
   count = write(r-output_fd, segment-buf, segment-size);
  
   vaUnmapBuffer(r-va_dpy, output_buf);
  
 - return count;
 + if (count  0)
 + return OUTPUT_WRITE_FATAL;
 +
 + return OUTPUT_WRITE_SUCCESS;
  }
  
  static void
 @@ -792,9 +803,8 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID 
 input)
  
   VABufferID buffers[8];
   int count = 0;
 -
 - int slice_type;
 - int ret, i;
 + int i, slice_type;
 + enum output_write_status ret;
  
   if ((r-frame_count % r-encoder.intra_period) == 0)
   slice_type = SLICE_TYPE_I;
 @@ -829,7 +839,10 @@ encoder_encode(struct vaapi_recorder *r, VASurfaceID 
 input)
   output_buf = VA_INVALID_ID;
  
   vaDestroyBuffer(r-va_dpy, buffers[--count]);
 - } while (ret  0);
 + } while (ret == OUTPUT_WRITE_OVERFLOW);
 +
 + if (ret == OUTPUT_WRITE_FATAL)
 + r-error = errno;
  
   for (i = 0; i  count; i++)