On Thu, 19 Apr 2018 12:15:07 +0200 Martin Dørum <martid0...@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 <martid0...@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) _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
