[FFmpeg-devel] [PATCH 2/2] avcodec/h264: Check weight values to be within the specs limits.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 10:32:15PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Thu, Apr 6, 2017 at 8:10 PM, Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > On Wed, Mar 22, 2017 at 07:09:23PM +0100, Michael Niedermayer wrote:
> > > On Wed, Mar 22, 2017 at 09:06:09AM -0400, Ronald S. Bultje wrote:
> > > > Hi,
> > > >
> > > > On Tue, Mar 21, 2017 at 9:59 PM, Michael Niedermayer
> > <michael at niedermayer.cc
> > > > > wrote:
> > > >
> > > > > @@ -59,6 +59,9 @@ int ff_h264_pred_weight_table(GetBitContext *gb,
> > const
> > > > > SPS *sps,
> > > > >              if (luma_weight_flag) {
> > > > >                  pwt->luma_weight[i][list][0] = get_se_golomb(gb);
> > > > >                  pwt->luma_weight[i][list][1] = get_se_golomb(gb);
> > > > > +                if (   (int8_t)pwt->luma_weight[i][list][0] !=
> > > > > pwt->luma_weight[i][list][0]
> > > > > +                    || (int8_t)pwt->luma_weight[i][list][1] !=
> > > > > pwt->luma_weight[i][list][1])
> > > > > +                    goto error;
> > > > >
> > > >
> > > > Can we please put || on the line before? h264* and a very limited
> > subset of
> > > > other files in our codebase have always been an exception to the large
> > > > majority of the codebase and it's about time we fix that [1].
> > >
> > > i find putting the operators in the first column more readable
> > > but if its preferred in the last, iam happy to change it.
> > >
> > >
> > > >
> > > > It's interesting (I mean that in a positive way) how you use casting to
> > > > check for the range. It's a little obscure, but it's OK.
> > >
> > > yes, it caused me to pause to but it was the simplest way i saw to do
> > > the check
> > >
> > >
> > > >
> > > > +error:
> > > > > +    avpriv_request_sample(logctx, "Out of range weight\n");
> > > > > +    return AVERROR_INVALIDDATA;
> > > >
> > > >
> > > > Same comment as previously in other, but related, threads: unless
> > there is
> > > > real, demonstrable evidence that this happens in real-world files,
> > this is
> > > > fuzz/corruption only and shouldn't be accompanied by an explicit log
> > > > message. Just return AVERROR_INVALIDDATA directly and remove the
> > goto/error
> > > > message.
> > >
> > > If there is "real, demonstrable evidence that this happens in real-world
> > > files" then we would likely have a sample and not ask for one with
> > > avpriv_request_sample()
> > >
> > > I think its very plausible that a encoder would use a weight that is
> > > outside the range. Printing something does make sense.
> >
> > i will apply this with the label chaned to out_range_weight:
> > unless there are objections
> 
> 
> OK.

applied

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170407/ad84f8f3/attachment.sig>