[FFmpeg-devel] [PATCH] ffmpeg: handle the case when get_filtered_frame() fails

[Michael Niedermayer]

On Sat, May 21, 2011 at 12:04:45AM +0200, Stefano Sabatini wrote:
> On date Friday 2011-05-20 18:31:29 +0200, Michael Niedermayer encoded:
> > On Fri, May 20, 2011 at 09:11:40AM +0200, Stefano Sabatini wrote:
> > > On date Friday 2011-05-20 02:38:26 +0200, Michael Niedermayer encoded:
> > > > On Fri, May 20, 2011 at 01:02:26AM +0200, Stefano Sabatini wrote:
> > > > > ---
> > > > >  ffmpeg.c |    3 ++-
> > > > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > > > 
> > > > > diff --git a/ffmpeg.c b/ffmpeg.c
> > > > > index fb644ad..b6ee7e3 100644
> > > > > --- a/ffmpeg.c
> > > > > +++ b/ffmpeg.c
> > > > > @@ -1690,7 +1690,8 @@ static int output_packet(AVInputStream *ist, int ist_index,
> > > > >                  while (frame_available) {
> > > > >                      AVRational ist_pts_tb;
> > > > >                      if (ist->st->codec->codec_type == AVMEDIA_TYPE_VIDEO && ost->output_video_filter)
> > > > > -                        get_filtered_video_frame(ost->output_video_filter, &picture, &ost->picref, &ist_pts_tb);
> > > > > +                        if (get_filtered_video_frame(ost->output_video_filter, &picture, &ost->picref, &ist_pts_tb) < 0)
> > > > > +                            goto cont;
> > > > >                      if (ost->picref)
> > > > 
> > > > Can you elaborate when such failure happens ?
> > > > frame_available inplicates poll_frame() so request shouldnt fail i
> > > > think
> > > 
> > > Indeed this needs some explanation, as the failure I was experiencing
> > > comes from a pending select filter.
> > > 
> > > The filter works by selecting a frame according to an expression
> > > evaluated on the frame properties. poll() is propagated to the source,
> > > so we could have this situation:
> > > buffer tells that there is a frame, but when the frame is requested
> > > and is not selected request_frame() will fail. I tried other buffering
> > > methods but I concluded that this model was the cleaner/simpler.
> > 
> > I think thats problematic.
> > 
> > With a filter graph with multiple outputs one needs to know where
> > frames are available to pull them out
> > otherwise a split filter followed by a frame duplication filter on
> > one side could quickly eat up all memory as frames are pulled out of
> > one side while more and more accumulate on the other.
> > 
> > also its problematic inside the graph
> > a filter that combined n inputs might check with poll_frame if
> > they are all available before requesting all and combining.
> > if now one fails this makes buffering requirements for it more complex.
> > 
> > either way, this would require changing the documented API:
> >    /**
> >      * Frame poll callback. This returns the number of immediately available
> >      * samples. It should return a positive value if the next request_frame()
> >      * is guaranteed to return one frame (with no delay).
> >      *
> >      * Defaults to just calling the source poll_frame() method.
> >      *
> >      * Output video pads only.
> >      */
> >     int (*poll_frame)(AVFilterLink *link);
> 
> The select filter is problematic in the sense that you don't know if
> you have an available frame until you filter it. So I'm not sure there
> is a way to implement it with the current model, or if it is possible
> to use it only if some conditions are specified (for example it would
> be fine without the poll API).

a framedrop filter that drops similar frames has the same issue.

a possible solution might be to return a "dont know" value like -1
from poll_frame()
and have a filter (maybe fifo) that would simply try a request_frame()
when its source doesnt know.


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20110521/7b9d5c75/attachment.asc>