[FFmpeg-devel] [PATCH] Remove hard limit on number of files

[Lucas Clemente Vella]

Michael Niedermayer escreveu:
>>  ffmpeg.c |  261 +++++++++++++++++++++++++++++++++++++++------------------------
>>  1 file changed, 164 insertions(+), 97 deletions(-)
>> bafefd1fcc960a7f82c6291ac67df7f67abbd25b  no_file_limit.patch
>> diff --git a/ffmpeg.c b/ffmpeg.c
>> index e82194f..5913ace 100644
>> --- a/ffmpeg.c
>> +++ b/ffmpeg.c
>> @@ -89,25 +89,31 @@ typedef struct AVMetaDataMap {
>>  
>>  static const OptionDef options[];
>>  
>> -#define MAX_FILES 100
>> -
>>  static const char *last_asked_format = NULL;
>> -static AVFormatContext *input_files[MAX_FILES];
>> -static int64_t input_files_ts_offset[MAX_FILES];
>> -static double input_files_ts_scale[MAX_FILES][MAX_STREAMS];
>> -static AVCodec *input_codecs[MAX_FILES*MAX_STREAMS];
>> +
>> +typedef struct InputFileContext {
>> +    double input_file_ts_scale[MAX_STREAMS];
>> +    int64_t input_file_ts_offset;
>> +    AVFormatContext *format;
>> +    AVCodec *codecs[MAX_STREAMS];
>> +    int nb_codecs;
>> +} InputFileContext;
> 
> we have a AVinputFile struct, why a second input file struct ?
> this is a mess without explanation at least

This AVInputFile is only used internally by av_encode(), but it turns
out that av_encode creates one AVInputFile for each InputFileContext, so
they could somehow be merged. Should they be?

Could make sense leaving them separated, because InputFileContext array
is written while parsing the command line, and only read by av_encode.
AVInputFile is created and updated by the internal logic of av_encode,
and AVInputFile fields only makes sense within its context. (Actually
nb_streams and buffer_size doesn't make any sense, I shall make another
patch to address the issue).

>> +static InputFileContext *input_files = NULL;
>>  static int nb_input_files = 0;
>> -static int nb_icodecs;
>>  
>> -static AVFormatContext *output_files[MAX_FILES];
>> -static AVCodec *output_codecs[MAX_FILES*MAX_STREAMS];
>> +typedef struct OutputFileContext {
>> +    AVFormatContext *format;
>> +    AVCodec *codecs[MAX_STREAMS];
>> +    AVBitStreamFilterContext *bitstream_filters[MAX_STREAMS];
>> +    int nb_codecs;
>> +} OutputFileContext;
>> +static OutputFileContext *output_files = NULL;
>>  static int nb_output_files = 0;
>> -static int nb_ocodecs;
>>  
>> -static AVStreamMap stream_maps[MAX_FILES*MAX_STREAMS];
>> +static AVStreamMap *stream_maps;
>>  static int nb_stream_maps;
>>  
>> -static AVMetaDataMap meta_data_maps[MAX_FILES];
>> +static AVMetaDataMap *meta_data_maps;
>>  static int nb_meta_data_maps;
> 
> you move half of the fields in a struct and these 2 become seperate arrays
> this too is a mess without explanation 

All the fields inside the structs I created refers to the same file
(i.e. for all the arrays that where moved inside the struct, they had
the same index per file). These maps have unrelated index. Theirs
existence depends on command line options -map and -map_meta_data.

For what I am noticing now, these two arrays could be linked lists,
since they are always accessed sequentially (as opposed to output_files
and input_files, that are indexed randomly), but I believe this
implementation with av_realloc is satisfactory.

> [...]
>> @@ -1676,13 +1699,17 @@ static int av_encode(AVFormatContext **output_files,
>>      AVCodecContext *codec, *icodec;
>>      AVOutputStream *ost, **ost_table = NULL;
>>      AVInputStream *ist, **ist_table = NULL;
>> -    AVInputFile *file_table;
>> +    AVInputFile *file_table= NULL;
>> +    uint8_t *no_packet= NULL;
>>      char error[1024];
>>      int key;
>>      int want_sdp = 1;
>> -    uint8_t no_packet[MAX_FILES]={0};
>>      int no_packet_count=0;
>>  
>> +    no_packet= av_mallocz(nb_input_files * sizeof(uint8_t));
>> +    if (!no_packet)
>> +        goto fail;
>> +
> 
> again, an array instead of the struct you added

This array is local. The fields I moved to the struct where globals.

>> @@ -1827,7 +1854,8 @@ static int av_encode(AVFormatContext **output_files,
>>                      }
>>                      if (!found) {
>>                          int i= ost->file_index;
>> -                        dump_format(output_files[i], i, output_files[i]->filename, 1);
>> +                        /* Shouldn't be output_files[k] ? */
>> +                        dump_format(output_files[i].format, i, output_files[i].format->filename, 1);
>>                          fprintf(stderr, "Could not find input stream matching output stream #%d.%d\n",
>>                                  ost->file_index, ost->index);
>>                          av_exit(1);
> 
> annother random comment

Sorry about this one too. I forgot I had two random comments, and I
didn't see it on reviewing the patch.'

>>          default:
>>              abort();
>>          }
>>      }
>>  
>> -    input_files[nb_input_files] = ic;
>> -    input_files_ts_offset[nb_input_files] = input_ts_offset - (copy_ts ? 0 : timestamp);
>> +    fc->input_file_ts_offset = input_ts_offset - (copy_ts ? 0 : timestamp);
>>      /* dump the file content */
>>      if (verbose >= 0)
>>          dump_format(ic, nb_input_files, filename, 0);
> 
> the setting of ic dissappears.

It is in the beginning of the function:

     /* get default parameters from command line */
-    ic = avformat_alloc_context();
+    ic = fc->format = avformat_alloc_context();
     if (!ic) {



The new patch with the random comment removed is attached. By the way, I
renamed two fields from InputFileContext from previous patches:
-    double input_file_ts_scale[MAX_STREAMS];
-    int64_t input_file_ts_offset;
+    double ts_scale[MAX_STREAMS];
+    int64_t ts_offset;

-- 
Lucas Clemente Vella
lvella at gmail.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: no_file_limit.patch
Type: text/x-patch
Size: 32337 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100206/204e2d9c/attachment.bin>