[FFmpeg-devel] [PATCH] libavcodec/libvpx: Add VPx alpha decode support

[James Zern]

On Tue, Jul 12, 2016 at 11:48 AM, Vignesh Venkatasubramanian
<vigneshv-at-google.com at ffmpeg.org> wrote:
> On Mon, Jul 11, 2016 at 5:55 PM, James Zern
> <jzern-at-google.com at ffmpeg.org> wrote:
>> On Thu, Jul 7, 2016 at 11:43 AM, Vignesh Venkatasubramanian
>> <vigneshv-at-google.com at ffmpeg.org> wrote:
>>
>>> [...]
>>> -        av_image_copy(picture->data, picture->linesize, (const uint8_t **)img->planes,
>>> -                      img->stride, avctx->pix_fmt, img->d_w, img->d_h);
>>> +
>>> +        planes[0] = img->planes[VPX_PLANE_Y];
>>> +        planes[1] = img->planes[VPX_PLANE_U];
>>> +        planes[2] = img->planes[VPX_PLANE_V];
>>> +        planes[3] =
>>> +            ctx->has_alpha_channel ? img_alpha->planes[VPX_PLANE_Y] : NULL;
>>> +        linesizes[0] = img->stride[VPX_PLANE_Y];
>>> +        linesizes[1] = img->stride[VPX_PLANE_U];
>>> +        linesizes[2] = img->stride[VPX_PLANE_V];
>>> +        linesizes[3] =
>>> +            ctx->has_alpha_channel ? img_alpha->stride[VPX_PLANE_Y] : 0;
>>> +        av_image_copy(picture->data, picture->linesize, (const uint8_t**)planes,
>>> +                      linesizes, avctx->pix_fmt, img->d_w, img->d_h);
>>>
>>
>> couldn't this just be 1 additional av_image_copy_plane()?
>
> av_image_copy does some width computation [1] before calling
> av_image_copy_plane. i didn't want to duplicate that computation here.
> it just seemed cleaner this way. please let me know if you strongly
> feel otherwise and i can change this.
>

I think this would become something like:

        if (!ctx->has_alpha_channel) {
            av_image_copy(picture->data, picture->linesize,
                          (const uint8_t **)img->planes, img->stride,
                          avctx->pix_fmt, img->d_w, img->d_h);
        } else {
            av_image_copy(picture->data, picture->linesize,
                          (const uint8_t **)img->planes, img->stride,
                          AV_PIX_FMT_YUV420P, img->d_w, img->d_h);
            av_image_copy_plane(picture->data[3], picture->linesize[3],
                                img_alpha->planes[VPX_PLANE_Y], img_alpha->d_w,
                                img->d_w, img->d_h);
        }

The problem there being that you'd probably need to force the
non-alpha version of the first copy to avoid copying uninitialized
data. For now it works because only yuva420p is supported, but should
someone want a higher profile it would complicate things. I'm all
right with this, unless someone has a better suggestion.

> [1] https://github.com/FFmpeg/FFmpeg/blob/eae2d89bf715bc3edff478174b43e1f388e768bf/libavutil/imgutils.c#L326
>