[FFmpeg-devel] [PATCH] H264 DXVA2 implementation

Ramiro Polla ramiro.polla
Sun Jan 10 17:19:05 CET 2010


On Sun, Jan 10, 2010 at 1:54 PM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Sun, Jan 10, 2010 at 01:45:17PM -0200, Ramiro Polla wrote:
>> On Sun, Jan 10, 2010 at 1:22 PM, Reimar D?ffinger
>> <Reimar.Doeffinger at gmx.de> wrote:
>> > On Sun, Jan 10, 2010 at 04:13:43PM +0100, Reimar D?ffinger wrote:
>> >> On Sun, Jan 10, 2010 at 04:08:50PM +0100, Jean-Baptiste Kempf wrote:
>> >> > On Sun, Jan 10, 2010 at 03:01:25PM +0000, M?ns Rullg?rd wrote :
>> >> > > >> > j-b pointed me to the vlc buildbot where this doesn't compile under
>> >> > > >> > "-std=c99" (which is set by default in FFmpeg). I'm no C guru and I'm
>> >> > > >> > quite tired right now, but it seems you can't use anonymous unions in
>> >> > > >> > your dxva2api.h, such as:
>> >> > > >> >
>> >> > > >> > typedef struct {
>> >> > > >> > ? ? union {
>> >> > > >> > ? ? ? ? struct {
>> >> > > >> > ? ? ? ? ? ? UCHAR Index7Bits ? ? : 7;
>> >> > > >> > ? ? ? ? ? ? UCHAR AssociatedFlag : 1;
>> >> > > >> > ? ? ? ? };
>> >> > > >> > ? ? ? ? UCHAR bPicEntry;
>> >> > > >> > ? ? };
>> >> > > >> > } DXVA_PicEntry_H264;
>> >> > > >>
>> >> > > >> WTF? ?How would you even begin to access those fields?
>> >> > > >
>> >> > > > It's a Microsoft/GNU extension.
>> >> > > > DXVA_PicEntry_H264.Index7Bits
>> >> > > > accesses right into the union/struct
>> >> > >
>> >> > > What's the point of that? ?Why not put them directly in the outer
>> >> > > struct if you don't want a name?
>> >> >
>> >> > Because that is how it is done in the Microsoft header Dxva2api.h.
>> >> > See:
>> >> > http://download.microsoft.com/download/5/f/c/5fc4ec5c-bd8c-4624-8034-319c1bab7671/DXVA_H264.pdf
>> >> > page 16, chapter 3.1 for reference.
>> >> >
>> >> > Our dxva2api.h tries to stay compatible with Microsoft headers like the
>> >> > other DirectX headers from Mingw/wine.
>> >>
>> >> That doesn't fully excuse sending a patch for review that seems to not even compile...
>> >
>> > And to say something constructive: I suggest adding check for a define that if set
>> > will remove the non-portable bitfields and (if any remain) name the remaining unions/structs
>>
>> Or we could set -fms-extensions for that file, see
>> http://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
>
> Which of course means the requirements for FFmpeg change from "requires a C99 compatible compiler" to
> "requires a compiler compatible to some bastardized mix of C99 and whatever Microsoft calls C", not to
> mention that you'd probably also need -mms-bitfields.

Assuming that .c file does compile with the microsoft provided
dxva2api.h, and the microsoft provided dxva2api.h also has this
anonymous union, this seems to me like the proper way to be compatible
with the API (by the way I said "for that file", not for the whole of
FFmpeg). Or are you suggesting the videolan file should create its own
new API?

> And with all that compiler checks to make sure we don't mess up too much if any of these are not available.

Checking for a linker flag is one line in configure IIRC...



More information about the ffmpeg-devel mailing list