Hi,

On Sat, Nov 26, 2011 at 10:02 AM, Mashiat Sarker Shakkhar
<[email protected]> wrote:
> ----- Original Message -----
>> From: Mashiat Sarker Shakkhar <[email protected]>
>> To: libav development <[email protected]>
>> Cc:
>> Sent: Saturday, November 26, 2011 11:22 PM
>> Subject: Re: [libav-devel] [PATCH] vc1dec: fix scantable for advanced P 
>> frames
>>
>> ----- Original Message -----
>>
>>>  From: Kostya Shishkov <[email protected]>
>>>  To: libav development <[email protected]>
>>>  Cc:
>>>  Sent: Saturday, November 26, 2011 6:28 PM
>>>  Subject: Re: [libav-devel] [PATCH] vc1dec: fix scantable for advanced P
>> frames
>>>
>>>  On Sat, Nov 26, 2011 at 01:11:49PM +0100, Kostya Shishkov wrote:
>>>>   On Sat, Nov 26, 2011 at 01:02:43PM +0100, Luca Barbato wrote:
>>>>   > On 26/11/11 12:25, Kostya Shishkov wrote:
>>>>   > >On Sat, Nov 26, 2011 at 11:04:10AM +0100, Luca Barbato wrote:
>>>>   > >>On 26/11/11 06:31, Mashiat Sarker Shakkhar wrote:
>>>>   > >>>From: Michael Niedermayer<[email protected]>
>>>>   > >>>
>>>>   > >>>Fixes: vc1 file from Ticket606
>>>>   > >>>Fixes: vc1+vc1+++artifacts*.vc1
>>>>   > >>>Fixes: mpeg+vc1+++salxxos.evo
>>>>   > >>
>>
>> Michael's original version breaks samples that were decoding fine before.
>> For example SA10143.
>>
>>>>   > >>fcm; ///<  0->Progressive, 2->Frame-Interlace,
>>>  3->Field-Interlace
>>>>   > >>
>>>>   > >>So Use the same scantable for Progressive and
>> Field-Interlace
>>>  ?
>>>>   > >
>>>>   > >For us FCM is 0, 1 and 2. And scantables for interlaced mode
>>>  should be used
>>>>   > >(it seems from the standard unless I got it wrong) for both
>> FCM =
>>>  1 or 2.
>>>>   >
>>>>   > libavcodec/dxva2_vc1.c seems to use it
>>>>   >
>>>>   > pp->bSecondField            = v->interlace &&
>> v->fcm
>>>  != 0x03 &&
>>>>   > !s->first_field;
>>>>
>>>>   it's incorrect then
>>>>   in vc1.c
>>>>   v->fcm = decode012(gb);
>>>>   it's a bit silly to address it by binary code anyway
>>>>
>>>>   > >Though the old approach was wrong too - there are enough
>> files
>>>  with interlaced
>>>>   > >mode = 1 and FCM=0 (HD-DVD featured that, for example).
>>>>   >
>>>>   > uhm so the patch isn't right completely...
>>>>
>>>>   yes
>>>
>>>  So here's my variant. I'll wait till Mashiat tests it though.
>>>
>>
>> While Kostya's patch fixes samples that are mentioned in the
>> commit message without breaking any other sample, I have my
>> doubts about it. Of course it is not wrong, but iiuc it's incomplete.
>>
>> To briefly outline what the spec says (My copy is dated
>> 24-February-2006) :
>>
>> * Simple, Main and Adv. Prog. share 8x8 and 4x4 tables
>> * There are separate 8x8 and 4x4 tables for Adv. Interlaced
>> * Simple and Main Profiles share 8x4 and 4x8 tables
>> * Adv. Prog. has it's own 8x4 and 4x8 tables
>> * Adv. Interlaced has it's own 8x4 and 4x8 tables
>>
>> (All the above only applies to Inter zigzag scan)
>>
>> Now v->fcm == 0 can mean 2 things: either the sample has
>> Simple/Main profile _or_ it is Advanced Progressive. So imo,
>> the if (!v->fcm) blocks need a nested if() to check for the profile.
>> That can certainly be a separate patch, though.
>
> ^^^ I apologize for my carelessness, but just now I recalled
> that this selection is done during header parsing and checked
> the code to confirm.
>
> Hence, patch OK (Kostya's version). I apologize again.

Kostya's version pushed.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to