diff mbox

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

Message ID 1540808785-13289-1-git-send-email-mypopydev@gmail.com
State New
Headers show

Commit Message

Jun Zhao Oct. 29, 2018, 10:26 a.m. UTC
From: Jun Zhao <jun.zhao@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@intel.com>
Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 libavcodec/mjpegdec.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

Comments

Zhong Li Oct. 29, 2018, 10:38 a.m. UTC | #1
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of Jun Zhao

> Sent: Monday, October 29, 2018 6:26 PM

> To: ffmpeg-devel@ffmpeg.org

> Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>

> Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding

> uninitialized huffman table

> 

> From: Jun Zhao <jun.zhao@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@intel.com>

> Signed-off-by: Jun Zhao <jun.zhao@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?
mypopy@gmail.com Oct. 30, 2018, 1:02 a.m. UTC | #2
On Mon, Oct 29, 2018 at 6:39 PM Li, Zhong <zhong.li@intel.com> wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> > Of Jun Zhao
> > Sent: Monday, October 29, 2018 6:26 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>
> > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG decoding
> > uninitialized huffman table
> >
> > From: Jun Zhao <jun.zhao@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@intel.com>
> > Signed-off-by: Jun Zhao <jun.zhao@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.
Carl Eugen Hoyos Oct. 30, 2018, 1:43 a.m. UTC | #3
2018-10-29 11:26 GMT+01:00, Jun Zhao <mypopydev@gmail.com>:
> From: Jun Zhao <jun.zhao@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.

This sounds as if the issue you want to fix is in the vaapi code
but your change affects only general mjpeg code.

Carl Eugen
mypopy@gmail.com Oct. 30, 2018, 2:36 a.m. UTC | #4
On Tue, Oct 30, 2018 at 9:50 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>
> 2018-10-29 11:26 GMT+01:00, Jun Zhao <mypopydev@gmail.com>:
> > From: Jun Zhao <jun.zhao@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.
>
> This sounds as if the issue you want to fix is in the vaapi code
> but your change affects only general mjpeg code.
>

Yes, as you know, HWaccel code is part of software decoder , so
we need to work on the general mjpeg code sometime, and maybe
I need to add a build macro in this case?
Zhong Li Oct. 30, 2018, 2:41 a.m. UTC | #5
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> Of mypopy@gmail.com

> Sent: Tuesday, October 30, 2018 9:02 AM

> To: FFmpeg development discussions and patches

> <ffmpeg-devel@ffmpeg.org>

> Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> >

> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> > > Behalf Of Jun Zhao

> > > Sent: Monday, October 29, 2018 6:26 PM

> > > To: ffmpeg-devel@ffmpeg.org

> > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>

> > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG

> > > decoding uninitialized huffman table

> > >

> > > From: Jun Zhao <jun.zhao@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@intel.com>

> > > Signed-off-by: Jun Zhao <jun.zhao@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)?
mypopy@gmail.com Oct. 30, 2018, 3:10 a.m. UTC | #6
On Tue, Oct 30, 2018 at 10:41 AM Li, Zhong <zhong.li@intel.com> wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> > Of mypopy@gmail.com
> > Sent: Tuesday, October 30, 2018 9:02 AM
> > To: FFmpeg development discussions and patches
> > <ffmpeg-devel@ffmpeg.org>
> > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:
> > >
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On
> > > > Behalf Of Jun Zhao
> > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > To: ffmpeg-devel@ffmpeg.org
> > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>
> > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > decoding uninitialized huffman table
> > > >
> > > > From: Jun Zhao <jun.zhao@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@intel.com>
> > > > Signed-off-by: Jun Zhao <jun.zhao@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.
Eoff, Ullysses A Oct. 30, 2018, 3:41 a.m. UTC | #7
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com

> Sent: Monday, October 29, 2018 8:10 PM

> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> >

> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> > > Of mypopy@gmail.com

