[FFmpeg-devel] [PATCH] Adding support for OSX bundles to ffmpeg command line

[Robert Swain]

On 21/3/09 02:39, Chris Marrin wrote:
> On Mar 20, 2009, at 5:47 PM, Stefano Sabatini wrote:
>
>> On date Friday 2009-03-20 14:24:30 -0700, Chris Marrin encoded:
>>> On Mar 20, 2009, at 7:40 AM, Benoit Fouet wrote:
>>>> On 03/20/2009 03:36 PM, Chris Marrin wrote:
>> [...]
>>>>> Index: ffmpeg.c
>>>>> ===================================================================
>>>>> --- ffmpeg.c    (revision 18075)
>>>>> +++ ffmpeg.c    (working copy)
>>>>> @@ -88,6 +88,8 @@
>>>>>
>>>>> static const OptionDef options[];
>>>>>
>>>>> +static char* executable_path;
>>>>> +
>>>>> #define MAX_FILES 20
>>>>>
>>>>> static AVFormatContext *input_files[MAX_FILES];
>>>>> @@ -3736,17 +3738,30 @@
>>>>>                            FFMPEG_DATADIR,
>>>>>                          };
>>>>>
>>>>> -    for(i=!base[0]; i<2&&  !f; i++){
>>>>> -        snprintf(filename, sizeof(filename), "%s%s/%s.ffpreset",
>>>>> base[i], i ? "" : "/.ffmpeg", arg);
>>>>> +    // first check for path relative to executable (so presets can
>>>>> be
>>>>> in an OSX package)
>>>>> +    snprintf(filename, sizeof(filename), "%s/../ffpresets/
>>>>> %s.ffpreset",
>>>>> executable_path, arg);
>>>>> +    f= fopen(filename, "r");
>>>>> +    if(!f){
>>>>> +        char *codec_name= *opt == 'v' ? video_codec_name :
>>>>> +                          *opt == 'a' ? audio_codec_name :
>>>>> +                                        subtitle_codec_name;
>>>>> +        snprintf(filename, sizeof(filename),
>>>>> "%s/../ffpresets/%s-%s.ffpreset", executable_path, codec_name,
>>>>> arg);
>>>>>         f= fopen(filename, "r");
>>>>> -        if(!f){
>>>>> -            char *codec_name= *opt == 'v' ? video_codec_name :
>>>>> -                              *opt == 'a' ? audio_codec_name :
>>>>> -                                            subtitle_codec_name;
>>>>> -            snprintf(filename, sizeof(filename), "%s%s/%s-
>>>>> %s.ffpreset",
>>>>> base[i],  i ? "" : "/.ffmpeg", codec_name, arg);
>>>>> -            f= fopen(filename, "r");
>>>>> -        }
>>>>>     }
>>>>> +
>>>>> +    if (!f) {
>>>>> +        for(i=!base[0]; i<2&&  !f; i++){
>>>>> +            snprintf(filename, sizeof(filename), "%s%s/
>>>>> %s.ffpreset",
>>>>> base[i], i ? "" : "/.ffmpeg", arg);
>>>>> +            f= fopen(filename, "r");
>>>>> +            if(!f){
>>>>> +                char *codec_name= *opt == 'v' ? video_codec_name :
>>>>> +                                  *opt == 'a' ? audio_codec_name :
>>>>> +
>>>>> subtitle_codec_name;
>>>>> +                snprintf(filename, sizeof(filename),
>>>>> "%s%s/%s-%s.ffpreset", base[i],  i ? "" : "/.ffmpeg", codec_name,
>>>>> arg);
>>>>> +                f= fopen(filename, "r");
>>>>> +            }
>>>>> +        }
>>>>> +    }
>>>>>
>>>> this is mixing functionnal and cosmetical changes
>>> Could you be more specific? I believe all I have done is added a
>>> check
>>> to see if the preset is at a path relative to the executable_path.
>>> Then (after a check for a successful open) I go into the existing
>>> codepath.
>> Reindenting should be done in a separate patch, so each patch is
>> minimal and more readable.
>>
>> You can use git/quilt/whatever to create stack of patches.
>
> You mean the indenting due to the extra "if (!f)" test? Hmmm, that
> seems odd. It would mean submitting a patch with incorrect
> indentation, which seems confusing. Is that really the submission
> style for ffmpeg?

It is policy that commits only contain either functional or 
non-functional changes. So yes, indentation should not be affected for 
lines which do not contain functional changes. It's just the way we roll.

Regards,
Rob