On 2014-01-17 15:00:50 +0100, Diego Biurrun wrote:
> On Fri, Jan 17, 2014 at 02:00:06PM +0100, Janne Grunau wrote:
> > On 2014-01-16 03:55:44 +0100, Diego Biurrun wrote:
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -218,8 +219,10 @@ static int set_codec_from_probe_data(AVFormatContext
> > > *s, AVStream *st, AVProbeDa
> > >
> > > if (fmt) {
> > > int i;
> > > - av_log(s, AV_LOG_DEBUG, "Probe with size=%d, packets=%d detected
> > > %s with score=%d\n",
> > > - pd->buf_size, MAX_PROBE_PACKETS - st->probe_packets,
> > > fmt->name, score);
> > > + av_log(s, AV_LOG_DEBUG,
> > > + "Probe with size=%d, packets=%d detected %s with
> > > score=%d\n",
> > > + pd->buf_size, MAX_PROBE_PACKETS - st->probe_packets,
> > > + fmt->name, score);
> >
> > if you split the format string you could get rid of one line, also
> > 'av_log(s, AV_LOG_DEBUG,\n' lloks ugly
>
> I find this "logical grouping" of av_log parameters more readable.
I think it's a waste of space and it looks unbalenced
> > > @@ -738,49 +757,58 @@ static void compute_pkt_fields(AVFormatContext *s,
> > > AVStream *st,
> > >
> > > if (pkt->duration == 0 && st->codec->codec_type !=
> > > AVMEDIA_TYPE_AUDIO) {
> > > ff_compute_frame_duration(&num, &den, st, pc, pkt);
> > > if (den && num) {
> > > - pkt->duration = av_rescale_rnd(1, num *
> > > (int64_t)st->time_base.den, den * (int64_t)st->time_base.num,
> > > AV_ROUND_DOWN);
> > > + pkt->duration = av_rescale_rnd(1,
> > > + num * (int64_t)
> > > st->time_base.den,
> > > + den * (int64_t)
> > > st->time_base.num,
> > > + AV_ROUND_DOWN);
> >
> > please don't this makes my eyes bleed. if the resulting is sligtly above
> > 80 chars let it be
>
> The line is 131 so surely worth splitting; alternatives
>
> pkt->duration = av_rescale_rnd(1, num * (int64_t)
> st->time_base.den,
> den * (int64_t) st->time_base.num,
> AV_ROUND_DOWN);
>
> pkt->duration = av_rescale_rnd(1, num * (int64_t)
> st->time_base.den,
> den * (int64_t) st->time_base.num,
> AV_ROUND_DOWN);
>
> pkt->duration = av_rescale_rnd(1,
> num * (int64_t) st->time_base.den,
> den * (int64_t) st->time_base.num,
> AV_ROUND_DOWN);
>
> The first is still too long, the third nicely aligns parameters, that's
> why I went for that solution.
I would prefer option 2, my display is also limited in the number of
lines.
I'll continue to review the patch
Janne
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
