| Message ID | 1d9034fb-8f7c-1fac-c3ac-bc13804eeef7@googlemail.com |
|---|---|
| State | Accepted |
| Commit | 995512328ed84bb737bc364e4ef6fba1994f062a |
| Headers | show |
On 09.11.2016 23:27, Andreas Cadhalpun wrote: > Rects with positive w/h/linesize but no data are invalid. > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > libavcodec/pgssubdec.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c > index cef477d..b50b37b 100644 > --- a/libavcodec/pgssubdec.c > +++ b/libavcodec/pgssubdec.c > @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext *avctx, void *data, > > sub->rects[i]->x = ctx->presentation.objects[i].x; > sub->rects[i]->y = ctx->presentation.objects[i].y; > - sub->rects[i]->w = object->w; > - sub->rects[i]->h = object->h; > - > - sub->rects[i]->linesize[0] = object->w; > > if (object->rle) { > + sub->rects[i]->w = object->w; > + sub->rects[i]->h = object->h; > + > + sub->rects[i]->linesize[0] = object->w; > + > if (object->rle_remaining_len) { > av_log(avctx, AV_LOG_ERROR, "RLE data length %u is %u bytes shorter than expected\n", > object->rle_data_len, object->rle_remaining_len); > Ping. I intend to apply this in a few days. Best regards, Andreas
ti, 2016-11-22 kello 23:21 +0100, Andreas Cadhalpun kirjoitti: > On 09.11.2016 23:27, Andreas Cadhalpun wrote: > > > > Rects with positive w/h/linesize but no data are invalid. > > > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > > --- > > libavcodec/pgssubdec.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c > > index cef477d..b50b37b 100644 > > --- a/libavcodec/pgssubdec.c > > +++ b/libavcodec/pgssubdec.c > > @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext > > *avctx, void *data, > > > > sub->rects[i]->x = ctx->presentation.objects[i].x; > > sub->rects[i]->y = ctx->presentation.objects[i].y; > > - sub->rects[i]->w = object->w; > > - sub->rects[i]->h = object->h; > > - > > - sub->rects[i]->linesize[0] = object->w; > > > > if (object->rle) { > > + sub->rects[i]->w = object->w; > > + sub->rects[i]->h = object->h; > > + > > + sub->rects[i]->linesize[0] = object->w; > > + > > if (object->rle_remaining_len) { > > av_log(avctx, AV_LOG_ERROR, "RLE data length %u is > > %u bytes shorter than expected\n", > > object->rle_data_len, object- > > >rle_remaining_len); > > > Ping. I intend to apply this in a few days. Looks OK, w and h may contain old values if rle is NULL. Maybe those could be set after decode_rle() result check, you could then also simplify the error branch (no need to set w, h back to 0) ? > Best regards, > Andreas > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Wed, 9 Nov 2016 23:27:04 +0100 Andreas Cadhalpun <andreas.cadhalpun@googlemail.com> wrote: > Rects with positive w/h/linesize but no data are invalid. > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> > --- > libavcodec/pgssubdec.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c > index cef477d..b50b37b 100644 > --- a/libavcodec/pgssubdec.c > +++ b/libavcodec/pgssubdec.c > @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext *avctx, void *data, > > sub->rects[i]->x = ctx->presentation.objects[i].x; > sub->rects[i]->y = ctx->presentation.objects[i].y; > - sub->rects[i]->w = object->w; > - sub->rects[i]->h = object->h; > - > - sub->rects[i]->linesize[0] = object->w; > > if (object->rle) { > + sub->rects[i]->w = object->w; > + sub->rects[i]->h = object->h; > + > + sub->rects[i]->linesize[0] = object->w; > + > if (object->rle_remaining_len) { > av_log(avctx, AV_LOG_ERROR, "RLE data length %u is %u bytes shorter than expected\n", > object->rle_data_len, object->rle_remaining_len); Is there any indication that API users are fine with rects that have w=h=0 (or that the API allows it), and that they don't access data with memcpy? Seems a bit odd. Since error handling set w=h=0 before, it's probably ok though.
On 23.11.2016 14:48, wm4 wrote: > On Wed, 9 Nov 2016 23:27:04 +0100 > Andreas Cadhalpun <andreas.cadhalpun@googlemail.com> wrote: > >> Rects with positive w/h/linesize but no data are invalid. >> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >> --- >> libavcodec/pgssubdec.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c >> index cef477d..b50b37b 100644 >> --- a/libavcodec/pgssubdec.c >> +++ b/libavcodec/pgssubdec.c >> @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext *avctx, void *data, >> >> sub->rects[i]->x = ctx->presentation.objects[i].x; >> sub->rects[i]->y = ctx->presentation.objects[i].y; >> - sub->rects[i]->w = object->w; >> - sub->rects[i]->h = object->h; >> - >> - sub->rects[i]->linesize[0] = object->w; >> >> if (object->rle) { >> + sub->rects[i]->w = object->w; >> + sub->rects[i]->h = object->h; >> + >> + sub->rects[i]->linesize[0] = object->w; >> + >> if (object->rle_remaining_len) { >> av_log(avctx, AV_LOG_ERROR, "RLE data length %u is %u bytes shorter than expected\n", >> object->rle_data_len, object->rle_remaining_len); > > Is there any indication that API users are fine with rects that have > w=h=0 (or that the API allows it), and that they don't access data with > memcpy? Seems a bit odd. Since error handling set w=h=0 before, it's > probably ok though. Since rects can contain either a bitmap, or text/ass or both, API users have to deal gracefully with rects without bitmap, i.e. w=h=0. Best regards, Andreas
On 23.11.2016 14:05, Petri Hintukainen wrote: > ti, 2016-11-22 kello 23:21 +0100, Andreas Cadhalpun kirjoitti: >> On 09.11.2016 23:27, Andreas Cadhalpun wrote: >>> >>> Rects with positive w/h/linesize but no data are invalid. >>> >>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> >>> --- >>> libavcodec/pgssubdec.c | 9 +++++---- >>> 1 file changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c >>> index cef477d..b50b37b 100644 >>> --- a/libavcodec/pgssubdec.c >>> +++ b/libavcodec/pgssubdec.c >>> @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext >>> *avctx, void *data, >>> >>> sub->rects[i]->x = ctx->presentation.objects[i].x; >>> sub->rects[i]->y = ctx->presentation.objects[i].y; >>> - sub->rects[i]->w = object->w; >>> - sub->rects[i]->h = object->h; >>> - >>> - sub->rects[i]->linesize[0] = object->w; >>> >>> if (object->rle) { >>> + sub->rects[i]->w = object->w; >>> + sub->rects[i]->h = object->h; >>> + >>> + sub->rects[i]->linesize[0] = object->w; >>> + >>> if (object->rle_remaining_len) { >>> av_log(avctx, AV_LOG_ERROR, "RLE data length %u is >>> %u bytes shorter than expected\n", >>> object->rle_data_len, object- >>>> rle_remaining_len); >>> >> Ping. I intend to apply this in a few days. > > Looks OK, w and h may contain old values if rle is NULL. Pushed. > Maybe those could be set after decode_rle() result check, you could > then also simplify the error branch (no need to set w, h back to 0) ? That's not possible, because decode_rle actually needs w and h set to work correctly. Best regards, Andreas
diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c index cef477d..b50b37b 100644 --- a/libavcodec/pgssubdec.c +++ b/libavcodec/pgssubdec.c @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext *avctx, void *data, sub->rects[i]->x = ctx->presentation.objects[i].x; sub->rects[i]->y = ctx->presentation.objects[i].y; - sub->rects[i]->w = object->w; - sub->rects[i]->h = object->h; - - sub->rects[i]->linesize[0] = object->w; if (object->rle) { + sub->rects[i]->w = object->w; + sub->rects[i]->h = object->h; + + sub->rects[i]->linesize[0] = object->w; + if (object->rle_remaining_len) { av_log(avctx, AV_LOG_ERROR, "RLE data length %u is %u bytes shorter than expected\n", object->rle_data_len, object->rle_remaining_len);
Rects with positive w/h/linesize but no data are invalid. Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com> --- libavcodec/pgssubdec.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)