[FFmpeg-devel] [PATCH] api-example for libavfilter

Nicolas George nicolas.george
Sun Dec 12 17:45:19 CET 2010


Le primidi 21 frimaire, an CCXIX, Michael Niedermayer a ?crit?:
> what follows is more a review of the APIs than the api example, as i think its
> a great oppertunity to review the APIs based on the example ...

Thanks for that review.

> 
> 
> 
> >  2 files changed, 236 insertions(+), 0 deletions(-)
> > 
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index aece3ab..b5d9176 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -56,4 +56,8 @@ OBJS-$(CONFIG_NULLSINK_FILTER)               += vsink_nullsink.o
> >  
> >  DIRS = x86
> >  
> > +EXAMPLES  = api
> > +
> >  include $(SUBDIR)../subdir.mak
> > +
> > +$(SUBDIR)decode_filter-example$(EXESUF): ELIBS = -lavformat -lavcodec -lavcore -lavutil $(FFEXTRALIBS)
> > diff --git a/libavfilter/decode_filter-example.c b/libavfilter/decode_filter-example.c
> > new file mode 100644
> > index 0000000..526111a
> > --- /dev/null
> > +++ b/libavfilter/decode_filter-example.c
> > @@ -0,0 +1,232 @@
> > +/*
> > + * API example for libavfilter
> > + * Copyright (c) 2010 Nicolas George
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > + */
> > +
> > +#define _XOPEN_SOURCE 600 /* for usleep */
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <libavformat/avformat.h>
> > +#include <libavcodec/avcodec.h>
> > +#include <libavfilter/avfiltergraph.h>
> > +#include <libavfilter/vsrc_buffer.h>
> > +
> > +const char *filter_descr = "scale=78:24,format=gray";
> > +
> > +static AVFormatContext *avf;
> > +static AVCodecContext *video_dec;
> > +AVFilterContext *video_in_filter;
> > +AVFilterContext *video_out_filter;
> > +AVFilterGraph *filter_graph;
> > +static int video_stream = -1;
> > +static int64_t last_pts = AV_NOPTS_VALUE;
> > +
> > +static void fatal_libav_error(const char *tag, int r)
> > +{
> > +    char buf[1024];
> > +
> > +    av_strerror(r, buf, sizeof(buf));
> > +    fprintf(stderr, "%s: %s\n", tag, buf);
> > +    exit(1);
> > +}
> > +
> > +static void open_input_file(const char *filename)
> > +{
> > +    int r, i;
> > +    AVCodec *codec;
> > +    AVCodecContext *avc;
> > +

> > +    avcodec_register_all();
> > +    av_register_all();
> already called in main()

Fixed in my working repository.

> We have code in ffplay.c that selects the "best" stream of a kind
> (see surrounding code when you grep for st_best_packet_count)
> this should be moved into the public API of libavformat and used here
> and from ffplay

The logic seems to be to select the stream with the maximum value for
codec_info_nb_frames.

May I suggest:

	int av_find_best_streams(AVFormatContext *ic,
	                         int flags,
				 int *video_stream,
				 int *audio_stream,
				 int *subtitles_stream,
				 int flags);

> make libavcodec call avcodec_find_decoder() when codec=NULL and this becomes
> simpler

Good idea indeed.

> > +    /* Endpoints for the filter graph. */
> > +    outputs->name       = av_strdup("in");
> > +    outputs->filter_ctx = video_in_filter;
> > +    outputs->pad_idx    = 0;
> > +    outputs->next       = NULL;
> > +    inputs->name       = av_strdup("out");
> > +    inputs->filter_ctx = video_out_filter;
> > +    inputs->pad_idx    = 0;
> > +    inputs->next       = NULL;
> These 2 blocks seems somewhat redundant
> the avfilter_graph_create_filter() could do that already somehow, maybe if
> we move the 2 AVFilterInOut into filter_graph

I will leave Stefano answer to that one.

> > +            putchar(" .-+#"[*(p++) / 52]);
> p[x] would avoid the temp p vs p0 varible

Changed in my working repository.

> EAGAIN handling is missing

Changed in my working repository.

I would like another point about the API:

>>            if (packet.pts != AV_NOPTS_VALUE)
>>                video_dec->reordered_opaque =
>>                    av_rescale_q(packet.pts,
>>                                 avf->streams[video_stream]->time_base,
>>                                 video_dec->time_base);
>>            r = avcodec_decode_video2(video_dec, &frame, &got_frame, &packet);
>>            if (r < 0)
>>                fatal_libav_error("avcodec_decode_video2", r);
>>            if (got_frame) {
>>                if (frame.pts == AV_NOPTS_VALUE)
>>                    frame.pts = frame.reordered_opaque;
>>                got_video_frame(&frame);
>>            }

I think a wrapper around avcodec_decode_video2 that takes care of the
reordered_opaque stuff to fill in PTS using the packet data would be useful
for a lot of applications. It could include the guess_correct_pts logic too.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101212/711d1684/attachment.pgp>



More information about the ffmpeg-devel mailing list