[libav-devel] [PATCH 07/10] lavu: add a pixel format for new VDA hwaccel

Luca Barbato lu_zero at gentoo.org
Mon Mar 24 22:36:29 CET 2014


On 24/03/14 22:19, Rémi Denis-Courmont wrote:
> Le lundi 24 mars 2014, 21:46:54 Luca Barbato a écrit :
>>> If suddenly libavcodec started offering AV_PIX_FMT_VDPAU for 4:2:2,
>>> decoding would break. VLC would not know that suddenly a pixel format
>>> that was 4:2:0 has become 4:2:2 (too).
>>
>> Nothing would happen "suddenly", you set in the global context which is
>> the pixel format (so you know by the time your callback is it in
>> get_format).
> 
> Oh yes it will break.
> 
> The hardware might be able to decode a 420 bitstream into a 422 surface. But 
> it definitely will not be able to decode a 422 bitstream into a 420 surface, 
> because the memory allocation will be too small.

Who is asking the hardware to do the decoding is the same setting the
surface is the same software.

> Thus all applications that assume that AV_PIX_FMT_VDPAU or AV_PIX_FMT_VAAPI is 
> 420 will break. Even your own avconv_vdpau will break!

If it does there is a bug and will be fixed.

>>> It would therefore allocate VDP_CHROMA_TYPE_420
>>> surfaces from the hardware. That is obviously wrong and a backward
>>> compatibility breakage.
>>>
>>> The correct approach, should the need arise, would be to define
>>> AV_PIX_FMT_VDPAU_422 or whatever. Then old VLC versions would ignore and
>>> fall back to software decoding as before. New versions can add support
>>> safely.
>> No, and you got 3 different explanations why it is wrong.
> 
> The only explanation I got was that it is possible for an application to work 
> around the problem by checking the last pixel format. That is an ugly, 
> limiting work-around and does not solve backward compatibility in any way.

What I told you is that libavcodec knows _nothing_ about that since both
the global context setup and the get buffer is on the user.

I'm trying to move most of the boilerplate code back in libav so then
your approach should be considered (but then the backward compatibility
will bite me in the form of a stabbing Anton and a slicing Hendrik).

>> vdpau according to the code I'm seeing for avconv and mpv does work as
>> I'm stating:
> 
> As a matter of fact, the avconv_vdpau code hardcodes 420 chroma type both in 
> hardware capability check and in surfaces allocation. If you ever feed it 
> non-420 bitstream, it will break just as badly as VLC.

Then should be fixed, I was looking at this mapping

static const int vdpau_formats[][2] = {
    { VDP_YCBCR_FORMAT_YV12, AV_PIX_FMT_YUV420P },
    { VDP_YCBCR_FORMAT_NV12, AV_PIX_FMT_NV12 },
    { VDP_YCBCR_FORMAT_YUYV, AV_PIX_FMT_YUYV422 },
    { VDP_YCBCR_FORMAT_UYVY, AV_PIX_FMT_UYVY422 },
};


And

    switch (imgfmt) {
    case IMGFMT_420P:
        ycbcr = VDP_YCBCR_FORMAT_YV12;
        break;
    case IMGFMT_NV12:
        ycbcr = VDP_YCBCR_FORMAT_NV12;
        break;
    case IMGFMT_YUYV:
        ycbcr = VDP_YCBCR_FORMAT_YUYV;
        chroma = VDP_CHROMA_TYPE_422;
        break;
    case IMGFMT_UYVY:
        ycbcr = VDP_YCBCR_FORMAT_UYVY;
        chroma = VDP_CHROMA_TYPE_422;
        break;

This one.

I have no qualms in being wrong or correct, I'm just telling what I
gather after I discussed this topic for quite a bit of time and having a
look at the code.

>> - on get_format you figure out what you need and set it in the global
>> context.
>> - on get_buffer you provide the buffer using the correct VDP_
>> (CHROMA_TYPE|YCBCR_FORMAT).
> 
> The read-back code supports 422, but the surface allocation code does not and 
> will break. For ****'s sake, there is not a single mention of 
> VDP_CHROMA_TYPE_422 in the entire libav tree.

There is no need to use colorful language.

>> Nothing breaks and nothing would break even by moving to hwaccel2 while
>> keeping a single pixel format for hwaccel.
> 
> Oh yes things break.

Then we'll fix them, we are good at it =)

lu



More information about the libav-devel mailing list