To close debian bug #271052,
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=271052
I went ahead and did the following:
$ diff -u ../original/speex-1.1.6/libspeex/nb_celp.c libspeex/nb_celp.c
--- ../original/speex-1.1.6/libspeex/nb_celp.c 2004-07-15 01:16:52.000000000
-0400
+++ libspeex/nb_celp.c 2005-02-27 08:24:49.000000000 -0500
@@ -1746,6 +1746,21 @@
st->submodeSelect = st->submodeID =
((SpeexNBMode*)(st->mode->mode))->quality_map[quality];
}
break;
+ case SPEEX_GET_QUALITY:
+ {
+ int quality;
+
+ for(quality=0;quality <=10; quality++)
+ if (st->submodeID ==
((SpeexNBMode*)(st->mode->mode))->quality_map[quality]) break;
+
+ if (quality>10) {
+ (*(int*)ptr) = -1;
+ speex_warning("quality not set");
+ return -1;
+ } else
+ (*(int*)ptr) = quality;
+ }
+ break;
case SPEEX_SET_COMPLEXITY:
st->complexity = (*(int*)ptr);
if (st->complexity<1)
$ diff -u ../original/speex-1.1.6/libspeex/sb_celp.c libspeex/sb_celp.c
--- ../original/speex-1.1.6/libspeex/sb_celp.c 2004-07-15 01:16:52.000000000
-0400
+++ libspeex/sb_celp.c 2005-02-27 08:33:38.000000000 -0500
@@ -1217,6 +1217,13 @@
speex_encoder_ctl(st->st_low, SPEEX_SET_MODE, &nb_qual);
}
break;
+ case SPEEX_GET_QUALITY:
+ {
+ int quality;
+ speex_encoder_ctl(st->st_low, SPEEX_GET_QUALITY, &quality);
+ (*(int*)ptr) = quality;
+ }
+ break;
case SPEEX_SET_COMPLEXITY:
speex_encoder_ctl(st->st_low, SPEEX_SET_COMPLEXITY, ptr);
st->complexity = (*(int*)ptr);
Is that good enough to add to the speex subversion repo for inclusion in
future releases?
-Maitland
Hi Maitland, I think your solution is probably the best implementation you could do for SPEEX_GET_QUALITY. However, unless you have something against that, I think removing it would be better, as in many cases (e.g. VBR), the call just doesn't make sense). This would of course break the API (not the ABI), but given the fact that it was never implemented, any app that uses it is already broken anyway. Jean-Marc Le dimanche 27 f?vrier 2005 ? 08:45 -0500, A. Maitland Bottoms a ?crit :> To close debian bug #271052, > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=271052 > > I went ahead and did the following: > > $ diff -u ../original/speex-1.1.6/libspeex/nb_celp.c libspeex/nb_celp.c > --- ../original/speex-1.1.6/libspeex/nb_celp.c 2004-07-15 01:16:52.000000000 -0400 > +++ libspeex/nb_celp.c 2005-02-27 08:24:49.000000000 -0500 > @@ -1746,6 +1746,21 @@ > st->submodeSelect = st->submodeID = ((SpeexNBMode*)(st->mode->mode))->quality_map[quality]; > } > break; > + case SPEEX_GET_QUALITY: > + { > + int quality; > + > + for(quality=0;quality <=10; quality++) > + if (st->submodeID == ((SpeexNBMode*)(st->mode->mode))->quality_map[quality]) break; > + > + if (quality>10) { > + (*(int*)ptr) = -1; > + speex_warning("quality not set"); > + return -1; > + } else > + (*(int*)ptr) = quality; > + } > + break; > case SPEEX_SET_COMPLEXITY: > st->complexity = (*(int*)ptr); > if (st->complexity<1) > > $ diff -u ../original/speex-1.1.6/libspeex/sb_celp.c libspeex/sb_celp.c > --- ../original/speex-1.1.6/libspeex/sb_celp.c 2004-07-15 01:16:52.000000000 -0400 > +++ libspeex/sb_celp.c 2005-02-27 08:33:38.000000000 -0500 > @@ -1217,6 +1217,13 @@ > speex_encoder_ctl(st->st_low, SPEEX_SET_MODE, &nb_qual); > } > break; > + case SPEEX_GET_QUALITY: > + { > + int quality; > + speex_encoder_ctl(st->st_low, SPEEX_GET_QUALITY, &quality); > + (*(int*)ptr) = quality; > + } > + break; > case SPEEX_SET_COMPLEXITY: > speex_encoder_ctl(st->st_low, SPEEX_SET_COMPLEXITY, ptr); > st->complexity = (*(int*)ptr); > > > Is that good enough to add to the speex subversion repo for inclusion in > future releases? > > -Maitland > _______________________________________________ > Speex-dev mailing list > Speex-dev@xiph.org > http://lists.xiph.org/mailman/listinfo/speex-dev >-- Jean-Marc Valin <Jean-Marc.Valin@USherbrooke.ca> Universit? de Sherbrooke
On Sun, Feb 27, 2005 at 06:09:48PM -0500, Jean-Marc Valin wrote:> Hi Maitland, > > I think your solution is probably the best implementation you could do > for SPEEX_GET_QUALITY. However, unless you have something against that, > I think removing it would be better, as in many cases (e.g. VBR), the > call just doesn't make sense). This would of course break the API (not > the ABI), but given the fact that it was never implemented, any app that > uses it is already broken anyway.I like the symmetry of having both GET and SET. If you ask a library to set an internal value, it's nice to be able to retrieve that from the library later, and know that you're getting the actual current value. It's also nice to be able to GET the current value, before you've set anything -- eg. to render a slider when setting up an encode. Anyway, how is it that the call "doesn't make sense" for VBR? if there's no defined value in some cases, perhaps simply returning an UNDEFINED error would be useful? Conrad.