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

[Anton Khirnov]

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.

-- 
Anton Khirnov