[FFmpeg-devel] [PATCH 2/3] ffmpeg.c: refine picking default video stream

[Gyan Doshi]

On 19-10-2020 08:14 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2020-10-14 20:43:59)
>>
>> On 14-10-2020 02:23 pm, Anton Khirnov wrote:
>>> Use a floating-point score value to take into account bitrate, when
>>> multiple streams with the same resolution are present.
>>>
>>> Stop accessing private AVStream.codec_info_nb_frames field, as the
>>> sample in question
>>> http://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2639/Thailand-Wave.wmv
>>> is now handled by the above change.
>>> ---
>>>    fftools/ffmpeg_opt.c | 23 ++++++++++++++++++-----
>>>    1 file changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
>>> index a0d1b06f2d..afef23919c 100644
>>> --- a/fftools/ffmpeg_opt.c
>>> +++ b/fftools/ffmpeg_opt.c
>>> @@ -19,6 +19,7 @@
>>>     * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>>>     */
>>>    
>>> +#include <float.h>
>>>    #include <stdint.h>
>>>    
>>>    #include "ffmpeg.h"
>>> @@ -2208,15 +2209,27 @@ static int open_output_file(OptionsContext *o, const char *filename)
>>>            char *subtitle_codec_name = NULL;
>>>            /* pick the "best" stream of each type */
>>>    
>>> -        /* video: highest resolution */
>>> +        /* video */
>>>            if (!o->video_disable && av_guess_codec(oc->oformat, NULL, filename, NULL, AVMEDIA_TYPE_VIDEO) != AV_CODEC_ID_NONE) {
>>> -            int best_score = 0, idx = -1;
>>> +            double best_score = 0.0;
>>> +            int idx = -1;
>>>                int qcr = avformat_query_codec(oc->oformat, oc->oformat->video_codec, 0);
>>>                for (i = 0; i < nb_input_streams; i++) {
>>> -                int score;
>>> +                double score;
>>>                    ist = input_streams[i];
>>> -                score = ist->st->codecpar->width * ist->st->codecpar->height + 100000000*!!ist->st->codec_info_nb_frames
>>> -                           + 5000000*!!(ist->st->disposition & AV_DISPOSITION_DEFAULT);
>>> +
>>> +                /* base score is just the area in pixels */
>>> +                score = (double)ist->st->codecpar->width * ist->st->codecpar->height;
>>> +                /* add a fractional part favoring higher bitrate among same-area streams */
>>> +                if (ist->st->codecpar->bit_rate) {
>>> +                    const double bitrate_max = 100e6; // cap at 100Mb/s
>>> +                    const double bitrate = FFMIN(ist->st->codecpar->bit_rate, bitrate_max - 1.0);
>>> +                    score += bitrate / bitrate_max;
>>> +                }
>>> +                /* default streams get max score */
>>> +                if (ist->st->disposition & AV_DISPOSITION_DEFAULT)
>>> +                    score = DBL_MAX;
>> This is actually a mistake that shouldn't ever been committed.
>>
>> A default disposition only has relevance among companion video streams
>> from the same input. It does and should not apply to inter-input
>> comparisons.
>> Only a handful of demuxers set disposition. Default audio selection is
>> already broken with respect to its documented behaviour for, I believe,
>> a couple of years now.
>>
>> The saner way is to select best stream from each input and then select
>> from that shortlist without considering disposition.
> Watches pelcome. I am not bringing about world peace here, I just want
> ffmpeg.c to be a well-behaved user application and stop accessing lavf
> internals.

Will do in a couple of weeks.  So, this patch LGTM then.

Gyan