[FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table

[mypopy at gmail.com]

On Tue, Oct 30, 2018 at 10:41 AM Li, Zhong <zhong.li at intel.com> wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> > Of mypopy at gmail.com
> > Sent: Tuesday, October 30, 2018 9:02 AM
> > To: FFmpeg development discussions and patches
> > <ffmpeg-devel at ffmpeg.org>
> > Cc: Zhao, Jun <jun.zhao at intel.com>; Lin, Decai <decai.lin at intel.com>
> > Subject: Re: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > decoding uninitialized huffman table
> >
> > On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong <zhong.li at intel.com> wrote:
> > >
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> > > > Behalf Of Jun Zhao
> > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > To: ffmpeg-devel at ffmpeg.org
> > > > Cc: Zhao, Jun <jun.zhao at intel.com>; Lin, Decai <decai.lin at intel.com>
> > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > decoding uninitialized huffman table
> > > >
> > > > From: Jun Zhao <jun.zhao at 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.lin at intel.com>
> > > > Signed-off-by: Jun Zhao <jun.zhao at 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)
> > > > --
> > > > 1.7.1
> > >
> > > Should this be fixed in iHD driver instead of ffmpeg?
> > No, I don't think driver is a good place to initialize the huffman table.
>
> For the default Huffman table, thus should be initialized by driver itself.
> For non-default case, thus should be passed from ffmpeg.
> So, what is the case you want to fix? Both (if so, no need to mention iHD driver)?
Both, now HWAccel MJPEG always setting the huffman table and
can't distinguish the HWaccel JPEG whether default Huffman table.