[FFmpeg-devel] [PATCH] api-example for libavfilter
Michael Niedermayer
michaelni
Sun Dec 12 23:15:51 CET 2010
On Sun, Dec 12, 2010 at 05:45:19PM +0100, Nicolas George wrote:
> 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);
looks good
>
> > 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.
Yes, i agree
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 3
"Rare item" - "Common item with rare defect or maybe just a lie"
"Professional" - "'Toy' made in china, not functional except as doorstop"
"Experts will know" - "The seller hopes you are not an expert"
-------------- 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/271b58fb/attachment.pgp>
More information about the ffmpeg-devel
mailing list