Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
Andrew Kelley: > On 4/10/24 07:11, Andreas Rheinhardt wrote: >> I don't >> see a simplification of the Makefile. > > Relevant part from the diff: > > --- a/libavcodec/bsf/Makefile > +++ b/libavcodec/bsf/Makefile > @@ -45,5 +45,3 @@ OBJS-$(CONFIG_VP9_SUPERFRAME_BSF) += > bsf/vp9_superframe.o > OBJS-$(CONFIG_VP9_SUPERFRAME_SPLIT_BSF) += bsf/vp9_superframe_split.o > OBJS-$(CONFIG_VVC_METADATA_BSF) += bsf/h266_metadata.o > OBJS-$(CONFIG_VVC_MP4TOANNEXB_BSF) += bsf/vvc_mp4toannexb.o > - > -libavcodec/bsf/%.o: CPPFLAGS += -I$(SRC_PATH)/libavcodec/ > I am very well aware of the diff. I still don't see a simplification of the Makefile. >>> >>> It also reduces ambiguity, since there are many instances of same-named >>> header files existing in both libavformat/ and libavcodec/ >>> subdirectories. >> >> What ambiguity? > > For example, if a contributor sees #include "vvc.h", they do not know if > that is libavformat/vvc.h or libavcodec/vvc.h without also being aware > of other context, such as the above line in the Makefile. The > explicitness reduces the amount one must know in order to read the code. To quote myself: "It would be different if we did something nuts like adding -Ilibavcodec to the compilation of libavformat files". - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
On 4/10/24 07:11, Andreas Rheinhardt wrote: I don't see a simplification of the Makefile. Relevant part from the diff: --- a/libavcodec/bsf/Makefile +++ b/libavcodec/bsf/Makefile @@ -45,5 +45,3 @@ OBJS-$(CONFIG_VP9_SUPERFRAME_BSF) += bsf/vp9_superframe.o OBJS-$(CONFIG_VP9_SUPERFRAME_SPLIT_BSF) += bsf/vp9_superframe_split.o OBJS-$(CONFIG_VVC_METADATA_BSF) += bsf/h266_metadata.o OBJS-$(CONFIG_VVC_MP4TOANNEXB_BSF)+= bsf/vvc_mp4toannexb.o - -libavcodec/bsf/%.o: CPPFLAGS += -I$(SRC_PATH)/libavcodec/ It also reduces ambiguity, since there are many instances of same-named header files existing in both libavformat/ and libavcodec/ subdirectories. What ambiguity? For example, if a contributor sees #include "vvc.h", they do not know if that is libavformat/vvc.h or libavcodec/vvc.h without also being aware of other context, such as the above line in the Makefile. The explicitness reduces the amount one must know in order to read the code. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
And the New Dictator have Risen! On Wed, Apr 10, 2024 at 4:12 PM Andreas Rheinhardt < andreas.rheinha...@outlook.com> wrote: > Andrew Kelley: > > Removes the special -I flag specified in the avcodec/bsf/ subdirectory. > > > > This makes code copy-pastable to other parts of the ffmpeg codebase, as > > well as simplifying the build script. > > If you want to make the code copy-pastable to other parts, you need to > disallow using the ordinary relative-path header inclusion. And I don't > see a simplification of the Makefile. > > > > > It also reduces ambiguity, since there are many instances of same-named > > header files existing in both libavformat/ and libavcodec/ > > subdirectories. > > What ambiguity? As said above, said ambiguity emanates from using > inclusions with relative paths (and from using the same header > filenames) and not adding an -I for the parent folder for the files in a > subfolder. (It would be different if we did something nuts like adding > -Ilibavcodec to the compilation of libavformat files). > > - Andreas > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
Andrew Kelley: > Removes the special -I flag specified in the avcodec/bsf/ subdirectory. > > This makes code copy-pastable to other parts of the ffmpeg codebase, as > well as simplifying the build script. If you want to make the code copy-pastable to other parts, you need to disallow using the ordinary relative-path header inclusion. And I don't see a simplification of the Makefile. > > It also reduces ambiguity, since there are many instances of same-named > header files existing in both libavformat/ and libavcodec/ > subdirectories. What ambiguity? As said above, said ambiguity emanates from using inclusions with relative paths (and from using the same header filenames) and not adding an -I for the parent folder for the files in a subfolder. (It would be different if we did something nuts like adding -Ilibavcodec to the compilation of libavformat files). - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
Apr 10, 2024, 06:54 by an...@khirnov.net: > Quoting James Almer (2024-04-10 03:23:46) > >> On 4/9/2024 10:11 PM, Andrew Kelley wrote: >> > On 4/9/24 17:04, Lynne wrote: >> >> LGTM. >> >> That's how I wrote the AAC patchset as well. >> > >> > Thank you for the review, Lynne. >> > >> > What is the next step? I do not have commit access. >> >> Applied it, thanks. >> > > WTF? > > Please revert immediately. > In the past, even for sketchy commits with many follow-up fixes needed, and with multiple reverts requested, no one dared to revert someone else's commits out of fear of starting a revert war. You didn't issue a warning that you were going to revert, you didn't ask any of us who approved it, and you hardly allowed for anyone involved to respond. The only reason given on IRC was: > rather than argue about whatever this is it would be more > interesting to know what in the patch is controversial > <@elenril> I don't like it Please don't do this again. This project isn't a dictatorship, and it creates precedence for someone to do the same with your commits. What don't you like about this commit? It eliminates ambiguity and any danger of header conflicts for subsystems being switched to local folders. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
Quoting James Almer (2024-04-10 03:23:46) > On 4/9/2024 10:11 PM, Andrew Kelley wrote: > > On 4/9/24 17:04, Lynne wrote: > >> LGTM. > >> That's how I wrote the AAC patchset as well. > > > > Thank you for the review, Lynne. > > > > What is the next step? I do not have commit access. > > Applied it, thanks. WTF? Please revert immediately. -- Anton Khirnov ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
On 4/9/2024 10:11 PM, Andrew Kelley wrote: On 4/9/24 17:04, Lynne wrote: LGTM. That's how I wrote the AAC patchset as well. Thank you for the review, Lynne. What is the next step? I do not have commit access. Applied it, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
On 4/9/24 17:04, Lynne wrote: LGTM. That's how I wrote the AAC patchset as well. Thank you for the review, Lynne. What is the next step? I do not have commit access. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] bsf: use standard include paths
Apr 9, 2024, 23:24 by and...@ziglang.org: > Removes the special -I flag specified in the avcodec/bsf/ subdirectory. > > This makes code copy-pastable to other parts of the ffmpeg codebase, as > well as simplifying the build script. > > It also reduces ambiguity, since there are many instances of same-named > header files existing in both libavformat/ and libavcodec/ > subdirectories. > --- > libavcodec/bsf/Makefile | 2 -- > libavcodec/bsf/aac_adtstoasc.c| 16 > libavcodec/bsf/av1_frame_merge.c | 8 > libavcodec/bsf/av1_frame_split.c | 8 > libavcodec/bsf/av1_metadata.c | 10 +- > libavcodec/bsf/chomp.c| 4 ++-- > libavcodec/bsf/dca_core.c | 8 > libavcodec/bsf/dts2pts.c | 12 ++-- > libavcodec/bsf/dump_extradata.c | 4 ++-- > libavcodec/bsf/dv_error_marker.c | 4 ++-- > libavcodec/bsf/eac3_core.c| 8 > libavcodec/bsf/evc_frame_merge.c | 12 ++-- > libavcodec/bsf/extract_extradata.c| 22 +++--- > libavcodec/bsf/filter_units.c | 6 +++--- > libavcodec/bsf/h264_metadata.c| 20 ++-- > libavcodec/bsf/h264_mp4toannexb.c | 10 +- > libavcodec/bsf/h264_redundant_pps.c | 16 > libavcodec/bsf/h265_metadata.c| 16 > libavcodec/bsf/h266_metadata.c| 12 ++-- > libavcodec/bsf/hapqa_extract.c| 8 > libavcodec/bsf/hevc_mp4toannexb.c | 10 +- > libavcodec/bsf/imx_dump_header.c | 6 +++--- > libavcodec/bsf/media100_to_mjpegb.c | 6 +++--- > libavcodec/bsf/mjpeg2jpeg.c | 8 > libavcodec/bsf/mjpega_dump_header.c | 8 > libavcodec/bsf/movsub.c | 4 ++-- > libavcodec/bsf/mpeg2_metadata.c | 12 ++-- > libavcodec/bsf/mpeg4_unpack_bframes.c | 8 > libavcodec/bsf/noise.c| 4 ++-- > libavcodec/bsf/null.c | 2 +- > libavcodec/bsf/opus_metadata.c| 4 ++-- > libavcodec/bsf/pcm_rechunk.c | 4 ++-- > libavcodec/bsf/pgs_frame_merge.c | 4 ++-- > libavcodec/bsf/prores_metadata.c | 4 ++-- > libavcodec/bsf/remove_extradata.c | 14 +++--- > libavcodec/bsf/setts.c| 4 ++-- > libavcodec/bsf/showinfo.c | 4 ++-- > libavcodec/bsf/trace_headers.c| 6 +++--- > libavcodec/bsf/truehd_core.c | 10 +- > libavcodec/bsf/vp9_metadata.c | 10 +- > libavcodec/bsf/vp9_raw_reorder.c | 8 > libavcodec/bsf/vp9_superframe.c | 6 +++--- > libavcodec/bsf/vp9_superframe_split.c | 8 > libavcodec/bsf/vvc_mp4toannexb.c | 10 +- > 44 files changed, 184 insertions(+), 186 deletions(-) > LGTM. That's how I wrote the AAC patchset as well. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
