These may be used by hwaccel decoders when the standard tables are not otherwise available. At the same time, clean up that code into an array so it's a little less repetitive. --- On 29/10/18 10:26, Jun Zhao wrote: > From: Jun Zhao <jun.z...@intel.com> > > Now VA-API HWAccel MJPEG decoding uninitialized huffman table, it's > will lead to the decoding error like"Failed to sync surface 0x5: 23 > (internal decoding error)." in iHD open source driver. > > Signed-off-by: dlin2 <decai....@intel.com> > Signed-off-by: Jun Zhao <jun.z...@intel.com> > --- > libavcodec/mjpegdec.c | 19 +++++++++++++++++++ > 1 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c > index b0cb3ff..89effb6 100644 > --- a/libavcodec/mjpegdec.c > +++ b/libavcodec/mjpegdec.c > @@ -75,6 +75,25 @@ static int build_vlc(VLC *vlc, const uint8_t *bits_table, > static int build_basic_mjpeg_vlc(MJpegDecodeContext *s) > { > int ret; > + int i; > + > + /* initialize default huffman tables */ > + for (i = 0; i < 16; i++) > + s->raw_huffman_lengths[0][0][i] = avpriv_mjpeg_bits_dc_luminance[i + > 1]; > + for (i = 0; i < 12; i++) > + s->raw_huffman_values[0][0][i] = avpriv_mjpeg_val_dc[i]; > + for (i = 0; i < 16; i++) > + s->raw_huffman_lengths[0][1][i] = avpriv_mjpeg_bits_dc_chrominance[i > + 1]; > + for (i = 0; i < 12; i++) > + s->raw_huffman_values[0][1][i] = avpriv_mjpeg_val_dc[i]; > + for (i = 0; i < 16; i++) > + s->raw_huffman_lengths[1][0][i] = avpriv_mjpeg_bits_ac_luminance[i + > 1]; > + for (i = 0; i < 162; i++) > + s->raw_huffman_values[1][0][i] = avpriv_mjpeg_val_ac_luminance[i]; > + for (i = 0; i < 16; i++) > + s->raw_huffman_lengths[1][1][i] = avpriv_mjpeg_bits_ac_chrominance[i > + 1]; > + for (i = 0; i < 162; i++) > + s->raw_huffman_values[1][1][i] = avpriv_mjpeg_val_ac_chrominance[i]; > > if ((ret = build_vlc(&s->vlcs[0][0], avpriv_mjpeg_bits_dc_luminance, > avpriv_mjpeg_val_dc, 12, 0, 0)) < 0) >
Seems reasonable, but perhaps the enclosed patch instead makes it clearer what is going on and easier to verify correctness? (Even better: the builtin tables would be in DHT form and we would just call decode_dht() on them and avoid messing around with separate code, but since they are used in multiple places that looks more inconvenient to arrange.) - Mark libavcodec/mjpegdec.c | 67 +++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 96c425515a..2f1635838a 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -73,34 +73,45 @@ static int build_vlc(VLC *vlc, const uint8_t *bits_table, huff_code, 2, 2, huff_sym, 2, 2, use_static); } -static int build_basic_mjpeg_vlc(MJpegDecodeContext *s) +static int init_default_huffman_tables(MJpegDecodeContext *s) { - int ret; - - if ((ret = build_vlc(&s->vlcs[0][0], avpriv_mjpeg_bits_dc_luminance, - avpriv_mjpeg_val_dc, 12, 0, 0)) < 0) - return ret; - - if ((ret = build_vlc(&s->vlcs[0][1], avpriv_mjpeg_bits_dc_chrominance, - avpriv_mjpeg_val_dc, 12, 0, 0)) < 0) - return ret; - - if ((ret = build_vlc(&s->vlcs[1][0], avpriv_mjpeg_bits_ac_luminance, - avpriv_mjpeg_val_ac_luminance, 251, 0, 1)) < 0) - return ret; - - if ((ret = build_vlc(&s->vlcs[1][1], avpriv_mjpeg_bits_ac_chrominance, - avpriv_mjpeg_val_ac_chrominance, 251, 0, 1)) < 0) - return ret; - - if ((ret = build_vlc(&s->vlcs[2][0], avpriv_mjpeg_bits_ac_luminance, - avpriv_mjpeg_val_ac_luminance, 251, 0, 0)) < 0) - return ret; - - if ((ret = build_vlc(&s->vlcs[2][1], avpriv_mjpeg_bits_ac_chrominance, - avpriv_mjpeg_val_ac_chrominance, 251, 0, 0)) < 0) - return ret; + static const struct { + int class; + int index; + const uint8_t *bits; + const uint8_t *values; + int codes; + int length; + } ht[] = { + { 0, 0, avpriv_mjpeg_bits_dc_luminance, + avpriv_mjpeg_val_dc, 12, 12 }, + { 0, 1, avpriv_mjpeg_bits_dc_chrominance, + avpriv_mjpeg_val_dc, 12, 12 }, + { 1, 0, avpriv_mjpeg_bits_ac_luminance, + avpriv_mjpeg_val_ac_luminance, 251, 162 }, + { 1, 1, avpriv_mjpeg_bits_ac_chrominance, + avpriv_mjpeg_val_ac_chrominance, 251, 162 }, + { 2, 0, avpriv_mjpeg_bits_ac_luminance, + avpriv_mjpeg_val_ac_luminance, 251, 162 }, + { 2, 1, avpriv_mjpeg_bits_ac_chrominance, + avpriv_mjpeg_val_ac_chrominance, 251, 162 }, + }; + int i, ret; + + for (i = 0; i < FF_ARRAY_ELEMS(ht); i++) { + ret = build_vlc(&s->vlcs[ht[i].class][ht[i].index], + ht[i].bits, ht[i].values, ht[i].codes, + 0, ht[i].class == 1); + if (ret < 0) + return ret; + if (ht[i].class < 2) { + memcpy(s->raw_huffman_lengths[ht[i].class][ht[i].index], + ht[i].bits + 1, 16); + memcpy(s->raw_huffman_values[ht[i].class][ht[i].index], + ht[i].values, ht[i].length); + } + } return 0; } @@ -151,7 +162,7 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx) avctx->colorspace = AVCOL_SPC_BT470BG; s->hwaccel_pix_fmt = s->hwaccel_sw_pix_fmt = AV_PIX_FMT_NONE; - if ((ret = build_basic_mjpeg_vlc(s)) < 0) + if ((ret = init_default_huffman_tables(s)) < 0) return ret; if (s->extern_huff) { @@ -161,7 +172,7 @@ av_cold int ff_mjpeg_decode_init(AVCodecContext *avctx) if (ff_mjpeg_decode_dht(s)) { av_log(avctx, AV_LOG_ERROR, "error using external huffman table, switching back to internal\n"); - build_basic_mjpeg_vlc(s); + init_default_huffman_tables(s); } } if (avctx->field_order == AV_FIELD_BB) { /* quicktime icefloe 019 */ -- 2.19.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel