Re: [FFmpeg-devel] [PATCH] lavc: move bitstream filters into bsf/ subdir

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-01-29 10:57:19)
>> Anton Khirnov:
>>> +# add libavcodec/ to include path for bsfs
>>> +$(addprefix libavcodec/, $(sort $(filter bsf/%,$(OBJS_BSF-yes)))): CPPFLAGS += -I$(SRC_PATH)/libavcodec/
>>
>> 1. Why sort?
> 
> To get rid of duplicates, otherwise the flags can be added multiple
> times.
> 

Why not just use libavcodec/bsf/%.o: CPPFLAGS += -I$(SRC_PATH)/libavcodec/

>> 2. Adding dependencies for stuff not in this folder is different from
>> how we do it for arch-specific stuff.
> 
> Yes, but as I said in the previous email - bitstream filters tend to
> include more headers from libavcodec/ than code in arch/.
> 

Both 2. and 3. were not meant about headers/include folders, but about
the fact that you add dependencies to files in libavcodec/ in this very
Makefile.

> By my count, the 96 *.[ch] files in under libavcodec/x86 only include 98
> headers from libavcodec/, 1.02 per file on average.
> By contrast the 43 *.c files under bsf/ include 187 files from
> libavcodec/, which averages to 4.35 per file.
> So IMO it makes sense to avoid the pointless noise and busywork from adding
> libavcodec/ prefixes to all those includes.
> 
> I would also be fine with adding -Ilibavcodec for arch files.
> 
>> 3. And actually, it is worse: Imagine someone changed
>> h265_profile_level.c in such a way that h265_profile_level.o now relies
>> on stuff provided by a different translation unit. Then you need to add
>> said dependency to all the components that require h265_profile_level.o.
>> If such a dependency exists in another Makefile for a subfolder, it is
>> likely that this will be forgotten.
>> (Of course, seeing that a BSF requires h265_profile_level.o might make
>> the developer think twice whether it is really a good idea to add this
>> new code to it.)
> 
> That's an argument against subfolders in general though, not against
> modifying include flags. And it seems to me we have an overwhelming
> consensus in favor of subfolders.
> 

I was actually proposing that the dependencies for stuff in libavcodec/
stays in libavcodec/Makefile.

- Andreas

PS: Why don't you move e.g. bsf_internal.h as well as bsf.c itself?
And where has this actually been discussed for there to be
"overwhelming" consensus for it?

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".