[FFmpeg-devel] [PATCH 1/2] avcodec/dvdsub: fix partial packet assembly

wm4 nfxjfg at googlemail.com
Mon Sep 21 21:30:59 CEST 2015


On Mon, 21 Sep 2015 21:15:23 +0200
Michael Niedermayer <michaelni at gmx.at> wrote:

> On Mon, Sep 21, 2015 at 06:25:30PM +0200, wm4 wrote:
> > Assuming the first and second packets are partial, this would append the
> > reassembly buffer (ctx->buf) to itself with the second
> > append_to_cached_buf() call, because buf is set to ctx->buf.
> > 
> > I do not know a valid sample file which triggers this, and do not know
> > if packets can be split into more than 2 sub-packets, but it triggered
> > with a (differently) broken sample file in trac issue #4872.
> > ---
> >  libavcodec/dvdsubdec.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> > index 81432e1..57eafbf 100644
> > --- a/libavcodec/dvdsubdec.c
> > +++ b/libavcodec/dvdsubdec.c
> > @@ -535,6 +535,7 @@ static int dvdsub_decode(AVCodecContext *avctx,
> >      const uint8_t *buf = avpkt->data;
> >      int buf_size = avpkt->size;
> >      AVSubtitle *sub = data;
> > +    int appended = 0;
> >      int is_menu;
> >
> >      if (ctx->buf_size) {
> > @@ -545,12 +546,13 @@ static int dvdsub_decode(AVCodecContext *avctx,
> >          }
> >          buf = ctx->buf;
> >          buf_size = ctx->buf_size;
> > +        appended = 1;
> >      }
> >
> >      is_menu = decode_dvd_subtitles(ctx, sub, buf, buf_size);
> >      if (is_menu == AVERROR(EAGAIN)) {
> >          *data_size = 0;
> > -        return append_to_cached_buf(avctx, buf, buf_size);
> > +        return appended ? 0 : append_to_cached_buf(avctx, buf, buf_size);
> 
> this could be simplified by testing buf == ctx->buf or
> ctx->buf_size != 0 instead of adding appended but adding a explicit
> variable is fine too if you prefer
> 
> LGTM
> 
> thanks

I'd prefer to keep it explicit. What would make it actually a lot of
simpler would be copying to the reassembly buffer unconditionally, even
if the packet is not fragmented, but I didn't want to go there yet.


More information about the ffmpeg-devel mailing list