[FFmpeg-devel] [PATCH 3/3] lavd/v4l2: simplify return value checks
Stefano Sabatini
stefasab at gmail.com
Wed Jan 16 11:29:07 CET 2013
On date Wednesday 2013-01-16 00:32:15 +0100, Stephan Hilb encoded:
> ---
> libavdevice/v4l2.c | 49 ++++++++++++++++++-------------------------------
> 1 file changed, 18 insertions(+), 31 deletions(-)
>
> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
> index 41ae3e8..2b11359 100644
> --- a/libavdevice/v4l2.c
> +++ b/libavdevice/v4l2.c
> @@ -162,15 +162,14 @@ static int device_open(AVFormatContext *ctx)
> {
> struct v4l2_capability cap;
> int fd;
> - int res, err;
> + int err;
> int flags = O_RDWR;
>
> if (ctx->flags & AVFMT_FLAG_NONBLOCK) {
> flags |= O_NONBLOCK;
> }
>
> - fd = v4l2_open(ctx->filename, flags, 0);
> - if (fd < 0) {
> + if ((fd = v4l2_open(ctx->filename, flags, 0)) < 0) {
> err = errno;
>
> av_log(ctx, AV_LOG_ERROR, "Cannot open video device %s: %s\n",
> @@ -179,8 +178,7 @@ static int device_open(AVFormatContext *ctx)
> return AVERROR(err);
> }
>
> - res = v4l2_ioctl(fd, VIDIOC_QUERYCAP, &cap);
> - if (res < 0) {
> + if (v4l2_ioctl(fd, VIDIOC_QUERYCAP, &cap) < 0) {
> err = errno;
> av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QUERYCAP): %s\n",
> strerror(err));
> @@ -258,11 +256,9 @@ static int device_init(AVFormatContext *ctx, int *width, int *height,
>
> static int first_field(int fd)
> {
> - int res;
> v4l2_std_id std;
>
> - res = v4l2_ioctl(fd, VIDIOC_G_STD, &std);
> - if (res < 0) {
> + if (v4l2_ioctl(fd, VIDIOC_G_STD, &std) < 0) {
> return 0;
> }
> if (std & V4L2_STD_NTSC) {
> @@ -382,7 +378,7 @@ static void list_formats(AVFormatContext *ctx, int fd, int type)
>
> static int mmap_init(AVFormatContext *ctx)
> {
> - int i, res;
> + int i;
> struct video_data *s = ctx->priv_data;
> struct v4l2_requestbuffers req = {
> .type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
> @@ -390,8 +386,7 @@ static int mmap_init(AVFormatContext *ctx)
> .memory = V4L2_MEMORY_MMAP
> };
>
> - res = v4l2_ioctl(s->fd, VIDIOC_REQBUFS, &req);
> - if (res < 0) {
> + if (v4l2_ioctl(s->fd, VIDIOC_REQBUFS, &req) < 0) {
> if (errno == EINVAL) {
> av_log(ctx, AV_LOG_ERROR, "Device does not support mmap\n");
> } else {
> @@ -423,8 +418,7 @@ static int mmap_init(AVFormatContext *ctx)
> .index = i,
> .memory = V4L2_MEMORY_MMAP
> };
> - res = v4l2_ioctl(s->fd, VIDIOC_QUERYBUF, &buf);
> - if (res < 0) {
> + if (v4l2_ioctl(s->fd, VIDIOC_QUERYBUF, &buf) < 0) {
> av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QUERYBUF)\n");
> return AVERROR(errno);
> }
> @@ -453,7 +447,7 @@ static int mmap_init(AVFormatContext *ctx)
> static void mmap_release_buffer(AVPacket *pkt)
> {
> struct v4l2_buffer buf = { 0 };
> - int res, fd;
> + int fd;
> struct buff_data *buf_descriptor = pkt->priv;
>
> if (pkt->data == NULL)
> @@ -465,8 +459,7 @@ static void mmap_release_buffer(AVPacket *pkt)
> fd = buf_descriptor->fd;
> av_free(buf_descriptor);
>
> - res = v4l2_ioctl(fd, VIDIOC_QBUF, &buf);
> - if (res < 0)
> + if (v4l2_ioctl(fd, VIDIOC_QBUF, &buf) < 0)
> av_log(NULL, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n",
> strerror(errno));
>
> @@ -516,11 +509,10 @@ static int init_convert_timestamp(AVFormatContext *ctx, int64_t ts)
> static int convert_timestamp(AVFormatContext *ctx, int64_t *ts)
> {
> struct video_data *s = ctx->priv_data;
> + int res;
>
> - if (s->ts_mode) {
> - int r = init_convert_timestamp(ctx, *ts);
> - if (r < 0)
> - return r;
> + if (s->ts_mode && (res = init_convert_timestamp(ctx, *ts)) < 0) {
> + return res;
> }
> #if HAVE_CLOCK_GETTIME && defined(CLOCK_MONOTONIC)
> if (s->timefilter) {
> @@ -580,8 +572,7 @@ static int mmap_read_frame(AVFormatContext *ctx, AVPacket *pkt)
> pkt->data= s->buf_start[buf.index];
> pkt->size = buf.bytesused;
> pkt->pts = buf.timestamp.tv_sec * INT64_C(1000000) + buf.timestamp.tv_usec;
> - res = convert_timestamp(ctx, &pkt->pts);
> - if (res < 0)
> + if ((res = convert_timestamp(ctx, &pkt->pts)) < 0)
> return res;
> pkt->destruct = mmap_release_buffer;
> buf_descriptor = av_malloc(sizeof(struct buff_data));
> @@ -605,7 +596,7 @@ static int mmap_start(AVFormatContext *ctx)
> {
> struct video_data *s = ctx->priv_data;
> enum v4l2_buf_type type;
> - int i, res;
> + int i;
>
> for (i = 0; i < s->buffers; i++) {
> struct v4l2_buffer buf = {
> @@ -614,8 +605,7 @@ static int mmap_start(AVFormatContext *ctx)
> .memory = V4L2_MEMORY_MMAP
> };
>
> - res = v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf);
> - if (res < 0) {
> + if (v4l2_ioctl(s->fd, VIDIOC_QBUF, &buf) < 0) {
> av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_QBUF): %s\n",
> strerror(errno));
>
> @@ -624,8 +614,7 @@ static int mmap_start(AVFormatContext *ctx)
> }
>
> type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> - res = v4l2_ioctl(s->fd, VIDIOC_STREAMON, &type);
> - if (res < 0) {
> + if (v4l2_ioctl(s->fd, VIDIOC_STREAMON, &type) < 0) {
> av_log(ctx, AV_LOG_ERROR, "ioctl(VIDIOC_STREAMON): %s\n",
> strerror(errno));
>
> @@ -793,12 +782,10 @@ static int v4l2_read_header(AVFormatContext *s1)
> enum AVCodecID codec_id = AV_CODEC_ID_NONE;
> enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE;
>
> - st = avformat_new_stream(s1, NULL);
> - if (!st)
> + if (!(st = avformat_new_stream(s1, NULL)))
> return AVERROR(ENOMEM);
>
> - s->fd = device_open(s1);
> - if (s->fd < 0)
> + if ((s->fd = device_open(s1)) < 0)
> return s->fd;
LGTM since removes a few pointless "res" variables which stand into
the way (and which don't encode a meaningful error code).
As for the cosmetics riddle I have no strong opinion, but I agree that
in general cosmetic changes for the sake of themselves are not good
for various reasons (code churn which mask bugs, pollute history, make
merges more difficult, also what is "nice" for one is "ugly" for
another one, and there is more useful/interesting stuff to do), so I
leave this commit/approval up to the module maintainer.
--
FFmpeg = Frightening and Fancy MultiPurpose Entertaining Gladiator
More information about the ffmpeg-devel
mailing list