[FFmpeg-devel] [PATCH 3/5] avformat/imf: Demuxer implementation

[Pierre-Anthony Lemieux]

The comments have been addressed in v3 of the patch.


On Sun, Oct 31, 2021 at 1:22 PM Pierre-Anthony Lemieux <pal at sandflow.com> wrote:
>
> On Sun, Oct 31, 2021 at 11:45 AM James Almer <jamrial at 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 at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".