[FFmpeg-devel] Would like to work on lavf-based concatenation tool for GSoC qualification

[Diego Biurrun]

On Thu, Apr 02, 2009 at 06:38:44AM -0400, Geza Kovacs wrote:
> On Thu, 2009-04-02 at 11:26 +0200, Diego Biurrun wrote:
> > On Thu, Apr 02, 2009 at 05:09:16AM -0400, Geza Kovacs wrote:
> > > On Wed, 2009-04-01 at 13:50 -0700, Baptiste Coudurier wrote:
> > > > General comment please try to keep lines reasonably short, I personally
> > > > do not enforce 80 columns, but some people around here like it.
> > 
> > May I remind you of this again?  There are a lot of very long lines in
> > your patch that could be split sensibly.
> 
> I have split the lines where appropriate and have addressed your other
> concerns, see cleaned-up patch.
> 
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -1537,6 +1538,210 @@ static int stream_index_from_inputs(AVFormatContext **input_files,
>  
> +/**
> + * Function check_same_settings checks that the input formats are identical.

"Function check_same_settings" is redundant here.

> + * It is used internally by concatenation.

This sounds weird, what are you trying to say?

> +    char width_set = 0;
> +    char height_set = 0;
> +    char frame_rate_set = 0;

This could be aligned or put on one line.

> +            if (!frame_rate_set) {
> +                frame_rate_set = 1;
> +                frame_rate_num = input_format_contexts[i]->streams[j]->r_frame_rate.num;
> +                frame_rate_den = input_format_contexts[i]->streams[j]->r_frame_rate.den;
> +            }
> +            else if (input_format_contexts[i]->streams[j]->r_frame_rate.num != frame_rate_num

Michael prefers the else on the same line as the {, same below.

> +/**
> + * Retrives the CodecID of the audio stream.

RetriEves, same below.

> +static int get_video_codec(AVFormatContext *input_format_context)
> +{
> +    int i;
> +    for (i = 0; i < input_format_context->nb_streams; ++i) {
> +        if (input_format_context->streams[i]->codec->codec_type == CODEC_TYPE_VIDEO)
> +            return input_format_context->streams[i]->codec->codec_id;
> +    }

useless {}, more below

> +    output_format_context->oformat->video_codec = get_video_codec(input_format_context);
> +    output_format_context->oformat->audio_codec = get_audio_codec(output_format_context);
> +    output_format_context->timestamp = input_format_context->timestamp;
> +    output_format_context->start_time = input_format_context->start_time;
> +    output_format_context->bit_rate = input_format_context->bit_rate;

align

Diego