[FFmpeg-devel] [PATCH] libmp3lame: set noise shaping & psycho acoustic algorithms quality

[Nicolas George]

Le quartidi 14 thermidor, an CCXVI, Michael Niedermayer a ?crit?:
> i agree with robert that profile is not the correct choice and that 
> compression_level would be an option.

All right: here is a new version of the patch using compression_level. The
first one had tabs anyway, sorry.

> if you want an error message for unsupported options, it should be very
> trivial to do once you have a list of supported options for each codec.
> And before you have such a list no system will be able to tell you that
> the option has no effect.
> 
> To clarify how the actual code could work:
> Each AVCodec exports the list of options that do have some effect.
> ffmpeg checks that list before setting an option and prints a warning if
> its not on the list ...

That would do the work to warn about useless options, although I fear that
the list would not always be in sync with what the codecs do.

But that does not address the other problem: each time someone adds support
for a small option like this, he has to either find an existing option with
a name that vaguely resembles what he is doing, or to add a global option
that will clutter the global namespace.

In this particular case, "compression_level" is far from perfect, is it? And
"profile" was no more perfect. But I think people would have not been happy
either if I had proposed to add a brand new option, "algorithmic_complexity"
or something, that would be used only in libmp3lame.c.

And there is another problem: the allowed range for an options is not always
the same, depending on the codec.

The way to go may be not just a list of supported options, but an actual
per-codec set of AVOption structures. Unfortunately, I have no time to dig
into that at the moment.

(By the way, did you notice your PGP key has expired?)

Regards,

-- 
  Nicolas George
-------------- next part --------------
Index: libavcodec/libmp3lame.c
===================================================================
--- libavcodec/libmp3lame.c	(revision 14485)
+++ libavcodec/libmp3lame.c	(working copy)
@@ -50,8 +50,11 @@
     lame_set_in_samplerate(s->gfp, avctx->sample_rate);
     lame_set_out_samplerate(s->gfp, avctx->sample_rate);
     lame_set_num_channels(s->gfp, avctx->channels);
-    /* lame 3.91 dies on quality != 5 */
-    lame_set_quality(s->gfp, 5);
+    if(avctx->compression_level == FF_COMPRESSION_DEFAULT) {
+        lame_set_quality(s->gfp, 5);
+    } else {
+        lame_set_quality(s->gfp, avctx->compression_level);
+    }
     /* lame 3.91 doesn't work in mono */
     lame_set_mode(s->gfp, JOINT_STEREO);
     lame_set_brate(s->gfp, avctx->bit_rate/1000);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080731/f67879d9/attachment.pgp>