[FFmpeg-devel] [PATCH] libavfilter-soc: make ffplay.c:request_input_frame() check for got_frame
Michael Niedermayer
michaelni
Sun Jan 11 15:20:29 CET 2009
On Sun, Jan 11, 2009 at 01:38:12PM +0100, Stefano Sabatini wrote:
> On date Monday 2009-01-05 19:48:23 +0100, Michael Niedermayer encoded:
> > On Sun, Dec 28, 2008 at 05:22:24PM +0100, Stefano Sabatini wrote:
> > > On date Thursday 2008-12-25 11:45:26 +0100, Stefano Sabatini encoded:
> > > > Hi,
> > > >
> > > > get_video_frame() returns 0 if it didn't get any frame, 1 otherwise,
> > > > and input_get_frame() is *requested* to get a frame, patch add a
> > > > check for this.
> > > >
> > > > Problem discovered when playing MPlayer sample
> > > > MPEG2/res_change_ffmpeg_aspect.ts.
> > > >
> > > > I'm not sure this is the better way to do it from the intrface point of
> > > > view, maybe a get_video_frame() like this:
> > > >
> > > > static int get_video_frame(VideoState *is, AVFrame *frame, uint64_t *pts, AVPacket *pkt, int *got_frame)
> > > >
> > > > would be nicer/more consistent with the rest of libav*, opinions?
> > > >
> > > > Regards.
> > > > --
> > > > FFmpeg = Free and Furious Mysterious Problematic Elastic Gadget
> > >
> > > > Index: libavfilter-soc/ffmpeg/ffplay.c
> > > > ===================================================================
> > > > --- libavfilter-soc.orig/ffmpeg/ffplay.c 2008-12-25 11:35:57.000000000 +0100
> > > > +++ libavfilter-soc/ffmpeg/ffplay.c 2008-12-25 11:36:57.000000000 +0100
> > > > @@ -1472,9 +1472,11 @@
> > > > AVFilterPicRef *picref;
> > > > uint64_t pts = 0;
> > > > AVPacket pkt;
> > > > + int res = 0;
> > > >
> > > > - if(get_video_frame(priv->is, priv->frame, &pts, &pkt) < 0)
> > > > - return -1;
> > > > + while ((res = get_video_frame(priv->is, priv->frame, &pts, &pkt)) <= 0)
> > > > + if (res < 0)
> > > > + return -1;
> > > >
> > > > /* FIXME: until I figure out how to hook everything up to the codec
> > > > * right, we're just copying the entire frame. */
> > >
> > > And in attachment another possibility, for which I have a slight
> > > preference, please say which you prefer if any is acceptable.
> >
> > this probably is ok
>
> I was committing the change where I saw there was a (maybe) simpler
> solution.
> No need to change the get_video_frame() signature, after all it should
> a duty of the get_video_frame() function to ensure that it is indeed
> getting a frame, this change should be functionally equivalent to the
> other patch.
>
> Regards.
> --
> FFmpeg = Foolish and Fostering Minimal Portable Extreme Game
> Index: libavfilter-soc/ffmpeg/ffplay.c
> ===================================================================
> --- libavfilter-soc.orig/ffmpeg/ffplay.c 2009-01-11 12:27:12.000000000 +0100
> +++ libavfilter-soc/ffmpeg/ffplay.c 2009-01-11 12:27:38.000000000 +0100
> @@ -1407,8 +1407,9 @@
>
> static int get_video_frame(VideoState *is, AVFrame *frame, uint64_t *pts, AVPacket *pkt)
> {
> - int len1, got_picture;
> + int len1, got_picture = 0;
>
> + while (!got_picture) {
> if (packet_queue_get(&is->videoq, pkt, 1) < 0)
> return -1;
>
> @@ -1423,6 +1424,7 @@
> len1 = avcodec_decode_video(is->video_st->codec,
> frame, &got_picture,
> pkt->data, pkt->size);
> + }
are you ever freeing the packets?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090111/d0815f9f/attachment.pgp>
More information about the ffmpeg-devel
mailing list