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

[Chris Marrin]

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?

-----
~Chris
chris at marrin.com