The comments have been addressed in v3 of the patch.
On Sun, Oct 31, 2021 at 1:22 PM Pierre-Anthony Lemieux <p...@sandflow.com> wrote: > > On Sun, Oct 31, 2021 at 11:45 AM James Almer <jamr...@gmail.com> wrote: > > > > On 10/31/2021 3:34 PM, Pierre-Anthony Lemieux wrote: > > >>> The functions are not static and are defined in imf_internal.h. They > > >>> are used in both libavformat/imfdec.c and the tests at > > >>> libavformat/tests/imf.c. Ok? > > >> > > >> If they are used in libavformat only it should be static. > > > AFAIK static functions are only available in the compilation unit > > > where they are defined, so if a static function (e.g. > > > `parse_imf_asset_map_from_xml_dom()`) is defined in > > > libavformat/imf_dec.c, it will not be available in > > > libavformat/tests/imf.c. > > > > > > Functions that can be used by other FFMPEG modules are declared in > > > "imf.h", whereas functions that are intended to be used only by the > > > IMF demuxer are declared in "imf_internal.h". > > > > > > For example, both "libavformat/tests/imf.c" and "libavformat/imfdec.c" > > > include "libavformat/imf_internal.h" so they can access > > > `parse_imf_asset_map_from_xml_dom()`. > > > > > > Does that work? Any alternative? > > > > Tests in the library's test folder can and usually include the actual .c > > file from the module they are testing, so making it static will work fine. > > Ok. To summarize: > > - functions used only by "imf_xxxx.c" should be static in "imf_xxxx.c, > and tests should include "imf_xxxx.c" > - any function/structure used by both "imf_xxxx.c" and "imf_yyyy.c" > should be declared in "imf.h", even if the function/structure is not > intended to be used beyond "imf_xxxx.c" and "imf_yyyy.c" > > In other words, splitting imf-related functions into "imf.h" > (functions potentially for use beyond the IMF demuxer) and > "imf_internal.h" (functions not for use beyond the IMF demuxer) is not > desirable. > > Ok? Happy either way, just want to make sure I get the next patch > version right :) > > > > > > _______________________________________________ > > 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".