> > > Sent: Tuesday, October 30, 2018 9:02 AM

> > > To: FFmpeg development discussions and patches

> > > <ffmpeg-devel@ffmpeg.org>

> > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > >

> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> > > > > Behalf Of Jun Zhao

> > > > > Sent: Monday, October 29, 2018 6:26 PM

> > > > > To: ffmpeg-devel@ffmpeg.org

> > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>

> > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG

> > > > > decoding uninitialized huffman table

> > > > >

> > > > > From: Jun Zhao <jun.zhao@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@intel.com>

> > > > > Signed-off-by: Jun Zhao <jun.zhao@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.


If your VA-API HWAccel MJPEG decoding case(s) work with i965 driver
and not iHD then I would assume iHD needs fixing.

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
mypopy@gmail.com Oct. 30, 2018, 5:52 a.m. UTC | #8
On Tue, Oct 30, 2018 at 11:41 AM Eoff, Ullysses A
<ullysses.a.eoff@intel.com> wrote:
>
> > -----Original Message-----
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com
> > Sent: Monday, October 29, 2018 8:10 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:
> > >
> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> > > > Of mypopy@gmail.com
> > > > Sent: Tuesday, October 30, 2018 9:02 AM
> > > > To: FFmpeg development discussions and patches
> > > > <ffmpeg-devel@ffmpeg.org>
> > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:
> > > > >
> > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On
> > > > > > Behalf Of Jun Zhao
> > > > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>
> > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > > > decoding uninitialized huffman table
> > > > > >
> > > > > > From: Jun Zhao <jun.zhao@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@intel.com>
> > > > > > Signed-off-by: Jun Zhao <jun.zhao@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.
>
> 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
Eoff, Ullysses A Oct. 30, 2018, 6:42 p.m. UTC | #9
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com

> Sent: Monday, October 29, 2018 10:52 PM

> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> >

> > > -----Original Message-----

> > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com

> > > Sent: Monday, October 29, 2018 8:10 PM

> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > >

> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> > > > > Of mypopy@gmail.com

> > > > > Sent: Tuesday, October 30, 2018 9:02 AM

> > > > > To: FFmpeg development discussions and patches

> > > > > <ffmpeg-devel@ffmpeg.org>

> > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > > > >

> > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> > > > > > > Behalf Of Jun Zhao

> > > > > > > Sent: Monday, October 29, 2018 6:26 PM

> > > > > > > To: ffmpeg-devel@ffmpeg.org

> > > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>

> > > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG

> > > > > > > decoding uninitialized huffman table

> > > > > > >

> > > > > > > From: Jun Zhao <jun.zhao@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@intel.com>

> > > > > > > Signed-off-by: Jun Zhao <jun.zhao@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.

> >

> > 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?

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Eoff, Ullysses A Nov. 9, 2018, 5:27 a.m. UTC | #10
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Eoff, Ullysses A

> Sent: Tuesday, October 30, 2018 11:42 AM

> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@ffmpeg.org] On Behalf Of mypopy@gmail.com

> > Sent: Monday, October 29, 2018 10:52 PM

> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > >

> > > > -----Original Message-----

> > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com

> > > > Sent: Monday, October 29, 2018 8:10 PM

> > > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > > >

> > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> > > > > > Of mypopy@gmail.com

> > > > > > Sent: Tuesday, October 30, 2018 9:02 AM

> > > > > > To: FFmpeg development discussions and patches

> > > > > > <ffmpeg-devel@ffmpeg.org>

> > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > > > > >

> > > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> > > > > > > > Behalf Of Jun Zhao

> > > > > > > > Sent: Monday, October 29, 2018 6:26 PM

> > > > > > > > To: ffmpeg-devel@ffmpeg.org

> > > > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>

> > > > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG

> > > > > > > > decoding uninitialized huffman table

> > > > > > > >

> > > > > > > > From: Jun Zhao <jun.zhao@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@intel.com>

