| Message ID | AS8P250MB07448E77CAE61189C20C68A88F449@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM |
|---|---|
| State | Accepted |
| Commit | 262e7439c6d58564d21ffffb05ade84bac4482ef |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/x86/Makefile: Don't build empty files | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Am Mo., 12. Sept. 2022 um 17:04 Uhr schrieb Andreas Rheinhardt <andreas.rheinhardt@outlook.com>: > > Should fix ticket #9909, fixing a regression since > bfb28b5ce89f3e950214b67ea95b45e3355c2caf. > > Thanks to Carl Eugen Hoyos for analyzing the issue. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > This would be my solution. What do you think of it? No objection but I do not consider this patch cleaner. Thank you, Carl Eugen
Carl Eugen Hoyos: > Am Mo., 12. Sept. 2022 um 17:04 Uhr schrieb Andreas Rheinhardt > <andreas.rheinhardt@outlook.com>: >> >> Should fix ticket #9909, fixing a regression since >> bfb28b5ce89f3e950214b67ea95b45e3355c2caf. >> >> Thanks to Carl Eugen Hoyos for analyzing the issue. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> This would be my solution. What do you think of it? > > No objection but I do not consider this patch cleaner. > > Thank you, Carl Eugen Then please apply yours. - Andreas
Andreas Rheinhardt: > Should fix ticket #9909, fixing a regression since > bfb28b5ce89f3e950214b67ea95b45e3355c2caf. > > Thanks to Carl Eugen Hoyos for analyzing the issue. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > This would be my solution. What do you think of it? > > libavcodec/x86/Makefile | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/x86/Makefile b/libavcodec/x86/Makefile > index 4e448623af..41ca864849 100644 > --- a/libavcodec/x86/Makefile > +++ b/libavcodec/x86/Makefile > @@ -140,8 +140,11 @@ X86ASM-OBJS-$(CONFIG_QPELDSP) += x86/qpeldsp.o \ > X86ASM-OBJS-$(CONFIG_RV34DSP) += x86/rv34dsp.o > X86ASM-OBJS-$(CONFIG_VC1DSP) += x86/vc1dsp_loopfilter.o \ > x86/vc1dsp_mc.o > -X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o \ > - x86/simple_idct.o > +ifdef ARCH_X86_64 > +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o > +else > +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct.o > +endif > X86ASM-OBJS-$(CONFIG_VIDEODSP) += x86/videodsp.o > X86ASM-OBJS-$(CONFIG_VP3DSP) += x86/vp3dsp.o > X86ASM-OBJS-$(CONFIG_VP8DSP) += x86/vp8dsp.o \ It seems like that there are linkers out there that complain about empty object files like x86/simple_idct.o ("ranlib: file: libavcodec/libavcodec.a(simple_idct.o) has no symbols" as reported by BBB); the above patch is still needed to fix this. I will therefore apply it tonight (with an updated commit message) unless there are objections. - Andreas
Am Di., 13. Dez. 2022 um 16:02 Uhr schrieb Andreas Rheinhardt <andreas.rheinhardt@outlook.com>: > > Andreas Rheinhardt: > > Should fix ticket #9909, fixing a regression since > > bfb28b5ce89f3e950214b67ea95b45e3355c2caf. > > > > Thanks to Carl Eugen Hoyos for analyzing the issue. > > > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > > --- > > This would be my solution. What do you think of it? > > > > libavcodec/x86/Makefile | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/libavcodec/x86/Makefile b/libavcodec/x86/Makefile > > index 4e448623af..41ca864849 100644 > > --- a/libavcodec/x86/Makefile > > +++ b/libavcodec/x86/Makefile > > @@ -140,8 +140,11 @@ X86ASM-OBJS-$(CONFIG_QPELDSP) += x86/qpeldsp.o \ > > X86ASM-OBJS-$(CONFIG_RV34DSP) += x86/rv34dsp.o > > X86ASM-OBJS-$(CONFIG_VC1DSP) += x86/vc1dsp_loopfilter.o \ > > x86/vc1dsp_mc.o > > -X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o \ > > - x86/simple_idct.o > > +ifdef ARCH_X86_64 > > +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o > > +else > > +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct.o > > +endif > > X86ASM-OBJS-$(CONFIG_VIDEODSP) += x86/videodsp.o > > X86ASM-OBJS-$(CONFIG_VP3DSP) += x86/vp3dsp.o > > X86ASM-OBJS-$(CONFIG_VP8DSP) += x86/vp8dsp.o \ > > It seems like that there are linkers out there that complain about empty > object files like x86/simple_idct.o ("ranlib: file: > libavcodec/libavcodec.a(simple_idct.o) has no symbols" as reported by > BBB); the above patch is still needed to fix this. I will therefore > apply it tonight (with an updated commit message) unless there are > objections. (Three hours is not a lot) Which toolchain broke? Thank you, Carl Eugen
Carl Eugen Hoyos: > Am Di., 13. Dez. 2022 um 16:02 Uhr schrieb Andreas Rheinhardt > <andreas.rheinhardt@outlook.com>: >> >> Andreas Rheinhardt: >>> Should fix ticket #9909, fixing a regression since >>> bfb28b5ce89f3e950214b67ea95b45e3355c2caf. >>> >>> Thanks to Carl Eugen Hoyos for analyzing the issue. >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>> --- >>> This would be my solution. What do you think of it? >>> >>> libavcodec/x86/Makefile | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/libavcodec/x86/Makefile b/libavcodec/x86/Makefile >>> index 4e448623af..41ca864849 100644 >>> --- a/libavcodec/x86/Makefile >>> +++ b/libavcodec/x86/Makefile >>> @@ -140,8 +140,11 @@ X86ASM-OBJS-$(CONFIG_QPELDSP) += x86/qpeldsp.o \ >>> X86ASM-OBJS-$(CONFIG_RV34DSP) += x86/rv34dsp.o >>> X86ASM-OBJS-$(CONFIG_VC1DSP) += x86/vc1dsp_loopfilter.o \ >>> x86/vc1dsp_mc.o >>> -X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o \ >>> - x86/simple_idct.o >>> +ifdef ARCH_X86_64 >>> +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o >>> +else >>> +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct.o >>> +endif >>> X86ASM-OBJS-$(CONFIG_VIDEODSP) += x86/videodsp.o >>> X86ASM-OBJS-$(CONFIG_VP3DSP) += x86/vp3dsp.o >>> X86ASM-OBJS-$(CONFIG_VP8DSP) += x86/vp8dsp.o \ >> >> It seems like that there are linkers out there that complain about empty >> object files like x86/simple_idct.o ("ranlib: file: >> libavcodec/libavcodec.a(simple_idct.o) has no symbols" as reported by >> BBB); the above patch is still needed to fix this. I will therefore >> apply it tonight (with an updated commit message) unless there are >> objections. > > (Three hours is not a lot) > > Which toolchain broke? > "Broke" is the wrong word; Xcode emits warnings for empty object files. Said Ronald. I haven't reproduced it myself. - Andreas
Am Di., 13. Dez. 2022 um 20:03 Uhr schrieb Andreas Rheinhardt <andreas.rheinhardt@outlook.com>: > > Carl Eugen Hoyos: > > Am Di., 13. Dez. 2022 um 16:02 Uhr schrieb Andreas Rheinhardt > > <andreas.rheinhardt@outlook.com>: > >> > >> Andreas Rheinhardt: > >>> Should fix ticket #9909, fixing a regression since > >>> bfb28b5ce89f3e950214b67ea95b45e3355c2caf. > >>> > >>> Thanks to Carl Eugen Hoyos for analyzing the issue. > >>> > >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > >>> --- > >>> This would be my solution. What do you think of it? > >>> > >>> libavcodec/x86/Makefile | 7 +++++-- > >>> 1 file changed, 5 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/libavcodec/x86/Makefile b/libavcodec/x86/Makefile > >>> index 4e448623af..41ca864849 100644 > >>> --- a/libavcodec/x86/Makefile > >>> +++ b/libavcodec/x86/Makefile > >>> @@ -140,8 +140,11 @@ X86ASM-OBJS-$(CONFIG_QPELDSP) += x86/qpeldsp.o \ > >>> X86ASM-OBJS-$(CONFIG_RV34DSP) += x86/rv34dsp.o > >>> X86ASM-OBJS-$(CONFIG_VC1DSP) += x86/vc1dsp_loopfilter.o \ > >>> x86/vc1dsp_mc.o > >>> -X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o \ > >>> - x86/simple_idct.o > >>> +ifdef ARCH_X86_64 > >>> +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o > >>> +else > >>> +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct.o > >>> +endif > >>> X86ASM-OBJS-$(CONFIG_VIDEODSP) += x86/videodsp.o > >>> X86ASM-OBJS-$(CONFIG_VP3DSP) += x86/vp3dsp.o > >>> X86ASM-OBJS-$(CONFIG_VP8DSP) += x86/vp8dsp.o \ > >> > >> It seems like that there are linkers out there that complain about empty > >> object files like x86/simple_idct.o ("ranlib: file: > >> libavcodec/libavcodec.a(simple_idct.o) has no symbols" as reported by > >> BBB); the above patch is still needed to fix this. I will therefore > >> apply it tonight (with an updated commit message) unless there are > >> objections. > > > > (Three hours is not a lot) > > > > Which toolchain broke? > > "Broke" is the wrong word; Xcode emits warnings for empty object files. It does. (I did not consider this an issue when I saw it.) I consider this unmaintainable, an argument that in the past was used to object significantly more important patches (in cases where maintenance was imo no problem). Anyway: No objection. Carl Eugen
Hi, On Tue, Dec 13, 2022 at 2:12 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > It does. > (I did not consider this an issue when I saw it.) > > I consider this unmaintainable, an argument that in the past was > used to object significantly more important patches (in cases > where maintenance was imo no problem). > > Anyway: No objection. > It's not that it's a big deal, just that it's mildly annoying and there doesn't appear to be an obvious downside to fixing it. Ronald
diff --git a/libavcodec/x86/Makefile b/libavcodec/x86/Makefile index 4e448623af..41ca864849 100644 --- a/libavcodec/x86/Makefile +++ b/libavcodec/x86/Makefile @@ -140,8 +140,11 @@ X86ASM-OBJS-$(CONFIG_QPELDSP) += x86/qpeldsp.o \ X86ASM-OBJS-$(CONFIG_RV34DSP) += x86/rv34dsp.o X86ASM-OBJS-$(CONFIG_VC1DSP) += x86/vc1dsp_loopfilter.o \ x86/vc1dsp_mc.o -X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o \ - x86/simple_idct.o +ifdef ARCH_X86_64 +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct10.o +else +X86ASM-OBJS-$(CONFIG_IDCTDSP) += x86/simple_idct.o +endif X86ASM-OBJS-$(CONFIG_VIDEODSP) += x86/videodsp.o X86ASM-OBJS-$(CONFIG_VP3DSP) += x86/vp3dsp.o X86ASM-OBJS-$(CONFIG_VP8DSP) += x86/vp8dsp.o \
Should fix ticket #9909, fixing a regression since bfb28b5ce89f3e950214b67ea95b45e3355c2caf. Thanks to Carl Eugen Hoyos for analyzing the issue. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- This would be my solution. What do you think of it? libavcodec/x86/Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)