On Sun, Apr 23, 2017 at 11:31 PM, James Almer <jamr...@gmail.com> wrote:
> On 4/20/2017 12:34 PM, Vittorio Giovara wrote:
>>
>> Change input type of the type->str function and return a proper
>> error code for the str->type function.
>> ---
>> Similar reasoning to the previous patch.
>> Vittorio
>>
>>   libavutil/stereo3d.c | 6 +++---
>>   libavutil/stereo3d.h | 2 +-
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
>> index 5dc902e909..6e2f9f3ad2 100644
>> --- a/libavutil/stereo3d.c
>> +++ b/libavutil/stereo3d.c
>> @@ -54,9 +54,9 @@ static const char * const stereo3d_type_names[] = {
>>       [AV_STEREO3D_COLUMNS]             = "interleaved columns",
>>   };
>>   -const char *av_stereo3d_type_name(unsigned int type)
>> +const char *av_stereo3d_type_name(enum AVStereo3DType type)
>>   {
>> -    if (type >= FF_ARRAY_ELEMS(stereo3d_type_names))
>> +    if ((unsigned) type >= FF_ARRAY_ELEMS(stereo3d_type_names))
>>           return "unknown";
>>         return stereo3d_type_names[type];
>> @@ -72,5 +72,5 @@ int av_stereo3d_from_name(const char *name)
>>               return i;
>>       }
>>   -    return -1;
>> +    return AVERROR(EINVAL);
>
>
> Why EINVAL here but ENOSYS for spherical?

no reason, i can switch to enonsys

> Also, while changing -1 to AVERROR() isn't a considerable API break as
> changing NULL on failure to a string on failure, it's still one and really
> makes me wonder if it's worth doing.
> While most users probably just check for < 0, in which case both return
> values would work the same, anyone instead checking explicitly for -1 (a
> completely valid check as per the documentation, which you for that matter
> did not update in this patch) will instead break.

besides the fact that we're in the "break period" after the version
bump, there should be no reason to explicitly check for -1 so I don't
think that's a case that it's worth considering
-- 
Vittorio
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to