[MPlayer-dev-eng] -lameopts: conflicting documentation and code?

[D Richard Felker III]

On Sat, Mar 27, 2004 at 11:36:00PM +0000, Diego Biurrun wrote:
> Ivan, Attila and Rich agree this is a bug, current behavior
> contradicts the documentation and expectations.  Since nobody else
> seems to want to step up, I'd like to break through the current
> standstill and apply this next week if no one objects.
> 
> Diego

The patch can go thru, but you still have to drink 10l for top
posting... :)

Rich

> 
> Charles Wilcox writes:
>  > So it has been about a month since I submitted a patch, and the CVS tree
>  > does not show an update.
>  > 
>  > Anyone gonna fix this?
>  > 
>  >  -- Charles Wilcox
>  > 
>  > On Thu, 19 Feb 2004, Charles Wilcox wrote:
>  > 
>  > > Okay, it's kindof trivial, but here it is.
>  > >
>  > > All that is changed is the removal of the erroneous "+1" onto the value of
>  > > 'lame_param_quality'; I also changed the commented example VBR quality
>  > > values appropriately.
>  > >
>  > > Let me know if you have any questions about this,
>  > >
>  > >  -- Charles Wilcox
>  > >
>  > > On Thu, 19 Feb 2004, Diego Biurrun wrote:
>  > >
>  > > > Charles Wilcox writes:
>  > > >  > The man page and HTML documentation say that they q= value can be set from
>  > > >  > 0 to 9, the equivalent of the same option of Lame.  However, this is not
>  > > >  > the case of what happens.  mencoder.c:986 has a line that adds one onto
>  > > >  > the value specified before it is sent off to libmp3lame.  This is
>  > > >  > complicated by the fact that cfg-mencoder.h:25 says that the input is
>  > > >  > indeed in the 0 to 9 range.  Also, the code makes no effort to check to
>  > > >  > see if the resulting value is 10, which is an incorrect value to pass on.
>  > > >  >
>  > > >  > Considering all the docs imply this option should match up to what LAME
>  > > >  > would normally take, could this possibly be a error in the code?  Taking a
>  > > >  > cursory glance at the lame code in frontend/parse.c:1626, it does not alter
>  > > >  > the value before it calls lame_set_VBR_q.
>  > > >  >
>  > > >  > Sorry, I'm just learning how to use Mencoder, but I've used Lame
>  > > >  > extensively, and this inconsistency confuses me.  Is there a reason it it
>  > > >  > like this?  If so, there is no warning to those who'd assume that the VBR
>  > > >  > quality numbers mean the same thing.  (That, and it just doesn't make any
>  > > >  > sense to me.)  If this is a mistake... wow, I guess not many people
>  > > >  > specify -lameopts to have caught this.
>  > > >
>  > > > Sounds like you found a real bug, could you come up with a patch that
>  > > > fixes it, please?
>  > > > Thanks
>  > > >
>  > > > Diego
>  > > >
>  > > >--- MPlayer-1.0pre3/mencoder.c	2003-12-08 16:33:31.000000000 -0500
>  > +++ /usr/local/src/MPlayer-1.0pre3/mencoder.c	2004-02-19 00:07:31.000000000 -0500
>  > @@ -983,7 +983,7 @@
>  >  lame_set_quality(lame,lame_param_algqual); // 0 = best q
>  >  if(lame_param_vbr){  // VBR:
>  >      lame_set_VBR(lame,lame_param_vbr); // vbr mode
>  > -    lame_set_VBR_q(lame,lame_param_quality+1); // 1 = best vbr q  6=~128k
>  > +    lame_set_VBR_q(lame,lame_param_quality); // 0 = best vbr q  5=~128k
>  >      if(lame_param_br>0) lame_set_VBR_mean_bitrate_kbps(lame,lame_param_br);
>  >  } else {    // CBR:
>  >      if(lame_param_br>0) lame_set_brate(lame,lame_param_br);
>  > _______________________________________________
>  > MPlayer-dev-eng mailing list
>  > MPlayer-dev-eng at mplayerhq.hu
>  > http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
> 
> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng