[FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding uninitialized huffman table
mypopy at gmail.com
mypopy at gmail.com
Fri Nov 9 07:44:41 EET 2018
On Fri, Nov 9, 2018 at 1:27 PM Eoff, Ullysses A
<ullysses.a.eoff at intel.com> wrote:
>
> > -----Original Message-----
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of Eoff, Ullysses A
> > Sent: Tuesday, October 30, 2018 11:42 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
> > > -----Original Message-----
> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of mypopy at gmail.com
> > > Sent: Monday, October 29, 2018 10:52 PM
> > > 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 Tue, Oct 30, 2018 at 11:41 AM Eoff, Ullysses A
> > > <ullysses.a.eoff at intel.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of mypopy at gmail.com
> > > > > Sent: Monday, October 29, 2018 8:10 PM
> > > > > 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 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, le < 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.
> > > >
> > > > If your VA-API HWAccel MJPEG decoding case(s) work with i965 driver
> > > > and not iHD then I would assume iHD needs fixing.
> > > >
> > > i965 must be have some issue if MJPEG decoding, when I try to dump the
> > > data to CPU to check the frame
> > >
> > > LIBVA_DRIVER_NAME=i965 ./ffmpeg -hwaccel vaapi -vaapi_device
> > > /dev/dri/renderD128 -i
> > > ~/Videos/logitech_webcam_1280x720_30fps_2980frames.mjpeg -f null
> > > /dev/null
> > > ffmpeg version N-92305-g1ff4bd59df Copyright (c) 2000-2018 the FFmpeg developers
> > > built with gcc 7 (Ubuntu 7.3.0-27ubuntu1~18.04)
> > > configuration: --enable-libx264 --enable-libx265 --enable-gpl
> > > --enable-libwebp --disable-optimizations --samples=../fate-suite
> > > --enable-openssl --enable-nonfree --enable-libzmq --enable-libkvazaar
> > > --enable-libvpx --enable-libvorbis --enable-libsrt
> > > libavutil 56. 21.100 / 56. 21.100
> > > libavcodec 58. 34.100 / 58. 34.100
> > > libavformat 58. 19.102 / 58. 19.102
> > > libavdevice 58. 4.106 / 58. 4.106
> > > libavfilter 7. 38.100 / 7. 38.100
> > > libswscale 5. 2.100 / 5. 2.100
> > > libswresample 3. 2.100 / 3. 2.100
> > > libpostproc 55. 2.100 / 55. 2.100
> > > [mjpeg @ 0x55949b69a1c0] Format mjpeg detected only with low score of
> > > 25, misdetection possible!
> > > Input #0, mjpeg, from
> > > '/home/barry/Videos/logitech_webcam_1280x720_30fps_2980frames.mjpeg':
> > > Duration: N/A, bitrate: N/A
> > > Stream #0:0: Video: mjpeg, yuvj422p(pc, bt470bg/unknown/unknown),
> > > 1280x720, 25 tbr, 1200k tbn, 25 tbc
> > > Stream mapping:
> > > Stream #0:0 -> #0:0 (mjpeg (native) -> wrapped_avframe (native))
> > > Press [q] to stop, [?] for help
> > > [AVHWFramesContext @ 0x55949b6a5240] Failed to read image from surface
> > > 0x4000005: 20 (the requested function is not implemented).
> > > [mjpeg @ 0x55949b69d440] Failed to transfer data to output frame: -5.
> > > Error while processing the decoded data for stream #0:0
> >
> > Hmm, yes i965+ffmpeg fails with yuv422p mjpeg (and gray8). But it seems to handle
> > yuv440p, yuv420p and yuv444p mjpeg reasonably well with the inputs I tested.
> >
> > On the flip-side, iHD+ffmpeg seems to succeed on yuv422p mjpeg, but the output is
> > horribly messed up. iHD+ffmpeg appears to do yuv420p and yuv444p well
> > but fails on yuv440p and gray8 mjpeg.
> >
> > This patch did not seem to change any of these outcomes on my side. Can you
> > share your mjpeg input file that produces the behavior you described in the commit
> > message?
> >
> > Is this a case where the mjpeg file does not include DHT segments in the
> > encoded file? Or is it an abbreviated mjpeg format?
> >
>
> Ok, I analyzed your mjpeg input file. And, indeed, it does not contain any
> DHT segments (encoded huff table definitions) in any frame.
>
> Other mjpeg decoder software I've seen or worked on (e.g. gstreamer-vaapi,
> libyami) typically setup a default Huffman table, based on the JPEG standard,
> if the [m]jpeg file does not provide one. That is, they don't expect the driver
> to do it for them.
>
> I tried a jpeg file that is compatible (i.e. yuv422p format) with both drivers and
> encoded with JPEG standard Huffman table. First, with DHT segments in jpeg
> file, it works good on both drivers. But without DHT segments in the jpeg file,
> neither driver performs correctly. That is, ffmpeg+iHD command fails as you
> described. And ffmpeg+i965 command succeeds, but the output yuv is
> completely wrong. With this patch, it is fixed for both drivers I tested.
>
> So it seems to me that it would be best for middleware to provide default
> Huffman tables to drivers (as this patch does) for maximum compatibility,
> despite previous comments. But I'd like to hear more arguments if anyone
> disagrees.
>
> Regards,
> U. Artie
>
Thanks the detailed and substantial comments and help to double check
this patch in iHD/i965 driver both, U. Artie, let us wait other
comments.
More information about the ffmpeg-devel
mailing list