| Message ID | 20220315211544.40272-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | 11c4f4b455dcc39146e0caa09b1f5fc09c58cf06 |
| Headers | show |
| Series | [FFmpeg-devel] ffmpeg: remove usage of internal deprecation macro | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
James Almer: > Signed-off-by: James Almer <jamrial@gmail.com> > --- > fftools/ffmpeg.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index a98e49b775..3b625a9918 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, char *error, int error_len) > ist->dec_ctx->opaque = ist; > ist->dec_ctx->get_format = get_format; > #if LIBAVCODEC_VERSION_MAJOR < 60 > -FF_DISABLE_DEPRECATION_WARNINGS > + AV_NOWARN_DEPRECATED({ > ist->dec_ctx->thread_safe_callbacks = 1; > -FF_ENABLE_DEPRECATION_WARNINGS > + }) > #endif > > if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE && AV_NOWARN_DEPRECATED currently doesn't work with Clang; so you first need to find out from which version onward it supports these macros. - Andreas
On 3/16/2022 7:15 AM, Andreas Rheinhardt wrote: > James Almer: >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> fftools/ffmpeg.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >> index a98e49b775..3b625a9918 100644 >> --- a/fftools/ffmpeg.c >> +++ b/fftools/ffmpeg.c >> @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, char *error, int error_len) >> ist->dec_ctx->opaque = ist; >> ist->dec_ctx->get_format = get_format; >> #if LIBAVCODEC_VERSION_MAJOR < 60 >> -FF_DISABLE_DEPRECATION_WARNINGS >> + AV_NOWARN_DEPRECATED({ >> ist->dec_ctx->thread_safe_callbacks = 1; >> -FF_ENABLE_DEPRECATION_WARNINGS >> + }) >> #endif >> >> if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE && > > AV_NOWARN_DEPRECATED currently doesn't work with Clang; so you first > need to find out from which version onward it supports these macros. Does not work in what way? Not compile, or just be a no-op? If the latter, then that's a limitation FF_DISABLE_DEPRECATION_WARNINGS also has for some targets. We're not going to stop using a macro just because it does nothing in some scenarios.
James Almer: > > > On 3/16/2022 7:15 AM, Andreas Rheinhardt wrote: >> James Almer: >>> Signed-off-by: James Almer <jamrial@gmail.com> >>> --- >>> fftools/ffmpeg.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>> index a98e49b775..3b625a9918 100644 >>> --- a/fftools/ffmpeg.c >>> +++ b/fftools/ffmpeg.c >>> @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, >>> char *error, int error_len) >>> ist->dec_ctx->opaque = ist; >>> ist->dec_ctx->get_format = get_format; >>> #if LIBAVCODEC_VERSION_MAJOR < 60 >>> -FF_DISABLE_DEPRECATION_WARNINGS >>> + AV_NOWARN_DEPRECATED({ >>> ist->dec_ctx->thread_safe_callbacks = 1; >>> -FF_ENABLE_DEPRECATION_WARNINGS >>> + }) >>> #endif >>> if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE && >> >> AV_NOWARN_DEPRECATED currently doesn't work with Clang; so you first >> need to find out from which version onward it supports these macros. > > Does not work in what way? Not compile, or just be a no-op? > If the latter, then that's a limitation FF_DISABLE_DEPRECATION_WARNINGS > also has for some targets. We're not going to stop using a macro just > because it does nothing in some scenarios. Clang takes this path in AV_NOWARN_DEPRECATED: # define AV_NOWARN_DEPRECATED(code) code So it is a no-op and the warning is not gone; any recent version of it will understand the GCC pragma, so this can be rectified. Looking at our internal deprecation macros shows that it is possible to also support the Intel compiler. And I really don't like that you are basically declaring Clang to be irrelevant; it is so important that you should have tested it. - Andreas
On 3/16/2022 9:58 AM, Andreas Rheinhardt wrote: > James Almer: >> >> >> On 3/16/2022 7:15 AM, Andreas Rheinhardt wrote: >>> James Almer: >>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>> --- >>>> fftools/ffmpeg.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>> index a98e49b775..3b625a9918 100644 >>>> --- a/fftools/ffmpeg.c >>>> +++ b/fftools/ffmpeg.c >>>> @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, >>>> char *error, int error_len) >>>> ist->dec_ctx->opaque = ist; >>>> ist->dec_ctx->get_format = get_format; >>>> #if LIBAVCODEC_VERSION_MAJOR < 60 >>>> -FF_DISABLE_DEPRECATION_WARNINGS >>>> + AV_NOWARN_DEPRECATED({ >>>> ist->dec_ctx->thread_safe_callbacks = 1; >>>> -FF_ENABLE_DEPRECATION_WARNINGS >>>> + }) >>>> #endif >>>> if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE && >>> >>> AV_NOWARN_DEPRECATED currently doesn't work with Clang; so you first >>> need to find out from which version onward it supports these macros. >> >> Does not work in what way? Not compile, or just be a no-op? >> If the latter, then that's a limitation FF_DISABLE_DEPRECATION_WARNINGS >> also has for some targets. We're not going to stop using a macro just >> because it does nothing in some scenarios. > > Clang takes this path in AV_NOWARN_DEPRECATED: > # define AV_NOWARN_DEPRECATED(code) code > So it is a no-op and the warning is not gone; any recent version of it > will understand the GCC pragma, so this can be rectified. > Looking at our internal deprecation macros shows that it is possible to > also support the Intel compiler. > And I really don't like that you are basically declaring Clang to be > irrelevant; it is so important that you should have tested it. I'm not declaring it irrelevant, i'm saying that if a public macro has no implementation for a given compiler, then that's unrelated to its usage. The internal macro also has limitations, yet it hasn't stopped us from using it anywhere. I'm removing the usage of an internal macro from a file that should not access it, and replacing it with a public one. What happens under the hood is a separate issue. Supporting more compilers in AV_NOWARN_DEPRECATED() is a separate fix.
James Almer: > > > On 3/16/2022 9:58 AM, Andreas Rheinhardt wrote: >> James Almer: >>> >>> >>> On 3/16/2022 7:15 AM, Andreas Rheinhardt wrote: >>>> James Almer: >>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>> --- >>>>> fftools/ffmpeg.c | 4 ++-- >>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>> index a98e49b775..3b625a9918 100644 >>>>> --- a/fftools/ffmpeg.c >>>>> +++ b/fftools/ffmpeg.c >>>>> @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, >>>>> char *error, int error_len) >>>>> ist->dec_ctx->opaque = ist; >>>>> ist->dec_ctx->get_format = get_format; >>>>> #if LIBAVCODEC_VERSION_MAJOR < 60 >>>>> -FF_DISABLE_DEPRECATION_WARNINGS >>>>> + AV_NOWARN_DEPRECATED({ >>>>> ist->dec_ctx->thread_safe_callbacks = 1; >>>>> -FF_ENABLE_DEPRECATION_WARNINGS >>>>> + }) >>>>> #endif >>>>> if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE && >>>> >>>> AV_NOWARN_DEPRECATED currently doesn't work with Clang; so you first >>>> need to find out from which version onward it supports these macros. >>> >>> Does not work in what way? Not compile, or just be a no-op? >>> If the latter, then that's a limitation FF_DISABLE_DEPRECATION_WARNINGS >>> also has for some targets. We're not going to stop using a macro just >>> because it does nothing in some scenarios. >> >> Clang takes this path in AV_NOWARN_DEPRECATED: >> # define AV_NOWARN_DEPRECATED(code) code >> So it is a no-op and the warning is not gone; any recent version of it >> will understand the GCC pragma, so this can be rectified. >> Looking at our internal deprecation macros shows that it is possible to >> also support the Intel compiler. >> And I really don't like that you are basically declaring Clang to be >> irrelevant; it is so important that you should have tested it. > > I'm not declaring it irrelevant, i'm saying that if a public macro has > no implementation for a given compiler, then that's unrelated to its > usage. The internal macro also has limitations, yet it hasn't stopped us > from using it anywhere. > > I'm removing the usage of an internal macro from a file that should not > access it, and replacing it with a public one. What happens under the > hood is a separate issue. Supporting more compilers in > AV_NOWARN_DEPRECATED() is a separate fix. Separate, but not independent: Supporting more compilers should come first. - Andreas
On 3/16/2022 10:10 AM, Andreas Rheinhardt wrote: > James Almer: >> >> >> On 3/16/2022 9:58 AM, Andreas Rheinhardt wrote: >>> James Almer: >>>> >>>> >>>> On 3/16/2022 7:15 AM, Andreas Rheinhardt wrote: >>>>> James Almer: >>>>>> Signed-off-by: James Almer <jamrial@gmail.com> >>>>>> --- >>>>>> fftools/ffmpeg.c | 4 ++-- >>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>>> index a98e49b775..3b625a9918 100644 >>>>>> --- a/fftools/ffmpeg.c >>>>>> +++ b/fftools/ffmpeg.c >>>>>> @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, >>>>>> char *error, int error_len) >>>>>> ist->dec_ctx->opaque = ist; >>>>>> ist->dec_ctx->get_format = get_format; >>>>>> #if LIBAVCODEC_VERSION_MAJOR < 60 >>>>>> -FF_DISABLE_DEPRECATION_WARNINGS >>>>>> + AV_NOWARN_DEPRECATED({ >>>>>> ist->dec_ctx->thread_safe_callbacks = 1; >>>>>> -FF_ENABLE_DEPRECATION_WARNINGS >>>>>> + }) >>>>>> #endif >>>>>> if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE && >>>>> >>>>> AV_NOWARN_DEPRECATED currently doesn't work with Clang; so you first >>>>> need to find out from which version onward it supports these macros. >>>> >>>> Does not work in what way? Not compile, or just be a no-op? >>>> If the latter, then that's a limitation FF_DISABLE_DEPRECATION_WARNINGS >>>> also has for some targets. We're not going to stop using a macro just >>>> because it does nothing in some scenarios. >>> >>> Clang takes this path in AV_NOWARN_DEPRECATED: >>> # define AV_NOWARN_DEPRECATED(code) code >>> So it is a no-op and the warning is not gone; any recent version of it >>> will understand the GCC pragma, so this can be rectified. >>> Looking at our internal deprecation macros shows that it is possible to >>> also support the Intel compiler. >>> And I really don't like that you are basically declaring Clang to be >>> irrelevant; it is so important that you should have tested it. >> >> I'm not declaring it irrelevant, i'm saying that if a public macro has >> no implementation for a given compiler, then that's unrelated to its >> usage. The internal macro also has limitations, yet it hasn't stopped us >> from using it anywhere. >> >> I'm removing the usage of an internal macro from a file that should not >> access it, and replacing it with a public one. What happens under the >> hood is a separate issue. Supporting more compilers in >> AV_NOWARN_DEPRECATED() is a separate fix. > > Separate, but not independent: Supporting more compilers should come first. > > - Andreas Can you implement it? I don't have a Clang toolchain nor do i know how to check for supported Pragmas. defined(__clang__) is probably too broad for this.
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index a98e49b775..3b625a9918 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -2880,9 +2880,9 @@ static int init_input_stream(int ist_index, char *error, int error_len) ist->dec_ctx->opaque = ist; ist->dec_ctx->get_format = get_format; #if LIBAVCODEC_VERSION_MAJOR < 60 -FF_DISABLE_DEPRECATION_WARNINGS + AV_NOWARN_DEPRECATED({ ist->dec_ctx->thread_safe_callbacks = 1; -FF_ENABLE_DEPRECATION_WARNINGS + }) #endif if (ist->dec_ctx->codec_id == AV_CODEC_ID_DVB_SUBTITLE &&
Signed-off-by: James Almer <jamrial@gmail.com> --- fftools/ffmpeg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)