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

Vignesh Venkatasubramanian vigneshv at google.com
Tue Jul 12 21:48:09 EEST 2016


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:
>> VPx (VP8/VP9) alpha encoding has been part of FFmpeg. Now, add the
>> ability to decode such files with alpha channel.
>>
>> Signed-off-by: Vignesh Venkatasubramanian <vigneshv at google.com>
>> ---
>>  libavcodec/libvpxdec.c          | 111 ++++++++++++++++++++++++++++-------
>>  tests/fate/vpx.mak              |   3 +
>>  tests/ref/fate/vp8-alpha-decode | 125 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 219 insertions(+), 20 deletions(-)
>>  create mode 100644 tests/ref/fate/vp8-alpha-decode
>>
>> [...]
>> +static int vpx_codec_dec_init_wrapper(AVCodecContext *avctx,
>>
>
> maybe just decoder/dec_init?
>

have re-used vpx_init.

>> [...]
>> +static av_cold int vpx_init(AVCodecContext *avctx,
>> +                            const struct vpx_codec_iface *iface)
>> +{
>> +    VP8Context *ctx = avctx->priv_data;
>
> just leave this extraction to the init wrapper.

done.

>
>> +    return vpx_codec_dec_init_wrapper(avctx, ctx, iface, 0);
>
>> [...]
>> +static int vpx_codec_decode_wrapper(AVCodecContext *avctx,
>>
>
> maybe just decode / decode_frame?

done.

>
>> [...]
>> +    side_data = av_packet_get_side_data(avpkt,
>> +                                        AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL,
>> +                                        &side_data_size);
>> +    if (side_data_size > 1) {
>> +        uint64_t additional_id = AV_RB64(side_data);
>>
>
> could be const.

done.

>
>> +        side_data += 8;
>> +        side_data_size -= 8;
>>
>
> this adjustment could be within the if().

it's a bit better future-proof'ed this way. if we need to process
another additional_id later, it can just be an else-if.

>
>> [...]
>> -        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.

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

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
Vignesh


More information about the ffmpeg-devel mailing list