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

Baptiste Coudurier baptiste.coudurier
Wed Apr 1 22:50:37 CEST 2009


Hi,

Geza Kovacs wrote:
> Hello all,
> 
> I have attached my initial implementation of the lavf-based catenation
> tool for GSoC qualification (this does not of course address the full
> scope of the project description, it merely performs concatenation of
> identical-format files). For now it is implemented as a standalone
> application for simplicity (though I could also add it as an option in
> ffmpeg's parse_options() if needed, I would simply need details on the
> desired invocation syntax for the option).

Yes, please enhance ffmpeg binary. You can put code in a separate file,
but the goal is to enhance ffplay and ffmpeg binaries, so factorizing
the code is likely needed.

> The application currently supports both avi and mp4 input/output
> containers, and appears to work with a variety of codecs (though I have
> primarily tested with mpeg4 and huffyuv). According to ffplay there seem
> to be issues with some files related to writing the header files,
> presumably because I haven't fully populated the oformat struct, but the
> files appear to play nevertheless; I will hopefully be able to track
> this down before Friday.
> 
> I have attempted to follow the described coding and comments standards
> as closely as possible, but if there's an issue please let me know.
> 
> Thanks for the feedback,
> Geza
> 
> On Tue, 2009-03-31 at 10:55 -0700, Baptiste Coudurier wrote:
>> Hi guys,
>>
>> On 3/31/2009 7:26 AM, Robert Swain wrote:
>>> On 31/3/09 15:00, Geza Kovacs wrote:
>>>> I'd like to know, by "concatenation" I assume this means "without
>>>> re-encoding"; therefore I assume I'm not supposed to handle situations
>>>> when multiple files are encoded with different framerate settings or
>>>> different codecs (or do I instead transcode them into an intermediate
>>>> format in these situations?).
>>> Concatenate means to link to together. I'm not sure what the plans are
>>> for handling differences in frame rates or resolutions. Differing source
>>> formats shouldn't matter for transcoding purposes as long as on entry to
>>> the video encoder the resolution, frame rate and pixel format as all
>>> appropriate and similarly the number of channels, sample rate and sample
>>> format for the audio encoder, there shouldn't be any major problem.
>>>
>>> I expect Baptiste will elaborate on this. I think he had plans to allow
>>> specification of command line options per input so that one could affect
>>> the different inputs to make them concatenate-able.
>> Here is the project description:
>>
>> Playlist/Concatenation Support
>> - primary goal: implement a playlist/concatenation interface to
>>     transcode(FFmpeg) and play(FFplay) media
>> - interface will use commandline switches.
>> - interface must support every input format FFmpeg support
>> - interface must work with different input stream parameters (different
>>     formats, codecs, video resolution, audio sample rate, audio
>>     channels, etc..)
>> - interface must support track selection
>> - interface must support existing playlist format files .m3u, .pls,
>>     xpsf.
>>
>> So according to this, different resolution must be handled. About
>> different frame rates, this could be added later I think.
>>

General comment please try to keep lines reasonably short, I personally
do not enforce 80 columns, but some people around here like it.

>> ------------------------------------------------------------------------
>>
>> #include <libavcodec/avcodec.h>
>> #include <libavformat/avformat.h>
>> #include <libavutil/avstring.h>
>> #include <malloc.h>

Use av_malloc/etc.., see "libavutil/mem.h"

>> /**
>>  * open_input_files returns AVFormatContexts for a list of input files
>>  * @param char **input_files list of input files,
>>  * @param int num_input_files number of input files.
>>  * @return returns NULL on error, otherwise a list of input format contexts
>>  */
>> AVFormatContext** open_input_files(char **input_files, int num_input_files) {
>>     AVFormatContext **input_format_contexts = (AVFormatContext**)malloc(num_input_files*sizeof(AVFormatContext*));

Useless cast in plain C, there are plenty in the file.

>>     AVFormatParameters **input_format_parameters = (AVFormatParameters**)malloc(num_input_files*sizeof(AVFormatParameters*));
>>     int i;
>>     for (i = 0; i < num_input_files; ++i) {
>>         if (av_open_input_file(input_format_contexts+i, input_files[i], NULL, 0, NULL) != 0) {

< 0 is usually used here.

>>             fprintf(stderr, "Error while opening %s\n", input_files[i]);
>>             fflush(stderr);

Do you really need to flush here ?

>> [...]
>>
>> /**
>>  * close_input_files closes a list of AVFormatContext*
>>  * @param AVFormatContext **input_format_contexts list of input format contexts
>>  * @param int num_input_files number of input files.
>>  * @return returns nothing
>>  */
>> void close_input_files(AVFormatContext **input_format_contexts, int num_input_files) {
>>     int i;
>>     for (i = 0; i < num_input_files; ++i) {
>>         /* av_close_input_stream(input_format_contexts[i]); */
>>         av_close_input_file(input_format_contexts[i]);
>>         /* av_free(input_format_contexts[i]); */
>>     }

Please remove debugging code.

>> [...]
>>
>>             if (!color_table_id_set) {
>>                 color_table_id_set = 1;
>>                 color_table_id = input_format_contexts[i]->streams[j]->codec->color_table_id;
>>             }

There is a FIXME near color_table_id, did you encounter a case where you
need to check this ?

The mechanism you are using will be good for stream copying.
There are however some check lacking:

"extradata" handling: extradata might differ from one stream to another,
this happens offently with H.264

The code must be ready to add "filters" if video resolution does not
match, or audio sample format differs. This will be needed.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org



More information about the ffmpeg-devel mailing list