[FFmpeg-devel] [PATCHv3 3/6] dvbsubdec: always return the whole buffer as consumed bytes

[JULIAN GARDNER]

----- Original Message -----
> From: Marton Balint <cus at passwd.hu>
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Cc: 
> Sent: Wednesday, 16 October 2013, 2:12
> Subject: Re: [FFmpeg-devel] [PATCHv3 3/6] dvbsubdec: always return the whole buffer as consumed bytes
> 
> 
> On Wed, 16 Oct 2013, JULIAN GARDNER wrote:
> 
>>>  ________________________________
>>>  From: Marton Balint <cus at passwd.hu>
>>>  To: FFmpeg development discussions and patches 
> <ffmpeg-devel at ffmpeg.org> 
>>>  Sent: Wednesday, 16 October 2013, 1:13
>>>  Subject: Re: [FFmpeg-devel] [PATCHv3 3/6] dvbsubdec: always return the 
> whole buffer as consumed bytes
>>> 
>>> 
>>> 
>>>  On Tue, 15 Oct 2013, JULIAN GARDNER wrote:
>>> 
>>>>>  ________________________________
>>>>>  From: Clément Bœsch <u at pkh.me>
>>>>>  To: FFmpeg development discussions and patches 
> <ffmpeg-devel at ffmpeg.org> Sent: Tuesday, 15 October 2013, 7:53
>>>>>  Subject: Re: [FFmpeg-devel] [PATCHv3 3/6] dvbsubdec: always 
> return the whole buffer as consumed bytes
>>>>> 
>>>>> 
>>>>>  On Sun, Oct 06, 2013 at 09:14:12PM +0200, Marton Balint wrote:
>>>>>>  We don't want leftover junk bytes to be accidentally 
> feeded to us again. Also
>>>>>>  show the number of skipped bytes in the debug output.
>>>>>> 
>>>>>>  Signed-off-by: Marton Balint <cus at passwd.hu>
>>>>>>  ---
>>>>>>    libavcodec/dvbsubdec.c | 5 ++++-
>>>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>> 
>>>>>>  diff --git a/libavcodec/dvbsubdec.c 
> b/libavcodec/dvbsubdec.c
>>>>>>  index 4ce40aa..9754179 100644
>>>>>>  --- a/libavcodec/dvbsubdec.c
>>>>>>  +++ b/libavcodec/dvbsubdec.c
>>>>>>  @@ -1538,7 +1538,10 @@ static int 
> dvbsub_decode(AVCodecContext *avctx,
>>>>>>        if (got_segment == 15 && sub)
>>>>>>            *data_size = dvbsub_display_end_segment(avctx, p, 
> 0, sub);
>>>>>>    -    return p - buf;
>>>>>>  +    if (p - buf < buf_size)
>>>>>>  +        av_log(avctx, AV_LOG_DEBUG, "skipping %d 
> leftover junk bytes\n", buf_size - (int)(p - buf));
>>>>>>  +
>>>>>>  +    return buf_size;
>>>>>>    }
>>>>> 
>>>>>  Any idea what those junk bytes are? Maybe empty packets to stop 
> previous
>>>>>  sub presentation?
>>>>> 
>>>>>  -- Clément B.
>>>>> 
>>>>>  _______________________________________________
>>>>>  ffmpeg-devel mailing list
>>>>>  ffmpeg-devel at ffmpeg.org
>>>>>  http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>> 
>>>> 
>>>>  Before adding this patch, which i dont think is needed, where is 
> the sample Ts stream that has this problem so we can find our what is the real 
> cause as the pes stream shoud NOT have any trailing data.
>>>> 
>>>>  Remember the implementation of DVB subs cheats in the removal cases 
> as it does not use a real timer and/or the empty dvb subs packets.
>>>> 
>>>>  If you have a TS that is showing extraneous chars can i get a copy 
> to chkeck why
>>>> 
>>>>  joolz
>>> 
>>>  Unfortunately I don't have a sample transport stream. The only 
> reference I could find why leftover junk bytes are even possible is here:
>>> 
>>>  http://ffmpeg.org/pipermail/ffmpeg-devel/2011-February/103659.html
>>>  (mplayer ts demuxer outputs an extra byte)
>>> 
>>>  The patch is needed to ensure that leftover bytes still do not cause 
> any frame decode errors after extending avcodec_decode_subtitle2 to support 
> multiple frames in a single packet. So it's for compatibility and keeping 
> existing behaviour.
>>> 
>>>  Regards,
>>>  Marton
>>>  _______________________________________________
>>>  ffmpeg-devel mailing list
>>>  ffmpeg-devel at ffmpeg.org
>>>  http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>> 
>> 
>>  Well I would say "get mplayers demuxer fixed 1st"
>> 
> 
> Yes, sure, but are you saying that we should change the behaviour of the 
> decoder to return AVERROR_INVALIDDATA in case of leftover bytes?
> 
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

but you are changing it, instead of returning the correct number of bytes your returning the number of bytes passed to it, which is the wrong number hence the warning message. Who is to say that these bytes do not have some meaning? I know your trying to fix a bug in mplayer, but why not push them for a fix first.

joolz