| Message ID | 20170327075203.7499-2-u@pkh.me |
|---|---|
| State | Accepted |
| Headers | show |
2017-03-27 9:51 GMT+02:00 Clément Bœsch <u@pkh.me>: > --- > libavcodec/avcodec.h | 5 +++++ > libavcodec/version.h | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index 4f3303366f..5c891b531a 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -5667,6 +5667,7 @@ attribute_deprecated > void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > #endif > > +#if FF_API_TAG_STRING > /** > * Put a string representing the codec tag codec_tag in buf. > * > @@ -5675,8 +5676,12 @@ void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > * @param codec_tag codec tag to assign > * @return the length of the string that would have been generated if > * enough space had been available, excluding the trailing null > + * > + * @deprecated see av_fourcc_make_string() and av_4cc2str(). > */ > +attribute_deprecated > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > +#endif Sorry if I missed it: Please explain what the problem is with the current code. Carl Eugen
On Mon, Mar 27, 2017 at 10:00:09AM +0200, Carl Eugen Hoyos wrote: > 2017-03-27 9:51 GMT+02:00 Clément Bœsch <u@pkh.me>: > > --- > > libavcodec/avcodec.h | 5 +++++ > > libavcodec/version.h | 3 +++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > > index 4f3303366f..5c891b531a 100644 > > --- a/libavcodec/avcodec.h > > +++ b/libavcodec/avcodec.h > > @@ -5667,6 +5667,7 @@ attribute_deprecated > > void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > > #endif > > > > +#if FF_API_TAG_STRING > > /** > > * Put a string representing the codec tag codec_tag in buf. > > * > > @@ -5675,8 +5676,12 @@ void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > > * @param codec_tag codec tag to assign > > * @return the length of the string that would have been generated if > > * enough space had been available, excluding the trailing null > > + * > > + * @deprecated see av_fourcc_make_string() and av_4cc2str(). > > */ > > +attribute_deprecated > > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > > +#endif > > Sorry if I missed it: > Please explain what the problem is with the current code. > I can't make av_4cc2str() with that prototype. I take the opportunity to move it to lavu since fourcc are related to format and codecs.
2017-03-27 10:09 GMT+02:00 Clément Bœsch <u@pkh.me>: >> > +attribute_deprecated >> > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); >> > +#endif >> >> Sorry if I missed it: >> Please explain what the problem is with the current code. > > I can't make av_4cc2str() with that prototype. I take the opportunity to > move it to lavu since fourcc are related to format and codecs. What about moving av_get_codec_tag_string() to lavu if you need it there? Carl Eugen
On Mon, Mar 27, 2017 at 10:12:46AM +0200, Carl Eugen Hoyos wrote: > 2017-03-27 10:09 GMT+02:00 Clément Bœsch <u@pkh.me>: > > >> > +attribute_deprecated > >> > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > >> > +#endif > >> > >> Sorry if I missed it: > >> Please explain what the problem is with the current code. > > > > I can't make av_4cc2str() with that prototype. I take the opportunity to > > move it to lavu since fourcc are related to format and codecs. > > What about moving av_get_codec_tag_string() to lavu if you need > it there? Please also see my first sentence: I can't use av_get_codec_tag_string() with its current prototype anyway.
2017-03-27 10:21 GMT+02:00 Clément Bœsch <u@pkh.me>: > On Mon, Mar 27, 2017 at 10:12:46AM +0200, Carl Eugen Hoyos wrote: >> 2017-03-27 10:09 GMT+02:00 Clément Bœsch <u@pkh.me>: >> >> >> > +attribute_deprecated >> >> > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); >> >> > +#endif >> >> >> >> Sorry if I missed it: >> >> Please explain what the problem is with the current code. >> > >> > I can't make av_4cc2str() with that prototype. I take the opportunity to >> > move it to lavu since fourcc are related to format and codecs. >> >> What about moving av_get_codec_tag_string() to lavu if you need >> it there? > > Please also see my first sentence: I can't use av_get_codec_tag_string() > with its current prototype anyway. But could you explain why you cannot use it? (Sorry if it is obvious, I don't see it.) Changing API is always a big problem that is typically underestimated by FFmpeg developers. Therefore every change should be discussed and avoided if possible (imo if somehow possible). Carl Eugen
On Mon, Mar 27, 2017 at 10:47:55AM +0200, Carl Eugen Hoyos wrote: > 2017-03-27 10:21 GMT+02:00 Clément Bœsch <u@pkh.me>: > > On Mon, Mar 27, 2017 at 10:12:46AM +0200, Carl Eugen Hoyos wrote: > >> 2017-03-27 10:09 GMT+02:00 Clément Bœsch <u@pkh.me>: > >> > >> >> > +attribute_deprecated > >> >> > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > >> >> > +#endif > >> >> > >> >> Sorry if I missed it: > >> >> Please explain what the problem is with the current code. > >> > > >> > I can't make av_4cc2str() with that prototype. I take the opportunity to > >> > move it to lavu since fourcc are related to format and codecs. > >> > >> What about moving av_get_codec_tag_string() to lavu if you need > >> it there? > > > > Please also see my first sentence: I can't use av_get_codec_tag_string() > > with its current prototype anyway. > > But could you explain why you cannot use it? > (Sorry if it is obvious, I don't see it.) > Because we want to be able to do print_func("%s", av_4cc2str(tag)) and av_get_codec_tag_string() returns a size instead of the buffer. > Changing API is always a big problem that is typically underestimated > by FFmpeg developers. Therefore every change should be discussed > and avoided if possible (imo if somehow possible). > I'm aware of that
2017-03-27 10:52 GMT+02:00 Clément Bœsch <u@pkh.me>: > On Mon, Mar 27, 2017 at 10:47:55AM +0200, Carl Eugen Hoyos wrote: >> 2017-03-27 10:21 GMT+02:00 Clément Bœsch <u@pkh.me>: >> > On Mon, Mar 27, 2017 at 10:12:46AM +0200, Carl Eugen Hoyos wrote: >> >> 2017-03-27 10:09 GMT+02:00 Clément Bœsch <u@pkh.me>: >> >> >> >> >> > +attribute_deprecated >> >> >> > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); >> >> >> > +#endif >> >> >> >> >> >> Sorry if I missed it: >> >> >> Please explain what the problem is with the current code. >> >> > >> >> > I can't make av_4cc2str() with that prototype. I take the opportunity to >> >> > move it to lavu since fourcc are related to format and codecs. >> >> >> >> What about moving av_get_codec_tag_string() to lavu if you need >> >> it there? >> > >> > Please also see my first sentence: I can't use av_get_codec_tag_string() >> > with its current prototype anyway. >> >> But could you explain why you cannot use it? >> (Sorry if it is obvious, I don't see it.) >> > > Because we want to be able to do print_func("%s", av_4cc2str(tag)) and > av_get_codec_tag_string() returns a size instead of the buffer. Is the compiler doing anything stupid for the current code? >> Changing API is always a big problem that is typically underestimated >> by FFmpeg developers. Therefore every change should be discussed >> and avoided if possible (imo if somehow possible). > > I'm aware of that Then why do you propose to deprecate a public function? (Or are you just aware of my opinion?) Carl Eugen
On Mon, Mar 27, 2017 at 10:56:26AM +0200, Carl Eugen Hoyos wrote: > 2017-03-27 10:52 GMT+02:00 Clément Bœsch <u@pkh.me>: > > On Mon, Mar 27, 2017 at 10:47:55AM +0200, Carl Eugen Hoyos wrote: > >> 2017-03-27 10:21 GMT+02:00 Clément Bœsch <u@pkh.me>: > >> > On Mon, Mar 27, 2017 at 10:12:46AM +0200, Carl Eugen Hoyos wrote: > >> >> 2017-03-27 10:09 GMT+02:00 Clément Bœsch <u@pkh.me>: > >> >> > >> >> >> > +attribute_deprecated > >> >> >> > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > >> >> >> > +#endif > >> >> >> > >> >> >> Sorry if I missed it: > >> >> >> Please explain what the problem is with the current code. > >> >> > > >> >> > I can't make av_4cc2str() with that prototype. I take the opportunity to > >> >> > move it to lavu since fourcc are related to format and codecs. > >> >> > >> >> What about moving av_get_codec_tag_string() to lavu if you need > >> >> it there? > >> > > >> > Please also see my first sentence: I can't use av_get_codec_tag_string() > >> > with its current prototype anyway. > >> > >> But could you explain why you cannot use it? > >> (Sorry if it is obvious, I don't see it.) > >> > > > > Because we want to be able to do print_func("%s", av_4cc2str(tag)) and > > av_get_codec_tag_string() returns a size instead of the buffer. > > Is the compiler doing anything stupid for the current code? > No but it makes the code much simpler that way. > >> Changing API is always a big problem that is typically underestimated > >> by FFmpeg developers. Therefore every change should be discussed > >> and avoided if possible (imo if somehow possible). > > > > I'm aware of that > > Then why do you propose to deprecate a public function? > (Or are you just aware of my opinion?) > I'm aware it's a problem, but in the long term it's also an improvement for both our codebase and our users.
On Mon, 27 Mar 2017 09:51:55 +0200 Clément Bœsch <u@pkh.me> wrote: > --- > libavcodec/avcodec.h | 5 +++++ > libavcodec/version.h | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index 4f3303366f..5c891b531a 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -5667,6 +5667,7 @@ attribute_deprecated > void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > #endif > > +#if FF_API_TAG_STRING > /** > * Put a string representing the codec tag codec_tag in buf. > * > @@ -5675,8 +5676,12 @@ void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > * @param codec_tag codec tag to assign > * @return the length of the string that would have been generated if > * enough space had been available, excluding the trailing null > + * > + * @deprecated see av_fourcc_make_string() and av_4cc2str(). > */ > +attribute_deprecated > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > +#endif > > void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode); > > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 37defbc365..8dea5cb97b 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -235,6 +235,9 @@ > #ifndef FF_API_MERGE_SD_API > #define FF_API_MERGE_SD_API (LIBAVCODEC_VERSION_MAJOR < 59) > #endif > +#ifndef FF_API_TAG_STRING > +#define FF_API_TAG_STRING (LIBAVCODEC_VERSION_MAJOR < 59) > +#endif > > > #endif /* AVCODEC_VERSION_H */ To be honest, it's not really necessary to deprecate this. You could just make it call the "new" code (or the other way around).
On Mon, Mar 27, 2017 at 04:34:18PM +0200, wm4 wrote: > On Mon, 27 Mar 2017 09:51:55 +0200 > Clément Bœsch <u@pkh.me> wrote: > > > --- > > libavcodec/avcodec.h | 5 +++++ > > libavcodec/version.h | 3 +++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > > index 4f3303366f..5c891b531a 100644 > > --- a/libavcodec/avcodec.h > > +++ b/libavcodec/avcodec.h > > @@ -5667,6 +5667,7 @@ attribute_deprecated > > void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > > #endif > > > > +#if FF_API_TAG_STRING > > /** > > * Put a string representing the codec tag codec_tag in buf. > > * > > @@ -5675,8 +5676,12 @@ void avcodec_set_dimensions(AVCodecContext *s, int width, int height); > > * @param codec_tag codec tag to assign > > * @return the length of the string that would have been generated if > > * enough space had been available, excluding the trailing null > > + * > > + * @deprecated see av_fourcc_make_string() and av_4cc2str(). > > */ > > +attribute_deprecated > > size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); > > +#endif > > > > void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode); > > > > diff --git a/libavcodec/version.h b/libavcodec/version.h > > index 37defbc365..8dea5cb97b 100644 > > --- a/libavcodec/version.h > > +++ b/libavcodec/version.h > > @@ -235,6 +235,9 @@ > > #ifndef FF_API_MERGE_SD_API > > #define FF_API_MERGE_SD_API (LIBAVCODEC_VERSION_MAJOR < 59) > > #endif > > +#ifndef FF_API_TAG_STRING > > +#define FF_API_TAG_STRING (LIBAVCODEC_VERSION_MAJOR < 59) > > +#endif > > > > > > #endif /* AVCODEC_VERSION_H */ > > To be honest, it's not really necessary to deprecate this. You could > just make it call the "new" code (or the other way around). Yes, but we would end up with redundant functions with different names, and I think we already have way more than we want to maintain. If I don't deprecate it, I will have to use the new one within the old with, it will just be a wrapper, lavc specific, with not much purpose except being a source of confusion and inconsistency (because it may be used again in our code base).
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index 4f3303366f..5c891b531a 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -5667,6 +5667,7 @@ attribute_deprecated void avcodec_set_dimensions(AVCodecContext *s, int width, int height); #endif +#if FF_API_TAG_STRING /** * Put a string representing the codec tag codec_tag in buf. * @@ -5675,8 +5676,12 @@ void avcodec_set_dimensions(AVCodecContext *s, int width, int height); * @param codec_tag codec tag to assign * @return the length of the string that would have been generated if * enough space had been available, excluding the trailing null + * + * @deprecated see av_fourcc_make_string() and av_4cc2str(). */ +attribute_deprecated size_t av_get_codec_tag_string(char *buf, size_t buf_size, unsigned int codec_tag); +#endif void avcodec_string(char *buf, int buf_size, AVCodecContext *enc, int encode); diff --git a/libavcodec/version.h b/libavcodec/version.h index 37defbc365..8dea5cb97b 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -235,6 +235,9 @@ #ifndef FF_API_MERGE_SD_API #define FF_API_MERGE_SD_API (LIBAVCODEC_VERSION_MAJOR < 59) #endif +#ifndef FF_API_TAG_STRING +#define FF_API_TAG_STRING (LIBAVCODEC_VERSION_MAJOR < 59) +#endif #endif /* AVCODEC_VERSION_H */