On Mon, 21 Sep 2015 21:15:23 +0200
Michael Niedermayer <michae...@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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
