[FFmpeg-devel] [PATCH] HWAccel infrastructure (take 7.1)

[Gwenole Beauchesne]

Le 24 f?vr. 09 ? 20:13, Reimar D?ffinger a ?crit :

> On Tue, Feb 24, 2009 at 07:43:21PM +0100, Gwenole Beauchesne wrote:
>> Le 24 f?vr. 09 ? 18:08, Reimar D?ffinger a ?crit :
>>
>>> Not using per-codec PIX_FMTs seems more likely to me to just  
>>> complicate
>>> things, though I do not know for sure.
>>
>> It turned out the changes to my local mplayer tree were rather  
>> small. See
>> attachement + adding the extra codec_ids in the table.
>
> I guess there are some changes it conversion_map missing?

Only for HW accelerated formats. If codec_id is 0 (default), then  
nothing more is needed. e.g.

{IMGFMT_VDPAU_MPEG1,     PIX_FMT_VDPAU_MPEG1},
becomes
{IMGFMT_VDPAU_MPEG1,     PIX_FMT_VDPAU, CODEC_ID_MPEG1VIDEO},

>> However, I terribly dislike the pixfmt2imgfmt() name as it is now  
>> implied
>> by the removal of codec information in pixfmt.
>
> Well, IMO while it needs little changes that patch takes the
> "move the mess one step higher" approach.
> One particular disadvantage of that approach is that e.g. you can't  
> see
> if it would still be possible to "misuse" libswscale to convert  
> between
> VDPAU- and YV12-format without API changes.

I don't see how it was possible in "current" (in SVN) code either.

> There are also other questions like if it can cause any issues that
> AVFrame + PIX_FMT is no longer sufficient to interpret the data
> (obviously this can be solved e.g. by added a codec identifier to
> vdpau_render_state).

This would be an interesting addition at the API level too. I also  
have VDP_CODEC_MPEG2 et al. identifiers in my code and the fact you  
need something similar is also a sign this could be a good addition to  
the API. But back to your question, do you have an example please? I  
don't quite understand your case. Thanks.

It's probably unrelated to your case but the keywords you used there  
reminds me that we probably should think about a better place to stuff  
vdpau_render_state than into data[0]. I'd like to preserve data[0-2]  
for the actual pixels readback from the HW accelerator. Keep also in  
mind that .data[0] can't be NULL since h264.c expects it to be  
different for each Picture (during RefPicList1 build IIRC, more  
specifically when we need to swap top entries).

>>> In contrast, IMO per-level PIX_FMTs would have been a serious  
>>> detriment
>>> to future development and generally better hardware support (and I  
>>> still
>>> consider the usage of this stuff a serious misdesign in the  
>>> accelration
>>> APIs).
>>
>> Actually, some manufacturers use level information for error  
>> checking.
>
> With what purpose? Making sure that single-bit errors can break  
> playback
> even more thoroughly? :-P
> It also means the decoder must be reinitialized on level changes.  
> Not such
> a big issue by itself, but it is hard to make playback smooth in  
> that case.

That was exactly my question and complain but I don't write drivers. ;-)