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

[Geza Kovacs]

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,176 @@ static int stream_index_from_inputs(AVFormatContext **input_files,
> >  
> > +/**
> > + * check_same_settings checks that the input formats are identical, used internally by concatenation
> 
> Please capitalize the first word in a sentence and add in a period, same
> below.
> 
> > +static int check_same_settings(AVFormatContext **input_format_contexts, int num_input_files)
> > +{
> > +    int width;
> > +    int height;
> > +    int frame_rate_num;
> > +    int frame_rate_den;
> > +    int i;
> > +    int j;
> 
> I don't think you need to use 6 lines for this.
> 
> > +static int get_audio_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_AUDIO) {
> > +            return input_format_context->streams[i]->codec->codec_id;
> > +        }
> > +    }
> 
> useless {}, same below
> 
> > +static int setup_output_file(AVFormatContext *output_format_context, AVFormatContext *input_format_context)
> > +{
> > +    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;
> 
> This could be aligned, same below.
> 
> > +    int i;
> 
> Did you not get a 'ISO C90 forbids mixed declarations and code' warning?
> If yes, why did you ignore it?
> 
> Diego
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: concatenation-patch-v2.diff
Type: text/x-patch
Size: 11826 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090402/6611920c/attachment.bin>