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++)
>               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 stride)
>  {
> +     int ret = 0;
> +
>       pthread_mutex_lock(&r->mutex);
>  
> +     if (r->error) {
> +             errno = r->error;
> +             ret = -1;
> +             goto unlock;
> +     }
> +
>       /* The mutex is never released while encoding, so this point should
>        * never be reached if input.valid is true. */
>       assert(!r->input.valid);
> @@ -1152,5 +1173,8 @@ vaapi_recorder_frame(struct vaapi_recorder *r, int 
> prime_fd, int stride)
>       r->input.valid = 1;
>       pthread_cond_signal(&r->input_cond);
>  
> +unlock:
>       pthread_mutex_unlock(&r->mutex);
> +
> +     return ret;
>  }
> diff --git a/src/vaapi-recorder.h b/src/vaapi-recorder.h
> index 664b1f9..e304698 100644
> --- a/src/vaapi-recorder.h
> +++ b/src/vaapi-recorder.h
> @@ -29,7 +29,7 @@ struct vaapi_recorder *
>  vaapi_recorder_create(int drm_fd, int width, int height, const char 
> *filename);
>  void
>  vaapi_recorder_destroy(struct vaapi_recorder *r);
> -void
> +int
>  vaapi_recorder_frame(struct vaapi_recorder *r, int fd, int stride);
>  
>  #endif /* _VAAPI_RECORDER_H_ */
> -- 
> 1.8.3.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to