[FFmpeg-devel] [PATCH] avcodec/allcodecs: add FFMPEG_PREFER_* env vars

[wm4]

On Thu, 19 Apr 2018 12:15:07 +0200
Martin Dørum <martid0311 at gmail.com> wrote:

> When a program uses FFmpeg to decode or encode media, and uses
> `avcodec_find_decoder` or `avcodec_find_encoder`, I interpret that as
> the program not caring a whole lot what AVCodec is used; it just wants
> something which can encode or decode the desired format. I think it
> makes sense for the user to be able to specify what codec should be the
> default in that case, because the user might know that they're on a
> system with a fast hardware encoder or decoder. Currently, the only way
> to convince FFmpeg to default to a different codec is to recompile it
> without support for other codecs than the desired one, which feels
> unnecessary.

Obviously a program can do its own selection, and some programs do this.

> An argument against this patch could be that it's the application's
> responsibility to add an -encoder or -decoder option like the ffmpeg
> CLI utilities do. If that's the "official" stance of FFmpeg, I'm fine
> with that, even though I don't necessarily agree.

It probably is. The main problem is actually that we have something
against environment variables. The only cases in which the libs read
and use env vars are when it's a broadly used standard convention (like
http proxy override), or other special cases. I don't think we should
add new ones.

> Anotehr point is that some applications make assumptions about the
> pix_fmt the decoder they get from `avcodec_find_decoder` uses. Chromium
> does this. I believe this is a problem with those applications
> (and I submitted an issue to the Chromium project:
> https://bugs.chromium.org/p/webrtc/issues/detail?id=9137#c2), but it
> should be considered.

That's true, and in some special cases you don't have much of a choice.
But in most cases (including this one) the application should not
expect anything, because:

1. certain files might use different output parameters (e.g. 10 bit
   h264 files)
2. the output parameters could change if codec internals change
   (happens sometimes if optimizations or new features are added to a
   decoder)

> (I also haven't submitted a patch to FFmpeg before, or any open-source
> project which doesn't use GitHub pull requests, so I apologize if I
> have followed the process incorrectly.)

Looks ok. One issues is if you copy & pasted the git patch into your
email client, it can happen that the patch gets corrupted, so either
attaching the patch as text file, or using git send-email is preferred.

Regarding this patch, personally I don't think using getenv() to
configure what is pretty much API semantics is acceptable. But a new
API function that restricts what codecs are used based on a string
argument might be ok. Then applications could use it to implement
codec selection control via environment or command line arguments or
something else.

> This patch makes it possible to choose what codec avcodec_find_encoder
> and avcodec_find_decoder returns with environment variables.
> FFMPEG_PREFER_CODEC is an environment variable with a comma-separated
> list of codec names, and if it contains the name of an applicable
> codec, that codec will be returned instead of whatever av_codec_iterate
> happens to return first.
> 
> There's also FFMPEG_PREFER_ENCODER and FFMPEG_PREFER_DECODER, which
> specifically apply to avcodec_find_encoder and avcodec_find_decoder
> respectively. These are prioritized above FFMPEG_PREFER_CODEC.
> 
> Signed-off-by: Martin Dørum <martid0311 at gmail.com>
> ---
>   libavcodec/allcodecs.c | 59 ++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 4d4ef530e4..3c85908969 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -833,25 +833,74 @@ static enum AVCodecID remap_deprecated_codec_id(enum AVCodecID id)
>       }
>   }
>   
> +static int codec_matches_preferred(const AVCodec *p, const char *str)
> +{
> +    int pi = 0, stri = 0;
> +    int match = 1;
> +
> +    if (str == NULL)
> +        return 0;
> +
> +    while (1) {
> +        if (str[stri] == '\0' || str[stri] == ',') {
> +            if (match && p->name[pi] == '\0') {
> +                return 1;
> +            } else if (str[stri] == '\0') {
> +                return 0;
> +            } else {
> +                match = 1;
> +                pi = 0;
> +            }
> +        } else if (p->name[pi] == '\0') {
> +            match = 0;
> +        } else {
> +            if (match && p->name[pi] != str[stri])
> +                match = 0;
> +            pi += 1;
> +        }
> +
> +        stri += 1;
> +    }
> +}
> +
>   static AVCodec *find_codec(enum AVCodecID id, int (*x)(const AVCodec *))
>   {
> -    const AVCodec *p, *experimental = NULL;
> +    const AVCodec *p, *experimental = NULL, *fallback = NULL, *preferred = NULL;
>       void *i = 0;
>   
> +    char *preferred_codec = getenv("FFMPEG_PREFER_CODEC");
> +    char *preferred_encdec = NULL;
> +    if (x == av_codec_is_encoder)
> +        preferred_encdec = getenv("FFMPEG_PREFER_ENCODER");
> +    else if (x == av_codec_is_decoder)
> +        preferred_encdec = getenv("FFMPEG_PREFER_DECODER");
> +
>       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
> +            if (codec_matches_preferred(p, preferred_encdec)) {
>                   return (AVCodec*)p;
> +            } else if (codec_matches_preferred(p, preferred_codec) && !preferred) {
> +                preferred = p;
> +            } else if (p->capabilities & AV_CODEC_CAP_EXPERIMENTAL && !experimental) {
> +                experimental = p;
> +            } else if (!fallback) {
> +                fallback = p;
> +            }
>           }
>       }
>   
> -    return (AVCodec*)experimental;
> +    if (preferred)
> +        return (AVCodec*)preferred;
> +    else if (fallback)
> +        return (AVCodec*)fallback;
> +    else if (experimental)
> +        return (AVCodec*)experimental;
> +    else
> +        return NULL;
>   }
>   
>   AVCodec *avcodec_find_encoder(enum AVCodecID id)