[FFmpeg-devel] [PATCH 2/2] Do not fail DVB sub decoding because of a few padding bytes

[Reimar Döffinger]

On Wed, Feb 09, 2011 at 11:47:01PM +0000, M?ns Rullg?rd wrote:
> Janne Grunau <janne-ffmpeg at jannau.net> writes:
> 
> > On Wed, Feb 09, 2011 at 11:11:29PM +0000, M?ns Rullg?rd wrote:
> >> Janne Grunau <janne-ffmpeg at jannau.net> writes:
> >> 
> >> > On Wed, Feb 09, 2011 at 09:01:37PM +0000, M?ns Rullg?rd wrote:
> >> >> Janne Grunau <janne-ffmpeg at jannau.net> writes:
> >> >> 
> >> >> > On Wed, Feb 09, 2011 at 07:32:07PM +0100, Reimar D?ffinger wrote:
> >> >> >> Instead of returning an error when bytes are left over, just return
> >> >> >> the number of actually used bytes as other decoders do.
> >> >> >> Instead add a special case so an error will be returned when none
> >> >> >> of the data looks valid to avoid making debugging a pain.
> >> >> >> ---
> >> >> >>  libavcodec/dvbsubdec.c |    9 ++-------
> >> >> >>  1 files changed, 2 insertions(+), 7 deletions(-)
> >> >> >> 
> >> >> >> diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c
> >> >> >> index 8cc8d4f..401144f 100644
> >> >> >> --- a/libavcodec/dvbsubdec.c
> >> >> >> +++ b/libavcodec/dvbsubdec.c
> >> >> >> @@ -1423,7 +1423,7 @@ static int dvbsub_decode(AVCodecContext *avctx,
> >> >> >>  
> >> >> >>  #endif
> >> >> >>  
> >> >> >> -    if (buf_size <= 2)
> >> >> >> +    if (buf_size <= 2 || *buf != 0x0f)
> >> 
> >> What is the point in checking *buf here and again in the loop below?
> >> One of them should be enough.
> >
> > We don't want to return an error after already decoding something but
> > still have to return an error somewhere so that the caller can discard
> > the garbage.
> 
> Where is the garbage coming from?  Is it not an error for it to be there?

Yes, garbage tends to be an error. But a -1 currently is handled as "we couldn't
do anything useful with this, there is no output".
Thanks to the return value, any application can still detect the case where there
is garbage.
However we should still return as much as possible if e.g. there's just an extra
byte or a tiny last part is damaged.
Now of course I could just not add the extra != 0x0f check, however in that case
(there is only garbage) returning -1 seems more appropriate than returning 0.