[Ffmpeg-devel] [PATCH] split av_encode function and do related reorder
Limin Wang
lance.lmwang
Tue Feb 27 02:44:36 CET 2007
Hi,
* Michael Niedermayer <michaelni at gmx.at> [2007-02-26 15:40:19 +0100]:
> Hi
>
> On Mon, Feb 26, 2007 at 07:55:09PM +0800, Limin Wang wrote:
> > Hi,
> >
> > The attached patch try to cleanup ffmpeg.c av_encode function, I have pass
> > "make test" test. Please have review and give your comments. I don't know
> > whether ffmpeg developer like big function?
> >
> > I feel it has too many lines code and too difficult to maintain and develop.
> > So I split it into three functions:
> > av_transcode_init(): ffmpeg transcode initialization
> > av_transcode_start(): main loop of file transcode
> > av_transcode_close(): ffmpeg transcode close, including resource/mem release,
> > the memory release order is first allocate(in av_transcode_init), last release.
> >
> > If the patch is OK, I'll split av_transcode_init() into more functions for it's
> > too big still.
>
> [...]
>
> > Index: ffmpeg.c
> > ===================================================================
> > --- ffmpeg.c (revision 8129)
> > +++ ffmpeg.c (working copy)
> > @@ -277,6 +277,14 @@
> > int nb_streams; /* nb streams we are aware of */
> > } AVInputFile;
> >
> > +typedef struct AVTranscodeContext {
> > + AVOutputStream **ost_table;
> > + AVInputStream **ist_table;
> > + AVInputFile *file_table;
> > + int nb_istreams;
> > + int nb_ostreams;
> > +} AVTranscodeContext;
>
> i see no sense in this struct the variables should stay globals as they are
> global to the process, moving them into a struct just makes it harder to
> access them
>
> also adding such a struct must be in a seperate patch and commit from
> other unrelated changes like spliting av_encode()
OK, I have updated the patch to use global variables directly.
> spliting av_encode() certainly is welcome, but a patch doing so should
> be quite small and rather look like:
>
> -av_encode(){
> +av_encode_init(){
> ...
> +}
> +av_encode_main(){
> ...
> +}
> +av_encode_close(){
>
>
> +av_encode(){
> + av_encode_init();
> + av_encode_main();
> + av_encode_close();
> +}
Have updated the patch to reflect it. Please review it again.
Thanks,
Limin
-------------- next part --------------
Index: ffmpeg.c
===================================================================
--- ffmpeg.c (revision 8129)
+++ ffmpeg.c (working copy)
@@ -277,6 +277,12 @@
int nb_streams; /* nb streams we are aware of */
} AVInputFile;
+static AVOutputStream **ost_table = NULL;
+static AVInputStream **ist_table = NULL;
+static AVInputFile *file_table = NULL;
+static int nb_istreams = 0;
+static int nb_ostreams = 0;
+
#ifndef __MINGW32__
/* init terminal so that we can grab keys */
@@ -1339,28 +1345,136 @@
return -1;
}
+/*
+ * The following code close the av encode resource
+ */
+static int av_encode_close()
+{
+ int ret = 0;
+ int i;
+ AVOutputStream *ost;
+ AVInputStream *ist;
+ ByteIOContext *pb;
+ /* close each encoder */
+ for(i=0;i<nb_ostreams;i++) {
+ ost = ost_table[i];
+ if (ost && ost->encoding_needed) {
+ if( ost->st->codec->stats_in)
+ av_freep(&ost->st->codec->stats_in);
+ if( ost->st->codec )
+ avcodec_close(ost->st->codec);
+ }
+ }
+
+ /* close each decoder */
+ for(i=0;i<nb_istreams;i++) {
+ ist = ist_table[i];
+ if (ist && ist->decoding_needed) {
+ if( ist->st->codec )
+ avcodec_close(ist->st->codec);
+ }
+ }
+
+ if( bit_buffer )
+ av_freep(&bit_buffer);
+
+ if (ost_table) {
+ for(i=0;i<nb_ostreams;i++) {
+ ost = ost_table[i];
+ if (ost) {
+ if (ost->logfile) {
+ fclose(ost->logfile);
+ ost->logfile = NULL;
+ }
+ av_fifo_free(&ost->fifo); /* works even if fifo is not
+ initialized but set to zero */
+ av_free(ost->pict_tmp.data[0]);
+ if (ost->video_resample)
+ sws_freeContext(ost->img_resample_ctx);
+ if (ost->audio_resample)
+ audio_resample_close(ost->resample);
+ av_free(ost);
+ }
+ }
+ av_free(ost_table);
+ }
+
+ if (ist_table) {
+ for(i=0;i<nb_istreams;i++) {
+ ist = ist_table[i];
+ if( ist )
+ av_free(ist);
+ }
+ av_free(ist_table);
+ }
+
+ if( file_table )
+ av_free(file_table);
+
+ /* close output files which opened in opt_output_file */
+ for(i=0;i<nb_output_files;i++) {
+ /* maybe av_close_output_file ??? */
+ AVFormatContext *s = output_files[i];
+ int j;
+
+ pb = &s->pb;
+ if (!(s->oformat->flags & AVFMT_NOFILE) && pb )
+ url_fclose(pb);
+ for(j=0;j<s->nb_streams;j++) {
+ if( s->streams[j]->codec )
+ av_free(s->streams[j]->codec);
+ if( s->streams[j])
+ av_free(s->streams[j]);
+ }
+ av_free(s);
+ }
+
+ /* close input files which opened in opt_input_file */
+ for(i=0;i<nb_input_files;i++)
+ av_close_input_file(input_files[i]);
+
+ /* free memory which allocated in opt_intra_matrix */
+ if(intra_matrix)
+ av_free(intra_matrix);
+
+ /* free memory which allocated in opt_inter_matrix */
+ if(inter_matrix)
+ av_free(inter_matrix);
+
+ av_free_static();
+
+#ifdef CONFIG_POWERPC_PERF
+ extern void powerpc_display_perf_report(void);
+ powerpc_display_perf_report();
+#endif /* CONFIG_POWERPC_PERF */
+
+ if (received_sigterm) {
+ fprintf(stderr,
+ "Received signal %d: terminating.\n",
+ (int) received_sigterm);
+ exit (255);
+ }
+
+ return ret;
+}
+
/*
- * The following code is the main loop of the file converter
+ * The following code initialize encode
*/
-static int av_encode(AVFormatContext **output_files,
- int nb_output_files,
- AVFormatContext **input_files,
- int nb_input_files,
- AVStreamMap *stream_maps, int nb_stream_maps)
+static int av_encode_init()
{
- int ret, i, j, k, n, nb_istreams = 0, nb_ostreams = 0;
- AVFormatContext *is, *os;
- AVCodecContext *codec, *icodec;
- AVOutputStream *ost, **ost_table = NULL;
- AVInputStream *ist, **ist_table = NULL;
- AVInputFile *file_table;
- AVFormatContext *stream_no_data;
- int key;
+ int ret=0, i, j, k, n;
+ AVOutputStream *ost;
+ AVInputStream *ist;
+ AVFormatContext *is;
+ AVFormatContext *os;
+ AVCodecContext *codec;
+ AVCodecContext *icodec;
file_table= (AVInputFile*) av_mallocz(nb_input_files * sizeof(AVInputFile));
if (!file_table)
- goto fail;
+ goto fail_nomem;
/* input stream init */
j = 0;
@@ -1374,12 +1488,12 @@
ist_table = av_mallocz(nb_istreams * sizeof(AVInputStream *));
if (!ist_table)
- goto fail;
+ goto fail_nomem;
for(i=0;i<nb_istreams;i++) {
ist = av_mallocz(sizeof(AVInputStream));
if (!ist)
- goto fail;
+ goto fail_nomem;
ist_table[i] = ist;
}
j = 0;
@@ -1436,11 +1550,11 @@
ost_table = av_mallocz(sizeof(AVOutputStream *) * nb_ostreams);
if (!ost_table)
- goto fail;
+ goto fail_nomem;
for(i=0;i<nb_ostreams;i++) {
ost = av_mallocz(sizeof(AVOutputStream));
if (!ost)
- goto fail;
+ goto fail_nomem;
ost_table[i] = ost;
}
@@ -1458,7 +1572,8 @@
stream_maps[n-1].stream_index;
/* Sanity check that the stream types match */
- if (ist_table[ost->source_index]->st->codec->codec_type != ost->st->codec->codec_type) {
+ if (ist_table[ost->source_index]->st->codec->codec_type
+ != ost->st->codec->codec_type) {
fprintf(stderr, "Codec type mismatch for mapping #%d.%d -> #%d.%d\n",
stream_maps[n-1].file_index, stream_maps[n-1].stream_index,
ost->file_index, ost->index);
@@ -1548,7 +1663,7 @@
switch(codec->codec_type) {
case CODEC_TYPE_AUDIO:
if (av_fifo_init(&ost->fifo, 2 * MAX_AUDIO_PACKET_SIZE))
- goto fail;
+ goto fail_nomem;
if (codec->channels == icodec->channels &&
codec->sample_rate == icodec->sample_rate) {
@@ -1607,14 +1722,14 @@
avcodec_get_frame_defaults(&ost->pict_tmp);
if( avpicture_alloc( (AVPicture*)&ost->pict_tmp, codec->pix_fmt,
codec->width, codec->height ) )
- goto fail;
+ goto fail_nomem;
}
}
if (ost->video_resample) {
avcodec_get_frame_defaults(&ost->pict_tmp);
if( avpicture_alloc( (AVPicture*)&ost->pict_tmp, codec->pix_fmt,
codec->width, codec->height ) )
- goto fail;
+ goto fail_nomem;
ost->img_resample_ctx = sws_getContext(
icodec->width - (frame_leftBand + frame_rightBand),
@@ -1690,7 +1805,7 @@
if (!bit_buffer)
bit_buffer = av_malloc(bit_buffer_size);
if (!bit_buffer)
- goto fail;
+ goto fail_nomem;
/* dump the file output parameters - cannot be done before in case
of stream copy */
@@ -1728,7 +1843,8 @@
exit(1);
}
if (avcodec_open(ost->st->codec, codec) < 0) {
- fprintf(stderr, "Error while opening codec for output stream #%d.%d - maybe incorrect parameters such as bit_rate, rate, width or height\n",
+ fprintf(stderr, "Error while opening codec for output stream #%d.%d "
+ "- maybe incorrect parameters such as bit_rate, rate, width or height\n",
ost->file_index, ost->index);
exit(1);
}
@@ -1783,14 +1899,16 @@
int out_file_index = meta_data_maps[i].out_file;
int in_file_index = meta_data_maps[i].in_file;
if ( out_file_index < 0 || out_file_index >= nb_output_files ) {
- fprintf(stderr, "Invalid output file index %d map_meta_data(%d,%d)\n", out_file_index, out_file_index, in_file_index);
+ fprintf(stderr, "Invalid output file index %d map_meta_data(%d,%d)\n",
+ out_file_index, out_file_index, in_file_index);
ret = AVERROR(EINVAL);
- goto fail;
+ goto fail1;
}
if ( in_file_index < 0 || in_file_index >= nb_input_files ) {
- fprintf(stderr, "Invalid input file index %d map_meta_data(%d,%d)\n", in_file_index, out_file_index, in_file_index);
+ fprintf(stderr, "Invalid input file index %d map_meta_data(%d,%d)\n",
+ in_file_index, out_file_index, in_file_index);
ret = AVERROR(EINVAL);
- goto fail;
+ goto fail1;
}
out_file = output_files[out_file_index];
@@ -1812,7 +1930,7 @@
if (av_write_header(os) < 0) {
fprintf(stderr, "Could not write header for output file #%d (incorrect codec parameters ?)\n", i);
ret = AVERROR(EINVAL);
- goto fail;
+ goto fail1;
}
}
@@ -1820,6 +1938,32 @@
fprintf(stderr, "Press [q] to stop encoding\n");
url_set_interrupt_cb(decode_interrupt_cb);
}
+
+done:
+ return ret;
+
+fail1:
+ av_encode_close();
+ goto done;
+
+fail_nomem:
+ ret = AVERROR(ret);
+ goto fail1;
+}
+
+/*
+ * The following code is the main loop of the file converter
+ */
+static int av_encode_main()
+{
+ int i;
+ AVFormatContext *stream_no_data;
+ AVOutputStream *ost;
+ AVInputStream *ist;
+ AVFormatContext *is;
+ AVFormatContext *os;
+ int key;
+
term_init();
stream_no_data = 0;
@@ -1915,7 +2059,8 @@
if(FFABS(delta) > 1LL*dts_delta_threshold*AV_TIME_BASE && !copy_ts){
input_files_ts_offset[ist->file_index]-= delta;
if (verbose > 2)
- fprintf(stderr, "timestamp discontinuity %"PRId64", new offset= %"PRId64"\n", delta, input_files_ts_offset[ist->file_index]);
+ fprintf(stderr, "timestamp discontinuity %"PRId64", new offset= %"PRId64"\n",
+ delta, input_files_ts_offset[ist->file_index]);
for(i=0; i<file_table[file_index].nb_streams; i++){
int index= file_table[file_index].ist_index + i;
ist_table[index]->next_pts += delta;
@@ -1961,61 +2106,27 @@
/* dump report by using the first video and audio streams */
print_report(output_files, ost_table, nb_ostreams, 1);
- /* close each encoder */
- for(i=0;i<nb_ostreams;i++) {
- ost = ost_table[i];
- if (ost->encoding_needed) {
- av_freep(&ost->st->codec->stats_in);
- avcodec_close(ost->st->codec);
- }
- }
+ return 0;
+}
- /* close each decoder */
- for(i=0;i<nb_istreams;i++) {
- ist = ist_table[i];
- if (ist->decoding_needed) {
- avcodec_close(ist->st->codec);
- }
+/*
+ * The following code is to start the file converter
+ */
+static int av_encode()
+{
+ int ret;
+
+ ret = av_encode_init();
+ if( ret < 0 ) {
+ fprintf(stderr, "av_encode_init failed\n");
+ exit(1);
}
- /* finished ! */
+ av_encode_main();
- ret = 0;
- fail1:
- av_freep(&bit_buffer);
- av_free(file_table);
+ av_encode_close();
- if (ist_table) {
- for(i=0;i<nb_istreams;i++) {
- ist = ist_table[i];
- av_free(ist);
- }
- av_free(ist_table);
- }
- if (ost_table) {
- for(i=0;i<nb_ostreams;i++) {
- ost = ost_table[i];
- if (ost) {
- if (ost->logfile) {
- fclose(ost->logfile);
- ost->logfile = NULL;
- }
- av_fifo_free(&ost->fifo); /* works even if fifo is not
- initialized but set to zero */
- av_free(ost->pict_tmp.data[0]);
- if (ost->video_resample)
- sws_freeContext(ost->img_resample_ctx);
- if (ost->audio_resample)
- audio_resample_close(ost->resample);
- av_free(ost);
- }
- }
- av_free(ost_table);
- }
- return ret;
- fail:
- ret = AVERROR(ENOMEM);
- goto fail1;
+ return 0;
}
#if 0
@@ -3763,7 +3874,6 @@
int main(int argc, char **argv)
{
- int i;
int64_t ti;
av_register_all();
@@ -3791,48 +3901,15 @@
}
ti = getutime();
- av_encode(output_files, nb_output_files, input_files, nb_input_files,
- stream_maps, nb_stream_maps);
+
+ /* start the file converter */
+ av_encode();
+
ti = getutime() - ti;
if (do_benchmark) {
printf("bench: utime=%0.3fs\n", ti / 1000000.0);
}
- /* close files */
- for(i=0;i<nb_output_files;i++) {
- /* maybe av_close_output_file ??? */
- AVFormatContext *s = output_files[i];
- int j;
- if (!(s->oformat->flags & AVFMT_NOFILE))
- url_fclose(&s->pb);
- for(j=0;j<s->nb_streams;j++) {
- av_free(s->streams[j]->codec);
- av_free(s->streams[j]);
- }
- av_free(s);
- }
- for(i=0;i<nb_input_files;i++)
- av_close_input_file(input_files[i]);
-
- av_free_static();
-
- if(intra_matrix)
- av_free(intra_matrix);
- if(inter_matrix)
- av_free(inter_matrix);
-
-#ifdef CONFIG_POWERPC_PERF
- extern void powerpc_display_perf_report(void);
- powerpc_display_perf_report();
-#endif /* CONFIG_POWERPC_PERF */
-
- if (received_sigterm) {
- fprintf(stderr,
- "Received signal %d: terminating.\n",
- (int) received_sigterm);
- exit (255);
- }
-
exit(0); /* not all OS-es handle main() return value */
return 0;
}
More information about the ffmpeg-devel
mailing list