| Message ID | 20170901123354.26722-1-wachsler@google.com |
|---|---|
| State | Superseded |
| Headers | show |
On 01/09/17 13:33, Mark Wachsler wrote: > When parsing a monochrome file, chroma_log2_weight_denom was used without > being initialized, which could lead to a bogus error message being printed, e.g. > [h264 @ 0x61a000026480] chroma_log2_weight_denom 24576 is out of range > It also could result in warnings using AddressSanitizer. > --- > libavcodec/h264_parse.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) Perhaps fix the following uninitialised reference further down the function as well, as in <http://ffmpeg.org/pipermail/ffmpeg-devel/2017-June/212260.html>? I also prefer the erroring out on invalid values rather than picking a random value and decoding garbage, but that might be separate. - Mark > diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c > index 3d20075f6a..d9a3a0c154 100644 > --- a/libavcodec/h264_parse.c > +++ b/libavcodec/h264_parse.c > @@ -34,21 +34,22 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS *sps, > > pwt->use_weight = 0; > pwt->use_weight_chroma = 0; > - pwt->luma_log2_weight_denom = get_ue_golomb(gb); > - if (sps->chroma_format_idc) > - pwt->chroma_log2_weight_denom = get_ue_golomb(gb); > > + pwt->luma_log2_weight_denom = get_ue_golomb(gb); > if (pwt->luma_log2_weight_denom > 7U) { > av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom %d is out of range\n", pwt->luma_log2_weight_denom); > pwt->luma_log2_weight_denom = 0; > } > - if (pwt->chroma_log2_weight_denom > 7U) { > - av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out of range\n", pwt->chroma_log2_weight_denom); > - pwt->chroma_log2_weight_denom = 0; > - } > + luma_def = 1 << pwt->luma_log2_weight_denom; > > - luma_def = 1 << pwt->luma_log2_weight_denom; > - chroma_def = 1 << pwt->chroma_log2_weight_denom; > + if (sps->chroma_format_idc) { > + pwt->chroma_log2_weight_denom = get_ue_golomb(gb); > + if (pwt->chroma_log2_weight_denom > 7U) { > + av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out of range\n", pwt->chroma_log2_weight_denom); > + pwt->chroma_log2_weight_denom = 0; > + } > + chroma_def = 1 << pwt->chroma_log2_weight_denom; > + } > > for (list = 0; list < 2; list++) { > pwt->luma_weight_flag[list] = 0; >
> Am 01.09.2017 um 15:56 schrieb Mark Thompson <sw@jkqxz.net>: > I also prefer the erroring out on invalid values That's what -strict strict is there for, by default libavcodec should try as hard as possible to decode. Carl Eugen
diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c index 3d20075f6a..d9a3a0c154 100644 --- a/libavcodec/h264_parse.c +++ b/libavcodec/h264_parse.c @@ -34,21 +34,22 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS *sps, pwt->use_weight = 0; pwt->use_weight_chroma = 0; - pwt->luma_log2_weight_denom = get_ue_golomb(gb); - if (sps->chroma_format_idc) - pwt->chroma_log2_weight_denom = get_ue_golomb(gb); + pwt->luma_log2_weight_denom = get_ue_golomb(gb); if (pwt->luma_log2_weight_denom > 7U) { av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom %d is out of range\n", pwt->luma_log2_weight_denom); pwt->luma_log2_weight_denom = 0; } - if (pwt->chroma_log2_weight_denom > 7U) { - av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out of range\n", pwt->chroma_log2_weight_denom); - pwt->chroma_log2_weight_denom = 0; - } + luma_def = 1 << pwt->luma_log2_weight_denom; - luma_def = 1 << pwt->luma_log2_weight_denom; - chroma_def = 1 << pwt->chroma_log2_weight_denom; + if (sps->chroma_format_idc) { + pwt->chroma_log2_weight_denom = get_ue_golomb(gb); + if (pwt->chroma_log2_weight_denom > 7U) { + av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out of range\n", pwt->chroma_log2_weight_denom); + pwt->chroma_log2_weight_denom = 0; + } + chroma_def = 1 << pwt->chroma_log2_weight_denom; + } for (list = 0; list < 2; list++) { pwt->luma_weight_flag[list] = 0;