[FFmpeg-devel] [RFC][PATCH 0/3] VC-1 HW Accel field interlaced decoding

[Gwenole Beauchesne]

Hi,

2012/8/24 Michael Niedermayer <michaelni at gmx.at>:
> On Mon, Aug 20, 2012 at 06:18:13PM +0200, Hendrik Leppkes wrote:
>> On Mon, Aug 20, 2012 at 5:27 PM, Gwenole Beauchesne <gb.devel at gmail.com>wrote:
>>
>> > Hi,
>> >
>> > 2012/8/19 Hendrik Leppkes <h.leppkes at gmail.com>:
>> >
>> > > this patchset adds support for HW accel decoding of VC-1 field
>> > interlaced pictures.
>> > > It is tested using the DXVA2 hwaccel implementation only, because i do
>> > not have a system setup for VA-API (the only other VC1 HW accel).
>> >
>> > This patch series does not work with VA-API and I am even surprised it
>> > works with DXVA.
>> >
>>
>> I actually compared the calls to the accelerator with other closed-source
>> DXVA decoders, and they match this functionality.
>> I don't see whats so surprising about it, the two fields are simply being
>> submitted seperately to the HW decoder.
>>
>>
>> >
>> > > The first two patches are simple preparations and essentially usable
>> > stand-alone without any (known) issues.
>> > >
>> > > Patch 1 disables the full picture header parsing when using a hw accel,
>> > because it is not required.
>> > > The HW accelerator will perform this parsing itself, and none of the
>> > values parsed from the bistream are passed to the HW accelerator. In
>> > addition, VC-1 interlaced parsing is known to not be completely tested, so
>> > disabling it in a code path where its not required avoids potential bugs.
>> >
>> > The HW accelerator won't always parse the headers itself. Besides,
>> > even for DXVA2, you need to correctly fill in wMBbitOffset. If you
>> > stop the parsing earlier, get_bits_count() won't return the expected
>> > value. So, wMBbitOffset will get wrong and some HW accelerators may
>> > fail. Some other bits are also needed for DXVA2 like MV mode types,
>> > etc.
>>
>>
>> DXVA2 APIs may have fields for this, however neither NVIDIA or AMD seem to
>> care.
>> It works just beautifully on cards from both vendors the way it is.
>>
>> I personally don't care either way, if the parsing should be performed, go
>> for it.
>> Note that the wMBbitOffset will have to be handled seperately for the
>> second field anyway then, because the bit parser is not advanced through
>> the frame.
>>
>> Anyway, like i mentioned, its a RFC. All i know is that it works perfectly
>> on both AMD and NVIDIA hardware. I briefly tested Intel, and i think they
>
> i think if it works perfectly then its better to apply and improve
> the code instead of waiting a long time because its not filling all
> API fields

As I mentioned, this patch series breaks VA-API support, even for non
interlaced contents.

Besides, with this patch, the DXVA2 API fields are still filled in,
but with incorrect values. It's reported to work on AMD or NVIDIA but
they assume buggy software, hence the workarounds in the driver to
parse the bitstream instead of trusting the SW input. I don't really
want FFmpeg to be a reason for driver developers to implement
workarounds to client application bugs. Rather, let's better make
FFmpeg an exemplary implementation.

And, anyway, why not actually try to fix the VC-1 bitstream parser? It
would also benefit the non HW accelerated case.

Alternatively, the better thing that could be done may be to check for
DXVA2 hwaccel only prior to enabling "parse_only" mode.

Thanks,
Gwenole.