| Message ID | 20161117210922.24484-1-vittorio.giovara@gmail.com |
|---|---|
| State | Accepted |
| Commit | afb84857bf539d7cd58c99ff5983df1b4740d9d2 |
| Headers | show |
On Thu, Nov 17, 2016 at 4:11 PM, Ronald S. Bultje <rsbultje@gmail.com> wrote: > Hi, > > On Thu, Nov 17, 2016 at 4:09 PM, Vittorio Giovara > <vittorio.giovara@gmail.com> wrote: >> >> This prevents writing past bounds. >> >> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> >> --- >> Updated according review. >> Vittorio >> >> doc/filters.texi | 1 + >> libavfilter/vf_colorspace.c | 9 ++++++++- >> 2 files changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/doc/filters.texi b/doc/filters.texi >> index 208be42..aee43a8 100644 >> --- a/doc/filters.texi >> +++ b/doc/filters.texi >> @@ -5278,6 +5278,7 @@ colormatrix=bt601:smpte240m >> @section colorspace >> >> Convert colorspace, transfer characteristics or color primaries. >> +Input video needs to have an even size. >> >> The filter accepts the following options: >> >> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c >> index c7a2286..0024505 100644 >> --- a/libavfilter/vf_colorspace.c >> +++ b/libavfilter/vf_colorspace.c >> @@ -168,7 +168,7 @@ typedef struct ColorSpaceContext { >> int did_warn_range; >> } ColorSpaceContext; >> >> -// FIXME deal with odd width/heights (or just forbid it) >> +// FIXME deal with odd width/heights >> // FIXME faster linearize/delinearize implementation (integer pow) >> // FIXME bt2020cl support (linearization between yuv/rgb step instead of >> between rgb/xyz) >> // FIXME test that the values in (de)lin_lut don't exceed their container >> storage >> @@ -1031,8 +1031,15 @@ static int query_formats(AVFilterContext *ctx) >> >> static int config_props(AVFilterLink *outlink) >> { >> + AVFilterContext *ctx = outlink->dst; >> AVFilterLink *inlink = outlink->src->inputs[0]; >> >> + if (inlink->w % 2 || inlink->h % 2) { >> + av_log(ctx, AV_LOG_ERROR, "Invalid odd size (%dx%d)\n", >> + inlink->w, inlink->h); >> + return AVERROR_PATCHWELCOME; >> + } > > > If 422 && w%2 or 420 && (w%2||h%2), or some variant thereof. Odd sizes for > 444 or odd vertical sizes for 422 should be OK, right? well in theory, but i actually got a segfault on 422 ==4064== Invalid write of size 2 ==4064== at 0x4ED6637: yuv2rgb_422p10_c (colorspacedsp_template.c:89) ==4064== by 0x4F0444C: convert (vf_colorspace.c:504) and on 444 (with testsrc).
Hi, On Thu, Nov 17, 2016 at 5:02 PM, Vittorio Giovara < vittorio.giovara@gmail.com> wrote: > On Thu, Nov 17, 2016 at 4:11 PM, Ronald S. Bultje <rsbultje@gmail.com> > wrote: > > Hi, > > > > On Thu, Nov 17, 2016 at 4:09 PM, Vittorio Giovara > > <vittorio.giovara@gmail.com> wrote: > >> > >> This prevents writing past bounds. > >> > >> Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> > >> --- > >> Updated according review. > >> Vittorio > >> > >> doc/filters.texi | 1 + > >> libavfilter/vf_colorspace.c | 9 ++++++++- > >> 2 files changed, 9 insertions(+), 1 deletion(-) > >> > >> diff --git a/doc/filters.texi b/doc/filters.texi > >> index 208be42..aee43a8 100644 > >> --- a/doc/filters.texi > >> +++ b/doc/filters.texi > >> @@ -5278,6 +5278,7 @@ colormatrix=bt601:smpte240m > >> @section colorspace > >> > >> Convert colorspace, transfer characteristics or color primaries. > >> +Input video needs to have an even size. > >> > >> The filter accepts the following options: > >> > >> diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c > >> index c7a2286..0024505 100644 > >> --- a/libavfilter/vf_colorspace.c > >> +++ b/libavfilter/vf_colorspace.c > >> @@ -168,7 +168,7 @@ typedef struct ColorSpaceContext { > >> int did_warn_range; > >> } ColorSpaceContext; > >> > >> -// FIXME deal with odd width/heights (or just forbid it) > >> +// FIXME deal with odd width/heights > >> // FIXME faster linearize/delinearize implementation (integer pow) > >> // FIXME bt2020cl support (linearization between yuv/rgb step instead > of > >> between rgb/xyz) > >> // FIXME test that the values in (de)lin_lut don't exceed their > container > >> storage > >> @@ -1031,8 +1031,15 @@ static int query_formats(AVFilterContext *ctx) > >> > >> static int config_props(AVFilterLink *outlink) > >> { > >> + AVFilterContext *ctx = outlink->dst; > >> AVFilterLink *inlink = outlink->src->inputs[0]; > >> > >> + if (inlink->w % 2 || inlink->h % 2) { > >> + av_log(ctx, AV_LOG_ERROR, "Invalid odd size (%dx%d)\n", > >> + inlink->w, inlink->h); > >> + return AVERROR_PATCHWELCOME; > >> + } > > > > > > If 422 && w%2 or 420 && (w%2||h%2), or some variant thereof. Odd sizes > for > > 444 or odd vertical sizes for 422 should be OK, right? > > well in theory, but i actually got a segfault on 422 > > ==4064== Invalid write of size 2 > ==4064== at 0x4ED6637: yuv2rgb_422p10_c (colorspacedsp_template.c:89) > ==4064== by 0x4F0444C: convert (vf_colorspace.c:504) > > and on 444 (with testsrc). Well that's embarrassing. OK, I guess patch should go in then. I'll fix that later if anyone actually cares about odd resolutions... Ronald
diff --git a/doc/filters.texi b/doc/filters.texi index 208be42..aee43a8 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -5278,6 +5278,7 @@ colormatrix=bt601:smpte240m @section colorspace Convert colorspace, transfer characteristics or color primaries. +Input video needs to have an even size. The filter accepts the following options: diff --git a/libavfilter/vf_colorspace.c b/libavfilter/vf_colorspace.c index c7a2286..0024505 100644 --- a/libavfilter/vf_colorspace.c +++ b/libavfilter/vf_colorspace.c @@ -168,7 +168,7 @@ typedef struct ColorSpaceContext { int did_warn_range; } ColorSpaceContext; -// FIXME deal with odd width/heights (or just forbid it) +// FIXME deal with odd width/heights // FIXME faster linearize/delinearize implementation (integer pow) // FIXME bt2020cl support (linearization between yuv/rgb step instead of between rgb/xyz) // FIXME test that the values in (de)lin_lut don't exceed their container storage @@ -1031,8 +1031,15 @@ static int query_formats(AVFilterContext *ctx) static int config_props(AVFilterLink *outlink) { + AVFilterContext *ctx = outlink->dst; AVFilterLink *inlink = outlink->src->inputs[0]; + if (inlink->w % 2 || inlink->h % 2) { + av_log(ctx, AV_LOG_ERROR, "Invalid odd size (%dx%d)\n", + inlink->w, inlink->h); + return AVERROR_PATCHWELCOME; + } + outlink->w = inlink->w; outlink->h = inlink->h; outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
This prevents writing past bounds. Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com> --- Updated according review. Vittorio doc/filters.texi | 1 + libavfilter/vf_colorspace.c | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-)