On Wed, 3 Jan 2018 00:42:36 +0000
Josh de Kock <j...@itanimul.li> wrote:
> Also replace linked list with an array.
> ---
> configure | 12 +-
> doc/APIchanges | 4 +
> libavcodec/allcodecs.c | 1473
> ++++++++++++++++++++++++++++--------------------
> libavcodec/avcodec.h | 31 +
> libavcodec/parser.c | 87 ++-
> libavcodec/utils.c | 105 ----
> libavcodec/version.h | 3 +
> 7 files changed, 974 insertions(+), 741 deletions(-)
>
> +#if CONFIG_ME_CMP
> +#include "me_cmp.h"
> +#endif
> +
> +pthread_once_t av_codec_static_init = PTHREAD_ONCE_INIT;
> +static void av_codec_init_static(void)
> +{
> +#if CONFIG_ME_CMP
> + ff_me_cmp_init_static();
> +#endif
(This should probably be moved away, since it doesn't really have to do
with any of this - it's just a global init in the wrong place. But I
won't insist that this gets cleaned up now. Someone else can do it
later.)
> + for (int i = 0; codec_list[i]; i++) {
> + if (codec_list[i]->init_static_data)
> + codec_list[i]->init_static_data((AVCodec*)codec_list[i]);
> }
> +}
> +
> +const AVCodec *av_codec_iterate(void **opaque)
> +{
> + uintptr_t i = (uintptr_t)*opaque;
> + const AVCodec *c = codec_list[i];
> +
> + pthread_once(&av_codec_static_init, av_codec_init_static);
One issue: you need to use ff_thread_once() everywhere (it uses a
different initializer type too). The reason is because if it's built
without threads, pthread_once() will not be defined.
Same for all other uses of this.
>
> -#define REGISTER_ENCDEC(X, x) REGISTER_ENCODER(X, x); REGISTER_DECODER(X, x)
> + if (c)
> + *opaque = (void*)(i + 1);
>
> -#define REGISTER_PARSER(X, x) \
> - { \
> - extern AVCodecParser ff_##x##_parser; \
> - if (CONFIG_##X##_PARSER) \
> - av_register_codec_parser(&ff_##x##_parser); \
> + return c;
> +}
> +
> +#if FF_API_NEXT
> +pthread_once_t av_codec_next_init = PTHREAD_ONCE_INIT;
> +
> +static void av_codec_init_next(void)
> +{
> + AVCodec *prev = NULL, *p;
> + void *i = 0;
> + while ((p = (AVCodec*)av_codec_iterate(&i))) {
> + if (prev)
> + prev->next = p;
> + prev = p;
> }
> + if (prev)
> + prev->next = NULL;
What's this for? Isn't it already initialized to NULL?
> +}
> +
>
> -static void register_all(void)
> +
> +av_cold void avcodec_register(AVCodec *codec)
> {
> - /* video codecs */
> - REGISTER_ENCODER(A64MULTI, a64multi);
> + pthread_once(&av_codec_next_init, av_codec_init_next);
> +}
I don't see much of a point in this call. Could be dropped, so the body
is empty.
> +AVCodec *av_codec_next(const AVCodec *c)
> +{
> + pthread_once(&av_codec_next_init, av_codec_init_next);
> +
> + if (c)
> + return c->next;
> + else
> + return (AVCodec*)codec_list[0];
> }
>
> void avcodec_register_all(void)
> {
> - static AVOnce control = AV_ONCE_INIT;
> + pthread_once(&av_codec_next_init, av_codec_init_next);
> +}
So that means if you use the legacy API, you still need to call
the register function. Strange choice, but I'm fine with it.
> +#endif
> +
> +static enum AVCodecID remap_deprecated_codec_id(enum AVCodecID id)
> +{
> + switch(id){
> + //This is for future deprecatec codec ids, its empty since
> + //last major bump but will fill up again over time, please don't
> remove it
> + default : return id;
> + }
> +}
> +
> +static AVCodec *find_codec(enum AVCodecID id, int (*x)(const AVCodec *))
> +{
> + const AVCodec *p, *experimental = NULL;
> + void *i = 0;
> +
> + id = remap_deprecated_codec_id(id);
> +
> + while ((p = av_codec_iterate(&i))) {
> + if (!x(p))
> + continue;
> + if (p->id == id) {
> + if (p->capabilities & AV_CODEC_CAP_EXPERIMENTAL &&
> !experimental) {
> + experimental = p;
> + } else
> + return (AVCodec*)p;
> + }
> + }
>
> - ff_thread_once(&control, register_all);
> + return (AVCodec*)experimental;
> +}
> +
> +AVCodec *avcodec_find_encoder(enum AVCodecID id)
> +{
> + return find_codec(id, av_codec_is_encoder);
> +}
> +
> +AVCodec *avcodec_find_decoder(enum AVCodecID id)
> +{
> + return find_codec(id, av_codec_is_decoder);
> +}
> +
> +static AVCodec *find_codec_by_name(const char *name, int (*x)(const AVCodec
> *))
> +{
> + void *i = 0;
> + const AVCodec *p;
> +
> + if (!name)
> + return NULL;
> +
> + while ((p = av_codec_iterate(&i))) {
> + if (!x(p))
> + continue;
> + if (strcmp(name, p->name) == 0)
> + return (AVCodec*)p;
> + }
> +
> + return NULL;
> +}
> +
> +AVCodec *avcodec_find_encoder_by_name(const char *name)
> +{
> + return find_codec_by_name(name, av_codec_is_encoder);
> +}
> +
> +AVCodec *avcodec_find_decoder_by_name(const char *name)
> +{
> + return find_codec_by_name(name, av_codec_is_decoder);
> }
Personally I'd argue a bool parameter or so to select encoders vs.
decoders would be less roudnabout (like it was before), but I have no
strong opinion. Could be considered an improvement as well.
> +static void av_parser_init_next(void)
> +{
> + AVCodecParser *prev = NULL, *p;
> + int i = 0;
> + while ((p = (AVCodecParser*)parser_list[i++])) {
(Personally I'd prefer less than 2 side effects in conditional
expressions, but maybe it doesn't matter.)
> + if (prev)
> + prev->next = p;
> + prev = p;
> + }
> + if (prev)
> + prev->next = NULL;
Probably redundant assignment again.
Still missing: APIchanges entry.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
