[FFmpeg-devel] [PATCH 01/10] lavu: add av_fourcc_make_string() and av_4cc2str()

Clément Bœsch u at pkh.me
Tue Mar 28 11:19:46 EEST 2017


On Tue, Mar 28, 2017 at 12:47:39AM +0200, Alexander Strasser wrote:
[...]
> > +#define AV_FOURCC_MAX_STRING_SIZE 32
> 
> Should be fine, though I don't know how you came up with this max size in particular.
> 

IIRC the actual maximum is 21 characters, I just took the closest superior
power of two. Probably just like we did for AV_TS_MAX_STRING_SIZE.

> > +
> > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc)
> 
> Did your write it as 4cc2str to make the name shorter?
> I guess fourcc2str is more readable and more consistent.
> 

This is a shorthand, so yeah I wanted to keep it short. If you replace
"4cc" with "fourcc", why not do that for the '2' as well? Writing "four"
but keeping '2' was kind of weird to me :)

> > +
> > +/**
> > + * Fill the provided buffer with a string containing a FourCC (four-character
> > + * code) representation.
> > + *
> > + * @param buf    a buffer with size in bytes of at least AV_FOURCC_MAX_STRING_SIZE
> > + * @param fourcc the fourcc to represent
> > + * @return the buffer in input
> > + */
> > +char *av_fourcc_make_string(char *buf, uint32_t fourcc);
> > +
> >  /**
> >   * @}
> >   * @}
> > diff --git a/libavutil/utils.c b/libavutil/utils.c
> > index 36e4dd5fdb..ba557b9252 100644
> > --- a/libavutil/utils.c
> > +++ b/libavutil/utils.c
> > @@ -121,6 +121,27 @@ unsigned av_int_list_length_for_size(unsigned elsize,
> >      return i;
> >  }
> >  
> > +char *av_fourcc_make_string(char *buf, uint32_t fourcc)
> > +{
> > +    int i, len;
> > +    char *orig_buf = buf;
> > +    size_t buf_size = AV_FOURCC_MAX_STRING_SIZE;
> > +
> > +#define TAG_PRINT(x)                                              \
> > +    (((x) >= '0' && (x) <= '9') ||                                \
> > +     ((x) >= 'a' && (x) <= 'z') || ((x) >= 'A' && (x) <= 'Z') ||  \
> > +     ((x) == '.' || (x) == ' ' || (x) == '-' || (x) == '_'))
> 
> You could spare this macro, by using a temporary char for the character and
> an int to indicate if it is printable in the loop below. Would probably be 
> 1 or 2 lines longer.
> 
> You might want to at least rename TAG_PRINT to IS_PRINTABLE or
> IS_PRINTABLE_FOURCC_CHAR or similar as TAG_PRINT is a rather confusing name.
> 
> Also if the macro is only introduced for use this function, #undef could be 
> used, but I don't think we really do it anywhere else.
> 

Yeah well I just took the existing code in libavcodec, so I didn't want to
change it too much.

But you're right on all your comments so I just adjusted the function. See
patch attached.

[...]

Thanks

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavu-add-av_fourcc_make_string-and-av_4cc2str.patch
Type: text/x-diff
Size: 3200 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170328/e93cb510/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170328/e93cb510/attachment.sig>


More information about the ffmpeg-devel mailing list