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

[Diego Biurrun]

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

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