Message ID | 20170327075203.7499-1-u@pkh.me |
---|---|
State | Superseded |
Headers | show |
On 3/27/2017 4:51 AM, Clément Bœsch wrote: > --- > doc/APIchanges | 4 ++++ > libavutil/avutil.h | 14 ++++++++++++++ > libavutil/utils.c | 21 +++++++++++++++++++++ > libavutil/version.h | 2 +- > 4 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 6aaa9adceb..4736e3e6fc 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,10 @@ libavutil: 2015-08-28 > > API changes, most recent first: > > +2017-03-xx - xxxxxxx - lavu 55.52.100 - avutil.h > + add av_fourcc_make_string() function and av_4cc2str() macro to replace > + av_get_codec_tag_string() from lavc. > + > 2017-03-xx - xxxxxxx - lavc 57.85.101 - avcodec.h > vdpau hardware accelerated decoding now supports the new hwaccel API, which > can create the decoder context and allocate hardware frame automatically. > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > index e9aaa03722..98100fdcc5 100644 > --- a/libavutil/avutil.h > +++ b/libavutil/avutil.h > @@ -343,6 +343,20 @@ FILE *av_fopen_utf8(const char *path, const char *mode); > */ > AVRational av_get_time_base_q(void); > > +#define AV_FOURCC_MAX_STRING_SIZE 32 > + > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) > + > +/** > + * 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); Maybe this could go in avstring.h instead. > + > /** > * @} > * @} > 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) == '_')) > + > + for (i = 0; i < 4; i++) { > + len = snprintf(buf, buf_size, > + TAG_PRINT(fourcc & 0xFF) ? "%c" : "[%d]", fourcc & 0xFF); > + buf += len; > + buf_size = buf_size > len ? buf_size - len : 0; > + fourcc >>= 8; > + } > + return orig_buf; > +} > + > AVRational av_get_time_base_q(void) > { > return (AVRational){1, AV_TIME_BASE}; > diff --git a/libavutil/version.h b/libavutil/version.h > index 3c86c26638..d6d78e7dc2 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -79,7 +79,7 @@ > */ > > #define LIBAVUTIL_VERSION_MAJOR 55 > -#define LIBAVUTIL_VERSION_MINOR 51 > +#define LIBAVUTIL_VERSION_MINOR 52 > #define LIBAVUTIL_VERSION_MICRO 100 > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ >
On Mon, Mar 27, 2017 at 11:16:24AM -0300, James Almer wrote: > On 3/27/2017 4:51 AM, Clément Bœsch wrote: > > --- > > doc/APIchanges | 4 ++++ > > libavutil/avutil.h | 14 ++++++++++++++ > > libavutil/utils.c | 21 +++++++++++++++++++++ > > libavutil/version.h | 2 +- > > 4 files changed, 40 insertions(+), 1 deletion(-) > > > > diff --git a/doc/APIchanges b/doc/APIchanges > > index 6aaa9adceb..4736e3e6fc 100644 > > --- a/doc/APIchanges > > +++ b/doc/APIchanges > > @@ -15,6 +15,10 @@ libavutil: 2015-08-28 > > > > API changes, most recent first: > > > > +2017-03-xx - xxxxxxx - lavu 55.52.100 - avutil.h > > + add av_fourcc_make_string() function and av_4cc2str() macro to replace > > + av_get_codec_tag_string() from lavc. > > + > > 2017-03-xx - xxxxxxx - lavc 57.85.101 - avcodec.h > > vdpau hardware accelerated decoding now supports the new hwaccel API, which > > can create the decoder context and allocate hardware frame automatically. > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > > index e9aaa03722..98100fdcc5 100644 > > --- a/libavutil/avutil.h > > +++ b/libavutil/avutil.h > > @@ -343,6 +343,20 @@ FILE *av_fopen_utf8(const char *path, const char *mode); > > */ > > AVRational av_get_time_base_q(void); > > > > +#define AV_FOURCC_MAX_STRING_SIZE 32 > > + > > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) > > + > > +/** > > + * 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); > > Maybe this could go in avstring.h instead. > FourCC in strings? I feel like FourCC are more overall A/V specific than string related. avstring is more about actually parsing string or craft them from other strings. Similarly, we didn't put timestamp string crafting in avstring, av_ts2str() and its friends have their own header. If more people feel like FourCC code should be there I'll move it, otherwise I prefer if it stays here.
Hi Clément, I think your idea of introducing this function and the convenience macro is good. Below are some comments, feel free to ignore. On 2017-03-27 09:51 +0200, Clément Bœsch wrote: > --- > doc/APIchanges | 4 ++++ > libavutil/avutil.h | 14 ++++++++++++++ > libavutil/utils.c | 21 +++++++++++++++++++++ > libavutil/version.h | 2 +- > 4 files changed, 40 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 6aaa9adceb..4736e3e6fc 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -15,6 +15,10 @@ libavutil: 2015-08-28 > > API changes, most recent first: > > +2017-03-xx - xxxxxxx - lavu 55.52.100 - avutil.h > + add av_fourcc_make_string() function and av_4cc2str() macro to replace > + av_get_codec_tag_string() from lavc. > + > 2017-03-xx - xxxxxxx - lavc 57.85.101 - avcodec.h > vdpau hardware accelerated decoding now supports the new hwaccel API, which > can create the decoder context and allocate hardware frame automatically. > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > index e9aaa03722..98100fdcc5 100644 > --- a/libavutil/avutil.h > +++ b/libavutil/avutil.h > @@ -343,6 +343,20 @@ FILE *av_fopen_utf8(const char *path, const char *mode); > */ > AVRational av_get_time_base_q(void); > > +#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. > + > +#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. > + > +/** > + * 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. > + > + for (i = 0; i < 4; i++) { > + len = snprintf(buf, buf_size, > + TAG_PRINT(fourcc & 0xFF) ? "%c" : "[%d]", fourcc & 0xFF); > + buf += len; > + buf_size = buf_size > len ? buf_size - len : 0; > + fourcc >>= 8; > + } > + return orig_buf; > +} > + > AVRational av_get_time_base_q(void) > { > return (AVRational){1, AV_TIME_BASE}; [...] Alexander
On Mon, Mar 27, 2017 at 09:51:54AM +0200, Clément Bœsch wrote: > --- > doc/APIchanges | 4 ++++ > libavutil/avutil.h | 14 ++++++++++++++ > libavutil/utils.c | 21 +++++++++++++++++++++ > libavutil/version.h | 2 +- > 4 files changed, 40 insertions(+), 1 deletion(-) > I'll start pushing that patchset tomorrow if I see no objection. [...]
2017-03-28 17:31 GMT+02:00 Clément Bœsch <u@pkh.me>: > On Mon, Mar 27, 2017 at 09:51:54AM +0200, Clément Bœsch wrote: >> --- >> doc/APIchanges | 4 ++++ >> libavutil/avutil.h | 14 ++++++++++++++ >> libavutil/utils.c | 21 +++++++++++++++++++++ >> libavutil/version.h | 2 +- >> 4 files changed, 40 insertions(+), 1 deletion(-) >> > > I'll start pushing that patchset tomorrow if I see no objection. > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) Sorry, I really don't understand: If the preferred name for this function is "av_4cc2str", why not name it av_4cc2str()? Carl Eugen
On Wed, Mar 29, 2017 at 09:31:57AM +0200, Carl Eugen Hoyos wrote: > 2017-03-28 17:31 GMT+02:00 Clément Bœsch <u@pkh.me>: > > On Mon, Mar 27, 2017 at 09:51:54AM +0200, Clément Bœsch wrote: > >> --- > >> doc/APIchanges | 4 ++++ > >> libavutil/avutil.h | 14 ++++++++++++++ > >> libavutil/utils.c | 21 +++++++++++++++++++++ > >> libavutil/version.h | 2 +- > >> 4 files changed, 40 insertions(+), 1 deletion(-) > >> > > > > I'll start pushing that patchset tomorrow if I see no objection. > > > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) > > Sorry, I really don't understand: > If the preferred name for this function is "av_4cc2str", why not name > it av_4cc2str()? > I don't understand your question: are you asking why there is a macro and a function with different names? BTW, I'm renaming the macro av_4cc2str() to av_fourcc2str() as it seems more people prefer that name.
2017-03-29 9:43 GMT+02:00 Clément Bœsch <u@pkh.me>: > On Wed, Mar 29, 2017 at 09:31:57AM +0200, Carl Eugen Hoyos wrote: >> 2017-03-28 17:31 GMT+02:00 Clément Bœsch <u@pkh.me>: >> > On Mon, Mar 27, 2017 at 09:51:54AM +0200, Clément Bœsch wrote: >> >> --- >> >> doc/APIchanges | 4 ++++ >> >> libavutil/avutil.h | 14 ++++++++++++++ >> >> libavutil/utils.c | 21 +++++++++++++++++++++ >> >> libavutil/version.h | 2 +- >> >> 4 files changed, 40 insertions(+), 1 deletion(-) >> >> >> > >> > I'll start pushing that patchset tomorrow if I see no objection. >> >> > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) >> >> Sorry, I really don't understand: >> If the preferred name for this function is "av_4cc2str", why not name >> it av_4cc2str()? >> > > I don't understand your question: are you asking why there is a macro > and a function with different names? Yes, why is a function with the preferred name not sufficient? Carl Eugen
On Wed, Mar 29, 2017 at 09:52:51AM +0200, Carl Eugen Hoyos wrote: > 2017-03-29 9:43 GMT+02:00 Clément Bœsch <u@pkh.me>: > > On Wed, Mar 29, 2017 at 09:31:57AM +0200, Carl Eugen Hoyos wrote: > >> 2017-03-28 17:31 GMT+02:00 Clément Bœsch <u@pkh.me>: > >> > On Mon, Mar 27, 2017 at 09:51:54AM +0200, Clément Bœsch wrote: > >> >> --- > >> >> doc/APIchanges | 4 ++++ > >> >> libavutil/avutil.h | 14 ++++++++++++++ > >> >> libavutil/utils.c | 21 +++++++++++++++++++++ > >> >> libavutil/version.h | 2 +- > >> >> 4 files changed, 40 insertions(+), 1 deletion(-) > >> >> > >> > > >> > I'll start pushing that patchset tomorrow if I see no objection. > >> > >> > +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) > >> > >> Sorry, I really don't understand: > >> If the preferred name for this function is "av_4cc2str", why not name > >> it av_4cc2str()? > >> > > > > I don't understand your question: are you asking why there is a macro > > and a function with different names? > > Yes, why is a function with the preferred name not sufficient? > Because we save the user the need to create a buffer thanks the compound literal in the macro which can only exist in a macro. It's exactly like av_ts2str().
Patchset pushed with av_fourcc2str() macro name. [...]
diff --git a/doc/APIchanges b/doc/APIchanges index 6aaa9adceb..4736e3e6fc 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -15,6 +15,10 @@ libavutil: 2015-08-28 API changes, most recent first: +2017-03-xx - xxxxxxx - lavu 55.52.100 - avutil.h + add av_fourcc_make_string() function and av_4cc2str() macro to replace + av_get_codec_tag_string() from lavc. + 2017-03-xx - xxxxxxx - lavc 57.85.101 - avcodec.h vdpau hardware accelerated decoding now supports the new hwaccel API, which can create the decoder context and allocate hardware frame automatically. diff --git a/libavutil/avutil.h b/libavutil/avutil.h index e9aaa03722..98100fdcc5 100644 --- a/libavutil/avutil.h +++ b/libavutil/avutil.h @@ -343,6 +343,20 @@ FILE *av_fopen_utf8(const char *path, const char *mode); */ AVRational av_get_time_base_q(void); +#define AV_FOURCC_MAX_STRING_SIZE 32 + +#define av_4cc2str(fourcc) av_fourcc_make_string((char[AV_FOURCC_MAX_STRING_SIZE]){0}, fourcc) + +/** + * 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) == '_')) + + for (i = 0; i < 4; i++) { + len = snprintf(buf, buf_size, + TAG_PRINT(fourcc & 0xFF) ? "%c" : "[%d]", fourcc & 0xFF); + buf += len; + buf_size = buf_size > len ? buf_size - len : 0; + fourcc >>= 8; + } + return orig_buf; +} + AVRational av_get_time_base_q(void) { return (AVRational){1, AV_TIME_BASE}; diff --git a/libavutil/version.h b/libavutil/version.h index 3c86c26638..d6d78e7dc2 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 55 -#define LIBAVUTIL_VERSION_MINOR 51 +#define LIBAVUTIL_VERSION_MINOR 52 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \