[FFmpeg-devel] [PATCH] avcodec: Switch bitrate to 64bit with the bump

[Michael Niedermayer]

On Tue, Sep 15, 2015 at 12:49:31AM +0200, Carl Eugen Hoyos wrote:
> On Monday 14 September 2015 04:31:56 pm Michael Niedermayer wrote:
> > On Mon, Sep 14, 2015 at 01:16:34AM +0200, Carl Eugen Hoyos wrote:
> > > On Monday 14 September 2015 12:48:27 am Michael Niedermayer wrote:
> > > > the entries in avcodec_options are wrong
> > >
> > > New patch attached.
> >
> > [...]
> >
> > > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> > > index d6a5c69..f92bc25 100644
> > > --- a/libavcodec/options_table.h
> > > +++ b/libavcodec/options_table.h
> > > @@ -42,8 +42,8 @@
> > >  #define AV_CODEC_DEFAULT_BITRATE 200*1000
> > >
> > >  static const AVOption avcodec_options[] = {
> > > -{"b", "set bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT,
> > > {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, INT_MAX, A|V|E}, -{"ab", "set
> > > bitrate (in bits/s)", OFFSET(bit_rate), AV_OPT_TYPE_INT, {.i64 = 128*1000
> > > }, 0, INT_MAX, A|E}, +{"b", "set bitrate (in bits/s)", OFFSET(bit_rate),
> > > AV_OPT_TYPE_INT64, {.i64 = AV_CODEC_DEFAULT_BITRATE }, 0, INT_MAX,
> > > A|V|E}, +{"ab", "set bitrate (in bits/s)", OFFSET(bit_rate),
> > > AV_OPT_TYPE_INT64, {.i64 = 128*1000 }, 0, INT_MAX, A|E}, {"bt", "Set
> > > video bitrate tolerance (in bits/s). In 1-pass mode, bitrate tolerance
> > > specifies how far " "ratecontrol is willing to deviate from the target
> > > average bitrate value. This is not related " "to minimum/maximum bitrate.
> > > Lowering tolerance too much has an adverse effect on quality.",
> >
> > [...]
> >
> > > diff --git a/libavformat/rdt.c b/libavformat/rdt.c
> > > index 046d273..79c0314 100644
> > > --- a/libavformat/rdt.c
> > > +++ b/libavformat/rdt.c
> > > @@ -448,7 +448,7 @@ real_parse_asm_rule(AVStream *st, const char *p,
> > > const char *end) {
> > >      do {
> > >          /* can be either averagebandwidth= or AverageBandwidth= */
> > > -        if (sscanf(p, " %*1[Aa]verage%*1[Bb]andwidth=%d",
> > > &st->codec->bit_rate) == 1) +        if (sscanf(p, "
> > > %*1[Aa]verage%*1[Bb]andwidth=%"SCNd64, (int64_t *)&st->codec->bit_rate)
> > > == 1)
> >
> > if the type is not already int64 then this is broken
> 
> New patch attached.
> 
> > also noone objected to droping ABI compatibility either on the ML
> > nor on IRC when it was asked. Thus there is currently a unanimous
> > consent to droping it.
> 
> But nobody wants to send a patch with an updated commit message afaict.

maybe we should hire a scribe


> 
> > I dont understand why you modify this patch to support a case you
> > conset to be removed
> 
> Because at some point our API should be stable again.
> 
> > that said, the patches are fine, the bugs are just in cases that
> > the there exist unanimous consent that they should be dropped
> 
> Second patch attached that fixes ticket #2089 for me.

LGTM

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150915/df1bbb9a/attachment.sig>