Hi,
On Fri, Mar 24, 2017 at 10:05 AM, Clément Bœsch <u...@pkh.me> wrote:
> On Fri, Mar 24, 2017 at 10:40:21AM +0100, Carl Eugen Hoyos wrote:
> > Hi!
> >
> > Attached patch fixes #6255.
> >
> > Please comment, Carl Eugen
>
> > From 1c249440c62271565be12112f321ad9aa6a922fb Mon Sep 17 00:00:00 2001
> > From: Carl Eugen Hoyos <ceho...@ag.or.at>
> > Date: Fri, 24 Mar 2017 10:38:22 +0100
> > Subject: [PATCH] lavc/h264_ps: Check that chroma_location is within
> limits.
> >
> > Fixes ticket #6255.
> > ---
> > libavcodec/h264_ps.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> > index b78ad25..55e6a6e 100644
> > --- a/libavcodec/h264_ps.c
> > +++ b/libavcodec/h264_ps.c
> > @@ -181,6 +181,8 @@ static inline int decode_vui_parameters(GetBitContext
> *gb, AVCodecContext *avctx
> > if (get_bits1(gb)) {
> > /* chroma_sample_location_type_top_field */
> > avctx->chroma_sample_location = get_ue_golomb(gb) + 1;
> > + if (avctx->chroma_sample_location >= AVCHROMA_LOC_NB)
> > + avctx->chroma_sample_location = AVCHROMA_LOC_UNSPECIFIED;
>
> if (av_chroma_location_name(...)) ...
Looking at the bug report mentioned, I wonder why we have this
inconsistency in our code:
s = av_get_colorspace_name(par->color_space);
if (s) print_str ("color_space", s);
else print_str_opt("color_space", "unknown");
[..]
if (par->chroma_location != AVCHROMA_LOC_UNSPECIFIED)
print_str("chroma_location",
av_chroma_location_name(par->chroma_location));
else
print_str_opt("chroma_location",
av_chroma_location_name(par->chroma_location));
This seems inconsistent. Making the second (chroma location printing)
behave like the first (colorspace printing: print "unknown" for invalid
entries) would make the above unnecessary.
There is also the risk with the initial patch that compiling libavcodec
against a newer libavutil but then runtime linking it with another (older)
one would lead to crashes. Clement's idea prevents that, but leads to loss
of information. (I'm not saying the current approach of returning a
potentially out-of-range value and expecting the end user to check/clip is
any better, since some are likely to forget leading to crashes, but we need
to think this through and make behaviour of the above code consistent with
whatever we come up with.)
Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
