[FFmpeg-devel] [PATCH 1/2] avcodec: add avcodec_register_one() to register one codec/parser/bsf/hwaccel by its char* name
Michael Niedermayer
michaelni at gmx.at
Tue Sep 30 17:21:20 CEST 2014
Hi
On Tue, Sep 30, 2014 at 03:51:13PM +0200, wm4 wrote:
> On Tue, 30 Sep 2014 02:02:49 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
>
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> > libavcodec/allcodecs.c | 51 +++++++++++++++++++++++++++++++++---------------
> > libavcodec/avcodec.h | 5 +++++
> > 2 files changed, 40 insertions(+), 16 deletions(-)
> >
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index 7650543..019d5ea 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -29,49 +29,55 @@
> > #include "version.h"
> >
> > #define REGISTER_HWACCEL(X, x) \
> > - { \
> > + if (!name || !strcmp(name, AV_STRINGIFY(x##_hwaccel))) { \
> > extern AVHWAccel ff_##x##_hwaccel; \
> > - if (CONFIG_##X##_HWACCEL) \
> > + if (CONFIG_##X##_HWACCEL) { \
> > av_register_hwaccel(&ff_##x##_hwaccel); \
> > + found++; \
> > + } \
> > }
> >
> > #define REGISTER_ENCODER(X, x) \
> > - { \
> > + if (!name || !strcmp(name, AV_STRINGIFY(x##_encoder))) { \
> > extern AVCodec ff_##x##_encoder; \
> > - if (CONFIG_##X##_ENCODER) \
> > + if (CONFIG_##X##_ENCODER) { \
> > avcodec_register(&ff_##x##_encoder); \
> > + found++; \
> > + } \
> > }
> >
> > #define REGISTER_DECODER(X, x) \
> > - { \
> > + if (!name || !strcmp(name, AV_STRINGIFY(x##_decoder))) { \
> > extern AVCodec ff_##x##_decoder; \
> > - if (CONFIG_##X##_DECODER) \
> > + if (CONFIG_##X##_DECODER) { \
> > avcodec_register(&ff_##x##_decoder); \
> > + found++; \
> > + } \
> > }
> >
> > #define REGISTER_ENCDEC(X, x) REGISTER_ENCODER(X, x); REGISTER_DECODER(X, x)
> >
> > #define REGISTER_PARSER(X, x) \
> > - { \
> > + if (!name || !strcmp(name, AV_STRINGIFY(x##_parser))) { \
> > extern AVCodecParser ff_##x##_parser; \
> > - if (CONFIG_##X##_PARSER) \
> > + if (CONFIG_##X##_PARSER) { \
> > av_register_codec_parser(&ff_##x##_parser); \
> > + found++; \
> > + } \
> > }
> >
> > #define REGISTER_BSF(X, x) \
> > - { \
> > + if (!name || !strcmp(name, AV_STRINGIFY(x##_bsf))) { \
> > extern AVBitStreamFilter ff_##x##_bsf; \
> > - if (CONFIG_##X##_BSF) \
> > + if (CONFIG_##X##_BSF) { \
> > av_register_bitstream_filter(&ff_##x##_bsf); \
> > + found++; \
> > + } \
> > }
> >
> > -void avcodec_register_all(void)
> > +int avcodec_register_one(const char *name)
> > {
> > - static int initialized;
> > -
> > - if (initialized)
> > - return;
> > - initialized = 1;
> > + int found = 0;
> >
> > /* hardware accelerators */
> > REGISTER_HWACCEL(H263_VAAPI, h263_vaapi);
> > @@ -588,4 +594,17 @@ void avcodec_register_all(void)
> > REGISTER_BSF(NOISE, noise);
> > REGISTER_BSF(REMOVE_EXTRADATA, remove_extradata);
> > REGISTER_BSF(TEXT2MOVSUB, text2movsub);
> > +
> > + return found;
> > }
> > +
> > +void avcodec_register_all(void)
> > +{
> > + static int initialized;
> > +
> > + if (initialized)
> > + return;
> > + initialized = 1;
> > +
> > + avcodec_register_one(NULL);
> > +}
> > \ No newline at end of file
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 94e82f7..de936a5 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -3472,6 +3472,11 @@ void avcodec_register(AVCodec *codec);
> > void avcodec_register_all(void);
> >
> > /**
> > + * @returns the number of newly registered entities
> > + */
> > +int avcodec_register_one(const char *name);
> > +
> > +/**
> > * Allocate an AVCodecContext and set its fields to default values. The
> > * resulting struct should be freed with avcodec_free_context().
> > *
>
> What's the purpose? It requires global mutable state, and won't work is
> more than one library in a process is using ffmpeg. So this would just
> add a fundamentally misdesigned API.
>
> If the purpose is to restrict the codecs that can be used (maybe for
> security reasons?),
yes, its intended for security purposes
> then it should be done in a safe manner
yes
> with no
> global mutable state.
i disagree, applications that care about security must
restrict used entities globally, thus by definition want to change
global state. maybe not the way its done in this patch yes, ill
think about it and submit something better but
A security critical application does not want a mysterious unknown
library to load and use a unrestricted libavcodec behind its back
that would likely bypass the whole idea of restricting the decoders
and demuxers
> I can't see any other purpose; it won't prevent
> "uneeded" codecs from being pulled in by the linker.
>
> Even worse, it would make it impossible to fix the thread-safety and
> library-safety issues of avcodec_register_all().
what issues are you talking about? please elaborate
> With the current API,
> you would have the chance to by change the codec list to a static table,
> which wouldn't require building a codec list at runtime, and thus would
> be safe. With this patch applied it's obviously not possible anymore.
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140930/77bf2baa/attachment.asc>
More information about the ffmpeg-devel
mailing list