[FFmpeg-devel] [PATCH] check if bsf returns an error in ffmpeg.c

[Michael Niedermayer]

On Fri, Jan 18, 2008 at 12:45:22PM +0100, Baptiste Coudurier wrote:
> Hi Benoit, Michael,
> 
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Fri, Jan 18, 2008 at 12:31:15PM +0100, Benoit Fouet wrote:
> >> Michael Niedermayer wrote:
> >>> On Fri, Jan 18, 2008 at 12:05:48PM +0100, Benoit Fouet wrote:
> >>>   
> >>>> Michael Niedermayer wrote:
> >>>>     
> >>>>> On Tue, Jan 15, 2008 at 05:49:21PM +0100, Benoit Fouet wrote:
> >>>>>   
> >>>>>       
> >>>>>> Hi,
> >>>>>>
> >>>>>> attached is a patch to $subj
> >>>>>>     
> >>>>>>         
> >>>>> silently muxing broken data is not good
> >>>>>
> >>>>>   
> >>>>>       
> >>>> do you mean this should be done (muxing) with a message or not done at all ?
> >>>> if the bitstream filter returns an error, it shouldn't have touched the
> >>>> data IMHO, so we would mux input data unchanged
> >>>> (I'll have to have a look at current bitstream filters to know what they
> >>>> do, though)
> >>>>     
> >>> I think mux with error message ...
> >>>
> >>>   
> >> something like the attached patch then ?
> >> (i was wondering whether we should output the message only once or not...)
> > 
> > id say always print
> > 
> > [...]
> >> Index: ffmpeg.c
> >> ===================================================================
> >> --- ffmpeg.c	(revision 11551)
> >> +++ ffmpeg.c	(working copy)
> >> @@ -415,10 +415,12 @@
> >>                                            &new_pkt.data, &new_pkt.size,
> >>                                            pkt->data, pkt->size,
> >>                                            pkt->flags & PKT_FLAG_KEY);
> >> -        if(a){
> >> +        if(a>0){
> >>              av_free_packet(pkt);
> >>              new_pkt.destruct= av_destruct_packet;
> >>          }
> >> +        else if(a<0)
> >> +            print_error("av_bitstream_filter_filter()", ret);
> > 
> > this has a problem, it doesnt print which stream caused the error nor
> > which bitstream filter, both could be quite usefull
> > 
> 
> I wonder if we should not exit like when writing frame fails. If FFmpeg
> cannot do what it is being requested to do, it would be more honest to
> fail instead of creating broken/non compliant files (Im thinking of the
> use of mp4_to_annex_b h264 bistream filter for example).

i thought about this as well ...
my reason for suggesting to print just an error was that with stream copy
and a broken frame a exit() would not be reasonable ...

OTOH if the data came from an encoder exit() is surely appropriate as there
must be a real bug in the code ...

and then we could even use the actual return code of the bitstream filter to
decide how fatal it is ...

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire
-------------- 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/20080118/dba185a0/attachment.pgp>