[FFmpeg-devel] [PATCH 1/6] Return meaningful error codes, rather than always -1.

[Måns Rullgård]

Stefano Sabatini <stefano.sabatini-lala at poste.it> writes:

> On date Sunday 2010-04-25 10:49:50 +0200, Luca Abeni encoded:
>> Hi,
>> 
>> 
>> On Sat, 2010-04-24 at 20:53 +0200, Stefano Sabatini wrote:
>> > ---
>> >  libavdevice/v4l2.c |   34 +++++++++++++++++-----------------
>> >  1 files changed, 17 insertions(+), 17 deletions(-)
>> > 
>> > diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
>> [...]
>> 
>> the "return -1 ---> return AVERROR(errno)" changes look ok, but I am not
>> sure if the following change is correct:
>> > +        return AVERROR(errno);
>> >      }
>> >      if ((cap.capabilities & V4L2_CAP_VIDEO_CAPTURE) == 0) {
>> >          av_log(ctx, AV_LOG_ERROR, "Not a video capture device\n");
>> >          close(fd);
>> >  
>> > -        return -1;
>> > +        return AVERROR(ENOSYS);
>
> I was unsure between this and EPERM = "Operation not permitted".
>
> EPERM = -1 so it should be safe to just replace -1 with
> AVERROR(EPERM), would that be fine?

Not for that reason.  This is certainly not a problem of lack of
permissions.  Capturing from something which is not a capture device
is impossible no matter what permissions you have.  How about ENODEV?

>> BTW, I was going to miss this patch: you should use a more descriptive
>> subject (example: return meaningful error codes in v4l2.c).
>
> Yes I discovered that the second after I sent the patches. Maybe I
> should prefix the patches with a string providing context to the patch
> series (that may be the name of the corresponding git branch).

In this case I would have prefixed the subject with "v4l2:".

-- 
M?ns Rullg?rd
mans at mansr.com