Message ID | 20171118184713.11490-4-sw@jkqxz.net |
---|---|
State | Superseded |
Headers | show |
On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: > --- > doc/APIchanges | 4 ++++ > libavcodec/avcodec.h | 13 +++++++++++++ > libavcodec/utils.c | 16 +++++----------- > libavcodec/version.h | 3 +++ > 4 files changed, 25 insertions(+), 11 deletions(-) > > > int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) > { > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 5b25a9a8ac..693f67386c 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -123,6 +123,9 @@ > #ifndef FF_API_CODEC_GET_SET > #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) > #endif > +#ifndef FF_API_USER_VISIBLE_AVHWACCEL > +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) > 60? That's 2 bumps away, this should be 59, no?
On 18/11/17 19:00, Rostislav Pehlivanov wrote: > On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: > >> --- >> doc/APIchanges | 4 ++++ >> libavcodec/avcodec.h | 13 +++++++++++++ >> libavcodec/utils.c | 16 +++++----------- >> libavcodec/version.h | 3 +++ >> 4 files changed, 25 insertions(+), 11 deletions(-) >> >> >> int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) >> { >> diff --git a/libavcodec/version.h b/libavcodec/version.h >> index 5b25a9a8ac..693f67386c 100644 >> --- a/libavcodec/version.h >> +++ b/libavcodec/version.h >> @@ -123,6 +123,9 @@ >> #ifndef FF_API_CODEC_GET_SET >> #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) >> #endif >> +#ifndef FF_API_USER_VISIBLE_AVHWACCEL >> +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) >> > > 60? That's 2 bumps away, this should be 59, no? Two bumps away is the usual number, isn't it? Or does it count as not-bumped-yet for this purpose because we are still in the unstable period? - Mark
On 11/18/2017 4:07 PM, Mark Thompson wrote: > On 18/11/17 19:00, Rostislav Pehlivanov wrote: >> On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: >> >>> --- >>> doc/APIchanges | 4 ++++ >>> libavcodec/avcodec.h | 13 +++++++++++++ >>> libavcodec/utils.c | 16 +++++----------- >>> libavcodec/version.h | 3 +++ >>> 4 files changed, 25 insertions(+), 11 deletions(-) >>> >>> >>> int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) >>> { >>> diff --git a/libavcodec/version.h b/libavcodec/version.h >>> index 5b25a9a8ac..693f67386c 100644 >>> --- a/libavcodec/version.h >>> +++ b/libavcodec/version.h >>> @@ -123,6 +123,9 @@ >>> #ifndef FF_API_CODEC_GET_SET >>> #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) >>> #endif >>> +#ifndef FF_API_USER_VISIBLE_AVHWACCEL >>> +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) >>> >> >> 60? That's 2 bumps away, this should be 59, no? > > Two bumps away is the usual number, isn't it? Or does it count as not-bumped-yet for this purpose because we are still in the unstable period? The number doesn't matter as long as the condition evaluates to true when it's meant to. If you use 59 and there's a bump before the two years period for this specific deprecation is over, then the line will have to be changed to 60 during said bump to make sure the API doesn't get disabled. However using 60 would mean that the line can be left alone. The bump that changes the major to 60 is unlikely to happen before the two year mark. So it's fine as is. No need to change it. > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On 18 November 2017 at 19:17, James Almer <jamrial@gmail.com> wrote: > On 11/18/2017 4:07 PM, Mark Thompson wrote: > > On 18/11/17 19:00, Rostislav Pehlivanov wrote: > >> On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: > >> > >>> --- > >>> doc/APIchanges | 4 ++++ > >>> libavcodec/avcodec.h | 13 +++++++++++++ > >>> libavcodec/utils.c | 16 +++++----------- > >>> libavcodec/version.h | 3 +++ > >>> 4 files changed, 25 insertions(+), 11 deletions(-) > >>> > >>> > >>> int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) > >>> { > >>> diff --git a/libavcodec/version.h b/libavcodec/version.h > >>> index 5b25a9a8ac..693f67386c 100644 > >>> --- a/libavcodec/version.h > >>> +++ b/libavcodec/version.h > >>> @@ -123,6 +123,9 @@ > >>> #ifndef FF_API_CODEC_GET_SET > >>> #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) > >>> #endif > >>> +#ifndef FF_API_USER_VISIBLE_AVHWACCEL > >>> +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) > >>> > >> > >> 60? That's 2 bumps away, this should be 59, no? > > > > Two bumps away is the usual number, isn't it? Or does it count as > not-bumped-yet for this purpose because we are still in the unstable period? > > The number doesn't matter as long as the condition evaluates to true > when it's meant to. > If you use 59 and there's a bump before the two years period for this > specific deprecation is over, then the line will have to be changed to > 60 during said bump to make sure the API doesn't get disabled. > However using 60 would mean that the line can be left alone. The bump > that changes the major to 60 is unlikely to happen before the two year > mark. > > So it's fine as is. No need to change it. > > It'll be forgotten and it'll take 4 years to remove. How is anyone supposed to know when something's meant to be removed aside from the MAJOR version?
On 11/18/2017 5:03 PM, Rostislav Pehlivanov wrote: > On 18 November 2017 at 19:17, James Almer <jamrial@gmail.com> wrote: > >> On 11/18/2017 4:07 PM, Mark Thompson wrote: >>> On 18/11/17 19:00, Rostislav Pehlivanov wrote: >>>> On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: >>>> >>>>> --- >>>>> doc/APIchanges | 4 ++++ >>>>> libavcodec/avcodec.h | 13 +++++++++++++ >>>>> libavcodec/utils.c | 16 +++++----------- >>>>> libavcodec/version.h | 3 +++ >>>>> 4 files changed, 25 insertions(+), 11 deletions(-) >>>>> >>>>> >>>>> int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) >>>>> { >>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h >>>>> index 5b25a9a8ac..693f67386c 100644 >>>>> --- a/libavcodec/version.h >>>>> +++ b/libavcodec/version.h >>>>> @@ -123,6 +123,9 @@ >>>>> #ifndef FF_API_CODEC_GET_SET >>>>> #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) >>>>> #endif >>>>> +#ifndef FF_API_USER_VISIBLE_AVHWACCEL >>>>> +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) >>>>> >>>> >>>> 60? That's 2 bumps away, this should be 59, no? >>> >>> Two bumps away is the usual number, isn't it? Or does it count as >> not-bumped-yet for this purpose because we are still in the unstable period? >> >> The number doesn't matter as long as the condition evaluates to true >> when it's meant to. >> If you use 59 and there's a bump before the two years period for this >> specific deprecation is over, then the line will have to be changed to >> 60 during said bump to make sure the API doesn't get disabled. >> However using 60 would mean that the line can be left alone. The bump >> that changes the major to 60 is unlikely to happen before the two year >> mark. >> >> So it's fine as is. No need to change it. >> >> > It'll be forgotten and it'll take 4 years to remove. How is anyone supposed > to know when something's meant to be removed aside from the MAJOR version? By git blaming version.h to look at the exact date of the commits that deprecated every listed API, same as it's done every time a bump takes place. As long as whoever handles the bump does it properly then it shouldn't make a difference. But if you feel more comfortable making it 59 then it can be changed. Both values are fine.
On 18 November 2017 at 20:11, James Almer <jamrial@gmail.com> wrote: > On 11/18/2017 5:03 PM, Rostislav Pehlivanov wrote: > > On 18 November 2017 at 19:17, James Almer <jamrial@gmail.com> wrote: > > > >> On 11/18/2017 4:07 PM, Mark Thompson wrote: > >>> On 18/11/17 19:00, Rostislav Pehlivanov wrote: > >>>> On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: > >>>> > >>>>> --- > >>>>> doc/APIchanges | 4 ++++ > >>>>> libavcodec/avcodec.h | 13 +++++++++++++ > >>>>> libavcodec/utils.c | 16 +++++----------- > >>>>> libavcodec/version.h | 3 +++ > >>>>> 4 files changed, 25 insertions(+), 11 deletions(-) > >>>>> > >>>>> > >>>>> int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) > >>>>> { > >>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h > >>>>> index 5b25a9a8ac..693f67386c 100644 > >>>>> --- a/libavcodec/version.h > >>>>> +++ b/libavcodec/version.h > >>>>> @@ -123,6 +123,9 @@ > >>>>> #ifndef FF_API_CODEC_GET_SET > >>>>> #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) > >>>>> #endif > >>>>> +#ifndef FF_API_USER_VISIBLE_AVHWACCEL > >>>>> +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < > 60) > >>>>> > >>>> > >>>> 60? That's 2 bumps away, this should be 59, no? > >>> > >>> Two bumps away is the usual number, isn't it? Or does it count as > >> not-bumped-yet for this purpose because we are still in the unstable > period? > >> > >> The number doesn't matter as long as the condition evaluates to true > >> when it's meant to. > >> If you use 59 and there's a bump before the two years period for this > >> specific deprecation is over, then the line will have to be changed to > >> 60 during said bump to make sure the API doesn't get disabled. > >> However using 60 would mean that the line can be left alone. The bump > >> that changes the major to 60 is unlikely to happen before the two year > >> mark. > >> > >> So it's fine as is. No need to change it. > >> > >> > > It'll be forgotten and it'll take 4 years to remove. How is anyone > supposed > > to know when something's meant to be removed aside from the MAJOR > version? > > By git blaming version.h to look at the exact date of the commits that > deprecated every listed API, same as it's done every time a bump takes > place. > > As long as whoever handles the bump does it properly then it shouldn't > make a difference. But if you feel more comfortable making it 59 then it > can be changed. Both values are fine. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > I'd be more comfortable with 59, that should save effort on the part of the person doing the bump.
On 18/11/17 20:18, Rostislav Pehlivanov wrote: > On 18 November 2017 at 20:11, James Almer <jamrial@gmail.com> wrote: > >> On 11/18/2017 5:03 PM, Rostislav Pehlivanov wrote: >>> On 18 November 2017 at 19:17, James Almer <jamrial@gmail.com> wrote: >>> >>>> On 11/18/2017 4:07 PM, Mark Thompson wrote: >>>>> On 18/11/17 19:00, Rostislav Pehlivanov wrote: >>>>>> On 18 November 2017 at 18:47, Mark Thompson <sw@jkqxz.net> wrote: >>>>>> >>>>>>> --- >>>>>>> doc/APIchanges | 4 ++++ >>>>>>> libavcodec/avcodec.h | 13 +++++++++++++ >>>>>>> libavcodec/utils.c | 16 +++++----------- >>>>>>> libavcodec/version.h | 3 +++ >>>>>>> 4 files changed, 25 insertions(+), 11 deletions(-) >>>>>>> >>>>>>> >>>>>>> int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) >>>>>>> { >>>>>>> diff --git a/libavcodec/version.h b/libavcodec/version.h >>>>>>> index 5b25a9a8ac..693f67386c 100644 >>>>>>> --- a/libavcodec/version.h >>>>>>> +++ b/libavcodec/version.h >>>>>>> @@ -123,6 +123,9 @@ >>>>>>> #ifndef FF_API_CODEC_GET_SET >>>>>>> #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) >>>>>>> #endif >>>>>>> +#ifndef FF_API_USER_VISIBLE_AVHWACCEL >>>>>>> +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < >> 60) >>>>>>> >>>>>> >>>>>> 60? That's 2 bumps away, this should be 59, no? >>>>> >>>>> Two bumps away is the usual number, isn't it? Or does it count as >>>> not-bumped-yet for this purpose because we are still in the unstable >> period? >>>> >>>> The number doesn't matter as long as the condition evaluates to true >>>> when it's meant to. >>>> If you use 59 and there's a bump before the two years period for this >>>> specific deprecation is over, then the line will have to be changed to >>>> 60 during said bump to make sure the API doesn't get disabled. >>>> However using 60 would mean that the line can be left alone. The bump >>>> that changes the major to 60 is unlikely to happen before the two year >>>> mark. >>>> >>>> So it's fine as is. No need to change it. >>>> >>>> >>> It'll be forgotten and it'll take 4 years to remove. How is anyone >> supposed >>> to know when something's meant to be removed aside from the MAJOR >> version? >> >> By git blaming version.h to look at the exact date of the commits that >> deprecated every listed API, same as it's done every time a bump takes >> place. >> >> As long as whoever handles the bump does it properly then it shouldn't >> make a difference. But if you feel more comfortable making it 59 then it >> can be changed. Both values are fine. >> > > I'd be more comfortable with 59, that should save effort on the part of the > person doing the bump. Ok, changed to 59. Thanks, - Mark
On Sat, 18 Nov 2017 18:47:04 +0000 Mark Thompson <sw@jkqxz.net> wrote: > --- > doc/APIchanges | 4 ++++ > libavcodec/avcodec.h | 13 +++++++++++++ > libavcodec/utils.c | 16 +++++----------- > libavcodec/version.h | 3 +++ > 4 files changed, 25 insertions(+), 11 deletions(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index a3c5e21765..547d0019ba 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -16,6 +16,10 @@ libavutil: 2017-10-21 > API changes, most recent first: > > 2017-xx-xx - xxxxxxx - lavc 58.x+1.0 - avcodec.h > + Deprecate user visibility of the AVHWAccel structure and the > functions > + av_register_hwaccel() and av_hwaccel_next(). > + > +2017-xx-xx - xxxxxxx - lavc 58.x+1.0 - avcodec.h > Add AVCodecHWConfig and avcodec_get_hw_config(). > > 2017-xx-xx - xxxxxxx - lavc 58.3.100 - avcodec.h > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h > index fb18f46ea2..c05dbfe777 100644 > --- a/libavcodec/avcodec.h > +++ b/libavcodec/avcodec.h > @@ -3489,6 +3489,10 @@ const AVCodecHWConfig > *avcodec_get_hw_config(const AVCodec *codec, int index); > /** > * @defgroup lavc_hwaccel AVHWAccel > + * > + * @note Nothing in this structure should be accessed by the user. > At some > + * point in future it will not be externally visible at all. > + * > * @{ > */ > typedef struct AVHWAccel { > @@ -5874,17 +5878,26 @@ void av_fast_padded_mallocz(void *ptr, > unsigned int *size, size_t min_size); */ > unsigned int av_xiphlacing(unsigned char *s, unsigned int v); > > +#if FF_API_USER_VISIBLE_AVHWACCEL > /** > * Register the hardware accelerator hwaccel. > + * > + * @deprecated This function doesn't do anything. > */ > +attribute_deprecated > void av_register_hwaccel(AVHWAccel *hwaccel); > > /** > * If hwaccel is NULL, returns the first registered hardware > accelerator, > * if hwaccel is non-NULL, returns the next registered hardware > accelerator > * after hwaccel, or NULL if hwaccel is the last one. > + * > + * @deprecated AVHWaccel structures contain no user-serviceable > parts, so > + * this function should not be used. > */ > +attribute_deprecated > AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel); > +#endif > > > /** > diff --git a/libavcodec/utils.c b/libavcodec/utils.c > index 01bf7556f7..417ebcb770 100644 > --- a/libavcodec/utils.c > +++ b/libavcodec/utils.c > @@ -1897,22 +1897,16 @@ const AVCodecHWConfig > *avcodec_get_hw_config(const AVCodec *codec, int index) return > &codec->hw_configs[index]->public; } > > -static AVHWAccel *first_hwaccel = NULL; > -static AVHWAccel **last_hwaccel = &first_hwaccel; > - > -void av_register_hwaccel(AVHWAccel *hwaccel) > +#if FF_API_USER_VISIBLE_AVHWACCEL > +AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel) > { > - AVHWAccel **p = last_hwaccel; > - hwaccel->next = NULL; > - while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, > hwaccel)) > - p = &(*p)->next; > - last_hwaccel = &hwaccel->next; > + return NULL; > } > > -AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel) > +void av_register_hwaccel(AVHWAccel *hwaccel) > { > - return hwaccel ? hwaccel->next : first_hwaccel; > } > +#endif > > int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) > { > diff --git a/libavcodec/version.h b/libavcodec/version.h > index 5b25a9a8ac..693f67386c 100644 > --- a/libavcodec/version.h > +++ b/libavcodec/version.h > @@ -123,6 +123,9 @@ > #ifndef FF_API_CODEC_GET_SET > #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) > #endif > +#ifndef FF_API_USER_VISIBLE_AVHWACCEL > +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) > +#endif > > > #endif /* AVCODEC_VERSION_H */ LGTM --phil
diff --git a/doc/APIchanges b/doc/APIchanges index a3c5e21765..547d0019ba 100644 --- a/doc/APIchanges +++ b/doc/APIchanges @@ -16,6 +16,10 @@ libavutil: 2017-10-21 API changes, most recent first: 2017-xx-xx - xxxxxxx - lavc 58.x+1.0 - avcodec.h + Deprecate user visibility of the AVHWAccel structure and the functions + av_register_hwaccel() and av_hwaccel_next(). + +2017-xx-xx - xxxxxxx - lavc 58.x+1.0 - avcodec.h Add AVCodecHWConfig and avcodec_get_hw_config(). 2017-xx-xx - xxxxxxx - lavc 58.3.100 - avcodec.h diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h index fb18f46ea2..c05dbfe777 100644 --- a/libavcodec/avcodec.h +++ b/libavcodec/avcodec.h @@ -3489,6 +3489,10 @@ const AVCodecHWConfig *avcodec_get_hw_config(const AVCodec *codec, int index); /** * @defgroup lavc_hwaccel AVHWAccel + * + * @note Nothing in this structure should be accessed by the user. At some + * point in future it will not be externally visible at all. + * * @{ */ typedef struct AVHWAccel { @@ -5874,17 +5878,26 @@ void av_fast_padded_mallocz(void *ptr, unsigned int *size, size_t min_size); */ unsigned int av_xiphlacing(unsigned char *s, unsigned int v); +#if FF_API_USER_VISIBLE_AVHWACCEL /** * Register the hardware accelerator hwaccel. + * + * @deprecated This function doesn't do anything. */ +attribute_deprecated void av_register_hwaccel(AVHWAccel *hwaccel); /** * If hwaccel is NULL, returns the first registered hardware accelerator, * if hwaccel is non-NULL, returns the next registered hardware accelerator * after hwaccel, or NULL if hwaccel is the last one. + * + * @deprecated AVHWaccel structures contain no user-serviceable parts, so + * this function should not be used. */ +attribute_deprecated AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel); +#endif /** diff --git a/libavcodec/utils.c b/libavcodec/utils.c index 01bf7556f7..417ebcb770 100644 --- a/libavcodec/utils.c +++ b/libavcodec/utils.c @@ -1897,22 +1897,16 @@ const AVCodecHWConfig *avcodec_get_hw_config(const AVCodec *codec, int index) return &codec->hw_configs[index]->public; } -static AVHWAccel *first_hwaccel = NULL; -static AVHWAccel **last_hwaccel = &first_hwaccel; - -void av_register_hwaccel(AVHWAccel *hwaccel) +#if FF_API_USER_VISIBLE_AVHWACCEL +AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel) { - AVHWAccel **p = last_hwaccel; - hwaccel->next = NULL; - while(*p || avpriv_atomic_ptr_cas((void * volatile *)p, NULL, hwaccel)) - p = &(*p)->next; - last_hwaccel = &hwaccel->next; + return NULL; } -AVHWAccel *av_hwaccel_next(const AVHWAccel *hwaccel) +void av_register_hwaccel(AVHWAccel *hwaccel) { - return hwaccel ? hwaccel->next : first_hwaccel; } +#endif int av_lockmgr_register(int (*cb)(void **mutex, enum AVLockOp op)) { diff --git a/libavcodec/version.h b/libavcodec/version.h index 5b25a9a8ac..693f67386c 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -123,6 +123,9 @@ #ifndef FF_API_CODEC_GET_SET #define FF_API_CODEC_GET_SET (LIBAVCODEC_VERSION_MAJOR < 59) #endif +#ifndef FF_API_USER_VISIBLE_AVHWACCEL +#define FF_API_USER_VISIBLE_AVHWACCEL (LIBAVCODEC_VERSION_MAJOR < 60) +#endif #endif /* AVCODEC_VERSION_H */