[Mesa-dev] [PATCH] vl/mpeg12: implement inverse scan/quantization steps

[Maarten Lankhorst]

Op 26-06-13 10:33, Christian König schreef:
> Am 26.06.2013 05:29, schrieb Ilia Mirkin:
>> On Mon, Jun 24, 2013 at 2:13 PM, Christian König
>> <deathsimple at vodafone.de> wrote:
>>> Am 24.06.2013 18:39, schrieb Ilia Mirkin:
>>>
>>>> On Mon, Jun 24, 2013 at 4:48 AM, Christian König
>>>> <deathsimple at vodafone.de> wrote:
>>>>> Am 23.06.2013 18:59, schrieb Ilia Mirkin:
>>>>>
>>>>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>>>>> ---
>>>>>>
>>>>>> These changes make MPEG2 I-frames generate the correct macroblock data
>>>>>> (as
>>>>>> compared to mplayer via xvmc). Other MPEG2 frames are still misparsed,
>>>>>> and
>>>>>> MPEG1 I-frames have some errors (but largely match up).
>>>>>
>>>>> NAK, zscan and mismatch handling are handled in vl/vl_zscan.c.
>>>>>
>>>>> Please use/fix that one instead of adding another implementation.
>>>> Yes, I noticed these after Andy pointed out that my patch broke things
>>>> for him. Here's my situation, perhaps you can advise on how to
>>>> proceed:
>>>>
>>>> NVIDIA VP2 hardware (NV84-NV96, NVA0) doesn't do bitstream parsing,
>>>> but it can take the macroblocks and render them. When I use my
>>>> implementation with xvmc, everything works fine. If I try to use vdpau
>>>> by using vl_mpeg12_bitstream to parse the bitstream, the data comes
>>>> out all wrong. It appears that decode_macroblock is called with data
>>>> before inverse z-scan and quantization, while mplayer pushes data to
>>>> xvmc after those steps. So should I basically have a bit of logic in
>>>> my decode_macroblock impl that says "if using mpeg12_bitstream then do
>>>> some more work on this data"? Or what data should decode_macroblock
>>>> expect to receive?
>>>
>>> Yes exactly, for the bitstream case decode_macroblock gets the blocks in
>>> original zscan order without mismatch correction or quantification.
>>>
>>> You can either do the missing steps on the gpu with shaders or on the cpu
>>> while uploading the data and use the entrypoint member on the decoder to
>>> distinct between the different usecases.
>> That sounds reasonable. But I'm looking at the MPEG-2 spec, and it
>> goes something like (7.4.5):
>>
>> if ( macroblock_intra ) {
>>    F’’[v][u] = ( QF[v][u] * W[w][v][u] * quantiser_scale * 2 ) / 32;
>> } else {
>>    F’’[v][u] = ( ( ( QF[v][u] * 2 ) + Sign(QF[v][u]) ) * W[w][v][u] *
>> quantiser_scale ) / 32;
>> }
>
> Only the multiplication with W[w][v][u] (and IIRC the divide with 32) isn't done in decode_dct, since everything else depends on the bitstream context.
>
>> However the current vl_mpeg12_bitstream::decode_dct code seems to be
>> returning roughly QF[v][u] * quantiser_scale in mb->blocks. But in the
>> non-intra case it seems like we need the original value of QF[v][u] in
>> order to perform that operation. (I tried looking at vl_zscan, but I
>> don't really understand how shaders work.) Am I misunderstanding?
>> Should the quantiser_scale be passed in via the macroblock structure,
>> and the multiplication left out of the bitstream?
>
> Not exactly, all you need to do is the multiplication with W[w][v][u], division by 32 and application of mismatch control (see section 7.4.4 in the spec). As code it should look something like that:
>
> sum = 0;
> for (i=0; i<64;++i) {
>     value = src[i] * matrix[i] / 32;
>     if ( value > 2047 ) {
>         value = 2047;
>     } else if ( value < -2048 ) {
>         value = -2048;
>     }
>         sum += value;
>         dst[zscan[i]] = value;
> }
> if ((sum & 1) == 0) {
>     if ((dst[63] & 1) != 0) {
>         dst[63] -= 1;
>     } else {
>         dst[63] += 1;
>     }
> }
>
> I never really tested MPEG1, so it's highly likely that there are still MPEG1 cases that aren't handled in the bitstream decoder.
Well I did. :D

I fixed up the actual bitstream parsing to be more correct for mpeg-1, except for D-frames.
But it's still lacking some cases. Commit 6aabd9490c7b71d363a384935597b3e37295e61e
lists exactly what's missing atm.

For the slices not having to end on the same line support I was considering this patch:

diff --git a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
index b0fb1bb..99178b4 100644
--- a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
+++ b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
@@ -849,6 +849,11 @@ decode_slice(struct vl_mpg12_bs *bs, struct pipe_video_buffer *target)
          bs->decoder->decode_macroblock(bs->decoder, target, &bs->desc->base, &mb.base, 1);
       }
       mb.x = x += inc;
+      if (bs->decoder->profile == PIPE_VIDEO_PROFILE_MPEG1) {
+         int width = align(bs->decoder->width, 16) / 16;
+         mb.y += mb.x / width;
+         mb.x = x %= width;
+      }
 
       switch (bs->desc->picture_coding_type) {
       case PIPE_MPEG12_PICTURE_CODING_TYPE_I: