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

[Chris Marrin]

On Mar 20, 2009, at 7:40 AM, Benoit Fouet wrote:

> On 03/20/2009 03:36 PM, Chris Marrin wrote:
>>
>> On Mar 20, 2009, at 3:06 AM, M?ns Rullg?rd wrote:
>>
>>> Chris Marrin <chris at marrin.com> writes:
>>>
>>>> I'm developing an app called VideoMonkey (http://videomonkey.org).
>>>> It's a video encode for OSX which uses the ffmpeg command line for
>>>> encoding. I need to include ffpresets and I want to put them in the
>>>> bundle. As it is, ffmpeg doesn't support ffpresets in this  
>>>> location.
>>>> So I've made a patch that first looks in a location relative to the
>>>> executable so it can find them in the bundle. I'd like to offer  
>>>> it to
>>>> the main code base.
>>>>
>>>> Here is the patch. Please let me know if there is some other way of
>>>> submitting it. Thanks...
>>>>
>>>>
>>>> 91a92,93
>>>>> static char* executable_path;
>>>>>
>>>> 3717,3718c3719,3727
>>>> <     for(i=!base[0]; i<2 && !f; i++){
>>>
>>> Please create the patch with "svn diff".
>>
>>
>> Done:
>>
>
>
>> 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.

>
>
>>     if(!f && ((arg[0]=='.' && arg[1]=='/') || arg[0]=='/' ||
>>               is_dos_path(arg))){
>>         av_strlcpy(filename, arg, sizeof(filename));
>> @@ -3915,7 +3930,16 @@
>> {
>>     int i;
>>     int64_t ti;
>> -
>> +
>> +    // get the executable path
>> +    char* p = strrchr(argv[0], '/');
>> +    if (p) {
>> +        int length = p-argv[0];
>> +        executable_path = av_malloc(length+1);
>> +        strncpy(executable_path, argv[0], length);
>> +        executable_path[length] = '\0';
>> +    }
>> +
>>
>
> memory leak
>
> also, tabs and trailing white spaces are not allowed in FFmpeg svn


I will fix those. But the bigger question is whether or not the FFmpeg  
team is interested in this patch in general.

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