[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