[vlc-devel] [vlc-commits] ytdl: stream filter module for YoutubeDL

[Rémi Denis-Courmont]

Le tiistaina 29. syyskuuta 2020, 6.44.08 EEST Pierre Ynard via vlc-devel a 
écrit :
> > +     const struct json_object *best_fmt = NULL;
> > +     double pref_height = var_InheritInteger(s, "preferred-resolution");
> > +     double best_height = -1.;
> > +     double best_abr = -1.;
> > +
> > +     for (size_t i = 0; i < fmts->array.size; i++) {
> > +         const struct json_value *v = &fmts->array.entries[i];
> > +
> > +         if (v->type != JSON_OBJECT)
> > +             continue;
> > +
> > +         const struct json_object *fmt = &v->object;
> > +         double height = json_get_num(fmt, "height");
> > +         double abr = json_get_num(fmt, "abr");
> > +
> > +         if (!isgreaterequal(height, best_height)
> > +          || (best_height < pref_height && pref_height < height))
> > +             continue;
> > +
> > +         if (!isgreaterequal(abr, best_abr))
> > +             continue;
> > +
> > +         best_fmt = fmt;
> > +         best_height = height;
> > +         best_abr = abr;
> > +     }
> 
> This is confusing. First not everybody knows what isgreaterequal() is,
> nor what the point is of using it here (presumably to reject NAN), nor
> why and how we need to handle NAN (presumably because some platforms,
> like YouTube, offer audio- and video-only streams that we aren't
> interested in stitching back together). Maybe it's worth a comment.

I have to disagree. This is written in plain standard C. In fact, we already 
use <math.h> comparison functions elsewhere. Maybe that's not a function that 
HRH Pierre Yanard knew about, but that's not a design criteria for me.

> But I don't understand the resolution selection logic.

There's not much to understand, is there?

If the candidate is not greater or equal than the best so far, skip it.

Else if the best so far is smaller than preference and the candidate is bigger 
than the preference, skip it.

> I don't think it's right. You will want to pick a lower resolution than the
> current best one, if the current best one is above the preferred resolution
> limit. As for the second line I'm even less sure, but if you set
> --preferred-resolution to the lowest possible - 0, or anything lower
> than any available format - it looks to me like it will fail to pick a
> format instead of picking the lowest resolution available.

Point taken.

> It's also unclear whether the code makes the assumption that formats are
> sorted in a particular order, and whether that assumption can be made.

I can't even fathom what makes you think that it assumes a particular order.

-- 
Rémi Denis-Courmont
http://www.remlab.net/