av_guess_codec ignores short_name, filename, and mime_type

[Andrew Kelley]

Tested with current git master: a137e50ee54cd902c436d15463c6e621623cc9ad

av_guess_codec ignores the short_name, filename, and mime_type parameters. Observe this test program:

#include <stdio.h>
#include <libavcodec/avcodec.h>
#include <libavformat/avformat.h>

int main(int argc, char **argv) {
    avcodec_register_all();
    av_register_all();

    AVOutputFormat *oformat = av_guess_format(NULL, "test.ogg", NULL);
    enum AVCodecID codec_id = av_guess_codec(oformat, "flac", "test.flac", NULL, AVMEDIA_TYPE_AUDIO);
    AVCodec *codec = avcodec_find_encoder(codec_id);
    fprintf(stderr, "%s\n", codec->name);

    return 0;
}

It prints "libvorbis" when it should instead print "flac". The OGG container is capable of storing FLAC audio, and the "flac" short name should override the format name guessed with av_guess_format. I think even the filename and mime_type arguments of av_guess_codec, if supplied, should override whatever was guessed from av_guess_format.

The implementation of av_guess_codec is completely ignoring these parameters.

[Carl Eugen Hoyos]

Iiuc, you argue that the parameter short_name of av_guess_codec() should have precedence over the parameter fmt. Why do you think so? The function is there for over ten years, and its (arguably undocumented) behaviour was always that the fmt parameter has precendence.
Or to say it differently: Why don't you pass "test.flac" to av_guess_format?

[Andrew Kelley]

My example was confusing. Let me clarify it to replace "test.flac" with NULL:

#include <stdio.h>
#include <libavcodec/avcodec.h>
#include <libavformat/avformat.h>

int main(int argc, char **argv) {
    avcodec_register_all();
    av_register_all();

    AVOutputFormat *oformat = av_guess_format(NULL, "test.ogg", NULL);
    enum AVCodecID codec_id = av_guess_codec(oformat, "flac", NULL, NULL, AVMEDIA_TYPE_AUDIO);
    AVCodec *codec = avcodec_find_encoder(codec_id);
    fprintf(stderr, "%s\n", codec->name);

    return 0;
}

In this example, we intend to save a file with the file name "test.ogg" and store FLAC data inside of it. But it prints libvorbis instead of flac. The format should be ogg and the codec should be flac.

Can you really say it is working as intended if 3 of the parameters are in fact completely disregarded?

[Carl Eugen Hoyos]

My original interpretation of this ticket was that you argued that the short_name parameter of av_guess_codec() should have precedence over the fmt parameter. Was I correct? If yes, please explain why you think the short_name parameter should have precedence. The behaviour is not documented (which is a minor bug imo) but changing it after ten years would need a (very) good explanation.

Why are you calling av_guess_codec() if you know exactly what codec you want to use?

[Andrew Kelley]

My original interpretation of this ticket was that you argued that the short_name
parameter of av_guess_codec() should have precedence over the fmt parameter. Was I
correct?

No. The title of the ticket is correct as well as my clarified code example.

I do not think the short_name of the codec should change what *format* is used, but I do think the short_name of the codec should change what *codec* is used.

Why are you calling av_guess_codec() if you know exactly what codec you want to use?

The ingredients for choosing a format and codec are:

• format: format short name, filename hint, codec mime type
• codec: the format, codec short name, filename hint, codec mime type

So it seems that the API user should be able to call av_guess_format, supply the format choosing ingredients, get a format. Then call av_guess_codec, supply the codec choosing ingredients, get a codec.

But it is not this way. Here's how my code looks currently:

    e->oformat = av_guess_format(encoder->format_short_name,
            encoder->filename, encoder->mime_type);
    if (!e->oformat) {
        groove_encoder_detach(encoder);
        av_log(NULL, AV_LOG_ERROR, "unable to determine format\n");
        return -1;
    }

    // av_guess_codec ignores mime_type, filename, and codec_short_name. see
    // https://trac.ffmpeg.org/ticket/4706
    // because of this we do a workaround to return the correct codec based on
    // the codec_short_name.
    AVCodec *codec = NULL;
    if (encoder->codec_short_name) {
        codec = avcodec_find_encoder_by_name(encoder->codec_short_name);
        if (!codec) {
            const AVCodecDescriptor *desc =
                avcodec_descriptor_get_by_name(encoder->codec_short_name);
            if (desc) {
                codec = avcodec_find_encoder(desc->id);
            }
        }
    }
    if (!codec) {
        enum AVCodecID codec_id = av_guess_codec(e->oformat,
                encoder->codec_short_name, encoder->filename, encoder->mime_type,
                AVMEDIA_TYPE_AUDIO);
        codec = avcodec_find_encoder(codec_id);
        if (!codec) {
            groove_encoder_detach(encoder);
            av_log(NULL, AV_LOG_ERROR, "unable to find encoder\n");
            return -1;
        }
    }
    e->codec = codec;
    av_log(NULL, AV_LOG_INFO, "encoder: using codec: %s\n", codec->long_name);

This workaround is sub-optimal because figuring out which codec the user wants ignores the codec mime type. Ignoring the filename is not so bad, since really the format ingredient kind of overrides the filename one. Arguably that parameter should be deprecated.

[Carl Eugen Hoyos]

If you know what codec to use calling avcodec_find_encoder_by_name() is correct afaict, av_guess_codec() is wrong (it can be useful if you don't know the short_name of the codec you want to use).
Your code looks as if you had issues with avcodec_find_encoder_by_name() though: Which issues did you see?

[Carl Eugen Hoyos]

Replying to andrewrk:

My original interpretation of this ticket was that you argued that the short_name
parameter of av_guess_codec() should have precedence over the fmt parameter. Was I
correct?

No. The title of the ticket is correct as well as my clarified code example.

In your second example you pass an AVOutputFormat pointer as fmt parameter to av_guess_codec() that was already initialized with codec libvorbis (the default codec for ogg files if libvorbis was enabled). Why should this codec be overwritten if this is neither the documented behaviour of av_guess_codec() nor the behaviour since this function exists?

[Andrew Kelley]

Your code looks as if you had issues with avcodec_find_encoder_by_name() though: Which issues did you see?

I'm sorry, I don't remember.

Why should this codec be overwritten if this is neither the documented behaviour of av_guess_codec() nor the behaviour since this function exists?

I think what you said about this function being very old is reasonable. I would be satisfied with a resolution of updating the documentation for av_guess_codec to make it more clear what the role of the function is.

Thank you for your time and consideration.