[FFmpeg-devel] [PATCH] extract bit rate calculation into separate function

[Stefano Sabatini]

On date Saturday 2009-11-14 13:01:33 +0100, Robert Kr?ger encoded:
> On 14.11.2009, at 03:45, Michael Niedermayer wrote:
[...]
>> patch 1:
>> Move the existing code in a new function (this IIRC is pretty much  
>> your first
>> patch with minor changes like tabs/line breaks so it should be littel 
>> work)
>> (This should not change behaviour, any change of behaviour would be a 
>> bug)
>>
>
> I attached a patch that does just that (I hope). The only thing that is 
> already in there in terms of simplification is the elimination of  
> duplicate code by calling av_get_bits_per_sample. I hope that's not too 
> much for one patch to review as it is only the rather obvious  
> replacement of one switch/case statement. I'll send it in two patches if 
> you don't accept this (which I hope you do as I'm just trying to find the 
> best ways to make this splitting up of patches not as tedious as it is 
> now).

Yes please split the patch, as Michael said your first patch should be
quite fine, but for the few doxy and space cosmetics fixes required.

> Index: libavcodec/avcodec.h
> ===================================================================
> --- libavcodec/avcodec.h	(revision 20534)
> +++ libavcodec/avcodec.h	(working copy)
> @@ -30,7 +30,7 @@
>  #include "libavutil/avutil.h"
>  
>  #define LIBAVCODEC_VERSION_MAJOR 52
> -#define LIBAVCODEC_VERSION_MINOR 41
> +#define LIBAVCODEC_VERSION_MINOR 42
>  #define LIBAVCODEC_VERSION_MICRO  0

This is not required since you're not still makin the function public.

>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
> Index: libavcodec/utils.c
> ===================================================================
> --- libavcodec/utils.c	(revision 20534)
> +++ libavcodec/utils.c	(working copy)
> @@ -741,6 +741,35 @@
>      return NULL;
>  }
>
> +int av_get_bit_rate(AVCodecContext *ctx)
> +{
> +    int bit_rate;
> +    int bits_per_sample;
> +
> +    switch(ctx->codec_type) {
> +    case CODEC_TYPE_VIDEO:
> +        bit_rate = ctx->bit_rate;
> +        break;
> +    case CODEC_TYPE_AUDIO:
> +        bits_per_sample = av_get_bits_per_sample(ctx->codec_id);

This factorization is fine but I believe it deserves a separate patch.

[...]

Regards.
-- 
FFmpeg = Faboulous and Fostering MultiPurpose Easy Glue