> > > > > > > > Signed-off-by: Jun Zhao <jun.zhao@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.

> > >

> > > 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

> > _______________________________________________

> > ffmpeg-devel mailing list

> > ffmpeg-devel@ffmpeg.org

> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Eoff, Ullysses A Nov. 9, 2018, 5:30 a.m. UTC | #11
> -----Original Message-----

> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Eoff, Ullysses A

> Sent: Thursday, November 08, 2018 9:27 PM

> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@ffmpeg.org] On Behalf Of Eoff, Ullysses A

> > Sent: Tuesday, October 30, 2018 11:42 AM

> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@ffmpeg.org] On Behalf Of mypopy@gmail.com

> > > Sent: Monday, October 29, 2018 10:52 PM

> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > >

> > > > > -----Original Message-----

> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com

> > > > > Sent: Monday, October 29, 2018 8:10 PM

> > > > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>

> > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > > > >

> > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf

> > > > > > > Of mypopy@gmail.com

> > > > > > > Sent: Tuesday, October 30, 2018 9:02 AM

> > > > > > > To: FFmpeg development discussions and patches

> > > > > > > <ffmpeg-devel@ffmpeg.org>

> > > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:

> > > > > > > >

> > > > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On

> > > > > > > > > Behalf Of Jun Zhao

> > > > > > > > > Sent: Monday, October 29, 2018 6:26 PM

> > > > > > > > > To: ffmpeg-devel@ffmpeg.org

> > > > > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>

> > > > > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG

> > > > > > > > > decoding uninitialized huffman table

> > > > > > > > >

> > > > > > > > > From: Jun Zhao <jun.zhao@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@intel.com>

> > > > > > > > > Signed-off-by: Jun Zhao <jun.zhao@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.

> > > >

> > > > 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


Sorry, typo... I meant yuv420p format.

> 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

> 

> > > _______________________________________________

> > > ffmpeg-devel mailing list

> > > ffmpeg-devel@ffmpeg.org

> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> > _______________________________________________

> > ffmpeg-devel mailing list

> > ffmpeg-devel@ffmpeg.org

> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
mypopy@gmail.com Nov. 9, 2018, 5:44 a.m. UTC | #12
On Fri, Nov 9, 2018 at 1:27 PM Eoff, Ullysses A
<ullysses.a.eoff@intel.com> wrote:
>
> > -----Original Message-----
> > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of Eoff, Ullysses A
> > Sent: Tuesday, October 30, 2018 11:42 AM
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@ffmpeg.org] On Behalf Of mypopy@gmail.com
> > > Sent: Monday, October 29, 2018 10:52 PM
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf Of mypopy@gmail.com
> > > > > Sent: Monday, October 29, 2018 8:10 PM
> > > > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:
> > > > > >
> > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf
> > > > > > > Of mypopy@gmail.com
> > > > > > > Sent: Tuesday, October 30, 2018 9:02 AM
> > > > > > > To: FFmpeg development discussions and patches
> > > > > > > <ffmpeg-devel@ffmpeg.org>
> > > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@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@intel.com> wrote:
> > > > > > > >
> > > > > > > > > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On
> > > > > > > > > Behalf Of Jun Zhao
> > > > > > > > > Sent: Monday, October 29, 2018 6:26 PM
> > > > > > > > > To: ffmpeg-devel@ffmpeg.org
> > > > > > > > > Cc: Zhao, Jun <jun.zhao@intel.com>; Lin, Decai <decai.lin@intel.com>
> > > > > > > > > Subject: [FFmpeg-devel] [PATCH] lavc/mjpegdec: fix VA-API MJPEG
> > > > > > > > > decoding uninitialized huffman table
> > > > > > > > >
> > > > > > > > > From: Jun Zhao <jun.zhao@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@intel.com>
> > > > > > > > > Signed-off-by: Jun Zhao <jun.zhao@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.
diff mbox

Patch

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)