[FFmpeg-devel] [PATCH] Add the function libavcodec/utils.c:avcodec_type_string

[Måns Rullgård]

Aurelien Jacobs <aurel at gnuage.org> writes:

> Diego Biurrun wrote:
>
>> On Thu, Nov 08, 2007 at 06:02:30PM +0100, Stefano Sabatini wrote:
>> > --- libavcodec/utils.c	(revision 10959)
>> > +++ libavcodec/utils.c	(working copy)
>> > @@ -1230,6 +1230,33 @@
>> >      }
>> >  }
>> >  
>> > +char *avcodec_type_string (char *buf, int buf_size, int codec_type)
>> > +{
>> > +    switch (codec_type) {
>> > +    case CODEC_TYPE_VIDEO:
>> > +        snprintf(buf, buf_size, "video");
>> > +        break;
>> > +
>> > +    case CODEC_TYPE_AUDIO:
>> > +        snprintf(buf, buf_size, "audio");
>> > +        break;
>> > +
>> > +    case CODEC_TYPE_DATA:
>> > +        snprintf(buf, buf_size, "data");
>> > +        break;
>> > +
>> > +    case CODEC_TYPE_SUBTITLE:
>> > +        snprintf(buf, buf_size, "subtitle");
>> > +        break;
>> > +
>> > +    default:
>> > +        snprintf(buf, buf_size, "unknown");
>> > +        break;
>> > +    }
>> > +
>> > +    return buf;
>> > +}
>> 
>> I think this should rather be data_type, codec_type is not a good
>> variable name here.
>
> The enum is called CodecType and the constants are named CODEC_TYPE_*,
> so unless you want to change this, codec_type sounds like a good
> variable name.

Why don't you declare the argument as type CodecType?  That should
make it abundantly clear what is expected.

Also, why do you use snprintf() here, when av_strlcpy() would do just
as well, or even simply returning the string constants.

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