Thanks for Gwenole and Xiaowei's points.
It looks like the simplest way to fix this issue is that we need follow
driver's behavior and make changes in gst-vaapi/codecparser.
There will be 2 options to fix this bug.
Option 1:
do 3 step-conversions on decoding, need gst-vaapi and codec parser
change.
a. codec-parser: Fix mpeg video parser bugs Xiaowei pointed, change
the default zigzag order table to scanning order(replace intra Q-tables to
6.3.11 of ISO/IEC 13818-2 : 2000)
b. gst-vaapi: Do an conversion from zigzag to scanning order (both
mpeg2 and mpeg4)
Option 2:
Only change codecparser: remove the conversion from zigzag to scanning
order on explicit quantization tables(no change needed on default Q-table).
Just like the patch Cong post in
https://bugs.freedesktop.org/show_bug.cgi?id=58176
I'd prefer option 2. How about your idea? BTW Jpeg also used the zigzag table
passed through to driver.
Thanks,
Wind
> -----Original Message-----
> From: Gwenole Beauchesne [mailto:[email protected]]
> Sent: Thursday, January 31, 2013 1:37 PM
> To: Yuan, Feng
> Cc: Li, Jocelyn; Beauchesne, Gwenole; Xiang, Haihao;
> [email protected]; Zhong, CongX; Li, Xiaowei A
> Subject: Re: [Libva] [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed
> (MPEG2)
>
> Hi,
>
> 2013/1/31 Yuan, Feng <[email protected]>:
>
> > Question is how to fill <xxx_intra_quantiser_matrix>, is it
> > zigzag ordered or scan ordered? Mpeg2 data store the quantization
> > table in zigzag order.
>
> If the API is undefined, we have to cope with the driver that has the least
> chances to get updated, and that does depend on explicit quantization
> matrices. Define this as the expected order. Repeat and constrain this with
> all supported codecs to make a consistent behavior. So, if we chose scan
> order for MPEG-2, this also has to be the case for MPEG-4, etc.
>
> Then, the second element to consider is to carry on the less operations
> possible. So, based on your comments, the intel-driver does perform a
> conversion too. However, there are two similar conversions happening in
> FFmpeg too. So, in the end, there are 3 conversion steps!
> Two of them are most likely useless.
>
> > Ffmpeg would fill the quantiser_matrix by zigzag order, do we
> > need to change gstreamer-codec-parse to read data into zigzag order?
> > Does any other package/lib already using
> > gstreamer-codec-parser(mpeg2)? Which package should be the right role
> to do the conversion from zigzag to scanning order?
>
> For performance reasons, only one conversion should happen. So, FFmpeg
> and intel-driver would need to change.
>
> For compatibility reasons, we have to continue with any bad choices we
> made before. So, GStreamer (codecparser or gstreamer-vaapi) would need
> to change. Xiaowei also pointed out another issue with default matrices. I
> didn't have the time to look into it.
>
> Please check the expected behavior against the EMGD driver for example.
> Based on the OSS PVR driver, it seems that we also perform a conversion in
> there. So, we may end up to still use the "triple conversions" (parser /
> codec / driver) scenario...
>
> Regards,
> Gwenole.
>
> > From: Li, Jocelyn
> > Sent: Thursday, January 31, 2013 10:24 AM
> > To: Beauchesne, Gwenole; Xiang, Haihao
> > Cc: Zhao, Halley; Yuan, Feng; Zhong, CongX
> > Subject: FW: [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed
> > (MPEG2)
> >
> >
> >
> > Hi Gwenole and Haihao,
> >
> >
> >
> > We need your comments on the solution to fix this issue.
> >
> >
> >
> > Thanks,
> >
> > Jocelyn
> >
> >
> >
> > From: [email protected]
> > [mailto:[email protected]]
> > Sent: Wednesday, January 30, 2013 5:19 PM
> > To: Li, Jocelyn
> > Subject: [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed
> > (MPEG2)
> >
> >
> >
> > Comment # 5 on bug 58176 from Zhong Cong
> >
> > Created attachment 73914 [details] [review]
> >
> > This patch remove inverse zigzag in gstvaapi codecparser
> >
> >
> >
> > gstvaapi codecparse handles the data with inverse zigzag, and the
> > intel-driver
> >
> > alse asks for original data to inverse zigzag. Here comes the
> >
> > contradiction.This patch remove inverse zigzag in gstvaapi
> > codecparser,and it
> >
> > can solve this issue.
> >
> > ________________________________
> >
> > You are receiving this mail because:
> >
> > You are watching the assignee of the bug.
> >
> >
> > _______________________________________________
> > Libva mailing list
> > [email protected]
> > http://lists.freedesktop.org/mailman/listinfo/libva
> >
--- Begin Message ---
Yes, we need get aligned about the order type before send to driver. Also, I
think there is another issue in gstreamer-codec-parser, not all case we get
the converted scanning order , please review the following code in
“gst-libs/gst/codecparsers/gstmpegvideoparser.c”, If no matrix contained in
bitstream, default zig-zag matrix will be loaded. In two case , we get
different types of order, I think we need resolve this issue.
/* default intra quant matrix, in zig-zag order */
static const guint8 default_intra_quantizer_matrix[64] = {
8,
16, 16,
19, 16, 19,
22, 22, 22, 22,
22, 22, 26, 24, 26,
27, 27, 27, 26, 26, 26,
26, 27, 27, 27, 29, 29, 29,
34, 34, 34, 29, 29, 29, 27, 27,
29, 29, 32, 32, 34, 34, 37,
38, 37, 35, 35, 34, 35,
38, 38, 40, 40, 40,
48, 48, 46, 46,
56, 56, 58,
69, 69,
83
};
/* load_intra_quantiser_matrix */
READ_UINT8 (&br, load_intra_flag, 1);
if (load_intra_flag) {
gint i;
for (i = 0; i < 64; i++)
READ_UINT8 (&br, seqhdr->intra_quantizer_matrix[mpeg_zigzag_8x8[i]], 8);
//case 1. Here seqhdr->intra_quantizer_matrix is filled with scan order matrix
} else
memcpy (seqhdr->intra_quantizer_matrix, default_intra_quantizer_matrix,
64); //case 2. Here seqhdr->intra_quantizer_matrix is filled with zigzaged
order matrix
Thanks,
Xiaowei
From: Li, Jocelyn
Sent: Thursday, January 31, 2013 12:28 PM
To: Yuan, Feng; Beauchesne, Gwenole; Xiang, Haihao; Li, Xiaowei A
Cc: Zhao, Halley; Zhong, CongX; [email protected]
Subject: RE: [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed (MPEG2)
Added Xiaowei.
From: Yuan, Feng
Sent: Thursday, January 31, 2013 11:09 AM
To: Li, Jocelyn; Beauchesne, Gwenole; Xiang, Haihao
Cc: Zhao, Halley; Zhong, CongX;
[email protected]<mailto:[email protected]>
Subject: RE: [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed (MPEG2)
Copy to libva maillist, it’s more about
gstreamer-codec-parser/gst-vaapi/libva/driver related issues.
Hi all,
It’s an issue about how to process the zigzagged quantization table
between gstreamer level and driver level.
Libva defined API of
typedef struct _VAIQMatrixBufferMPEG2
{
int load_intra_quantiser_matrix;
int load_non_intra_quantiser_matrix;
int load_chroma_intra_quantiser_matrix;
int load_chroma_non_intra_quantiser_matrix;
unsigned char intra_quantiser_matrix[64];
unsigned char non_intra_quantiser_matrix[64];
unsigned char chroma_intra_quantiser_matrix[64];
unsigned char chroma_non_intra_quantiser_matrix[64];
} VAIQMatrixBufferMPEG2;
Question is how to fill <xxx_intra_quantiser_matrix>, is it zigzag
ordered or scan ordered? Mpeg2 data store the quantization table in zigzag
order. Currently gstreamer-codec-parser would parse all zigzagged quantization
table from MPEG2 data and converting into scanning order. Intel-Driver would
do the same thing again. The result certainly would be incorrect.
Ffmpeg would fill the quantiser_matrix by zigzag order, do we need to
change gstreamer-codec-parse to read data into zigzag order? Does any other
package/lib already using gstreamer-codec-parser(mpeg2)? Which package should
be the right role to do the conversion from zigzag to scanning order?
Thanks,
Wind
From: Li, Jocelyn
Sent: Thursday, January 31, 2013 10:24 AM
To: Beauchesne, Gwenole; Xiang, Haihao
Cc: Zhao, Halley; Yuan, Feng; Zhong, CongX
Subject: FW: [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed (MPEG2)
Hi Gwenole and Haihao,
We need your comments on the solution to fix this issue.
Thanks,
Jocelyn
From: [email protected]<mailto:[email protected]>
[mailto:[email protected]]
Sent: Wednesday, January 30, 2013 5:19 PM
To: Li, Jocelyn
Subject: [Bug 58176] [gst-vaapi] gi4_video.bits decoding failed (MPEG2)
Comment # 5<https://bugs.freedesktop.org/show_bug.cgi?id=58176#c5> on bug
58176<https://bugs.freedesktop.org/show_bug.cgi?id=58176> from Zhong
Cong<mailto:[email protected]>
Created attachment 73914<https://bugs.freedesktop.org/attachment.cgi?id=73914>
[details]<https://bugs.freedesktop.org/attachment.cgi?id=73914&action=edit>
[review]<https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=58176&attachment=73914>
This patch remove inverse zigzag in gstvaapi codecparser
gstvaapi codecparse handles the data with inverse zigzag, and the intel-driver
alse asks for original data to inverse zigzag. Here comes the
contradiction.This patch remove inverse zigzag in gstvaapi codecparser,and it
can solve this issue.
_____
You are receiving this mail because:
* You are watching the assignee of the bug.
--- End Message ---
_______________________________________________
Libva mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/libva
