[FFmpeg-devel] [PATCH 1/2] avcodec/libmp3lame: cosmetics

[Stefano Sabatini]

On date Wednesday 2013-10-23 21:03:51 -0700, Timothy Gu encoded:
> On Sun, Oct 20, 2013 at 10:46 AM, Stefano Sabatini <stefasab at gmail.com> wrote:
> > comments != cosmetics
> >
> > On date Friday 2013-10-18 17:47:39 -0700, Timothy Gu encoded:
> >> Signed-off-by: Timothy Gu <timothygu99 at gmail.com>
> >> ---
> >>  libavcodec/libmp3lame.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >> diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
> >> index 2204f55..9cd2f46 100644
> >> --- a/libavcodec/libmp3lame.c
> >> +++ b/libavcodec/libmp3lame.c
> >> @@ -115,11 +115,11 @@ static av_cold int mp3lame_encode_init(AVCodecContext *avctx)
> >>          lame_set_quality(s->gfp, avctx->compression_level);
> >>
> >>      /* rate control */
> >> -    if (avctx->flags & CODEC_FLAG_QSCALE) {
> >> +    if (avctx->flags & CODEC_FLAG_QSCALE) { // VBR
> >>          lame_set_VBR(s->gfp, vbr_default);
> >>          lame_set_VBR_quality(s->gfp, avctx->global_quality / (float)FF_QP2LAMBDA);
> >>      } else {
> >> -        if (avctx->bit_rate)
> >> +        if (avctx->bit_rate)                // CBR
> >>              lame_set_brate(s->gfp, avctx->bit_rate / 1000);
> >>      }
> >>
> >
> >> @@ -263,8 +263,8 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
> >>  #define OFFSET(x) offsetof(LAMEContext, x)
> >>  #define AE AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
> >>  static const AVOption options[] = {
> >> -    { "reservoir", "Use bit reservoir.", OFFSET(reservoir), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> >> -    { "joint_stereo", "Use joint stereo.", OFFSET(joint_stereo), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> >> +    { "reservoir",    "Use bit reservoir.", OFFSET(reservoir),    AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> >> +    { "joint_stereo", "Use joint stereo.",  OFFSET(joint_stereo), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> >
> > I'd suggest to avoid realign commits (especially if you're not the
> > maintainer of the file), they add up to code churnup for no evident
> > benefit.
> >
> > BTW the grammar form is inconsistent (it should be "use bit
> > reservoir") etc., in case you fix that a realign is acceptable.
> 
> Patch splitted and all remarks fixed.
> 
> Timothy

> From ad3291706576955d64926621e37f664507cf41fc Mon Sep 17 00:00:00 2001
> From: Timothy Gu <timothygu99 at gmail.com>
> Date: Wed, 23 Oct 2013 20:55:29 -0700
> Subject: [PATCH 1/3] avcodec/libmp3lame: fix alignment and capitalization in
>  AVOptions
> 
> Signed-off-by: Timothy Gu <timothygu99 at gmail.com>
> ---
>  libavcodec/libmp3lame.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
> index 2204f55..6402d3e 100644
> --- a/libavcodec/libmp3lame.c
> +++ b/libavcodec/libmp3lame.c
> @@ -263,8 +263,8 @@ static int mp3lame_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
>  #define OFFSET(x) offsetof(LAMEContext, x)
>  #define AE AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
>  static const AVOption options[] = {
> -    { "reservoir", "Use bit reservoir.", OFFSET(reservoir), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> -    { "joint_stereo", "Use joint stereo.", OFFSET(joint_stereo), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> +    { "reservoir",    "use bit reservoir", OFFSET(reservoir),    AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
> +    { "joint_stereo", "use joint stereo",  OFFSET(joint_stereo), AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, AE },
>      { NULL },
>  };
>  
> -- 
> 1.8.1.2
> 

> From e1767bfce804b0db4018faa9f80a651edcf08b61 Mon Sep 17 00:00:00 2001
> From: Timothy Gu <timothygu99 at gmail.com>
> Date: Wed, 23 Oct 2013 20:57:36 -0700
> Subject: [PATCH 2/3] avcodec/libmp3lame: add comment about CBR/VBR modes
> 
> Signed-off-by: Timothy Gu <timothygu99 at gmail.com>
> ---
>  libavcodec/libmp3lame.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/libmp3lame.c b/libavcodec/libmp3lame.c
> index 6402d3e..0fb9c6a 100644
> --- a/libavcodec/libmp3lame.c
> +++ b/libavcodec/libmp3lame.c
> @@ -115,11 +115,11 @@ static av_cold int mp3lame_encode_init(AVCodecContext *avctx)
>          lame_set_quality(s->gfp, avctx->compression_level);
>  
>      /* rate control */
> -    if (avctx->flags & CODEC_FLAG_QSCALE) {
> +    if (avctx->flags & CODEC_FLAG_QSCALE) { // VBR
>          lame_set_VBR(s->gfp, vbr_default);
>          lame_set_VBR_quality(s->gfp, avctx->global_quality / (float)FF_QP2LAMBDA);
>      } else {
> -        if (avctx->bit_rate)
> +        if (avctx->bit_rate)                // CBR
>              lame_set_brate(s->gfp, avctx->bit_rate / 1000);
>      }

Will push them soon, thanks.
-- 
FFmpeg = Formidable Fundamental Mind-dumbing Pitiless Enhanced Game