Re: [FFmpeg-devel] [PATCH] Adds decode support for formats other than 420
On Sun, Sep 14, 2014 at 07:43:27AM -0700, Deb Mukherjee wrote: > Handles decoding of new VP9 profiles 1-3 with different color sampling > and bit-depths. > > For high bitdepth (profiles 2 and 3) support, we currently need to link > with the highbitdepth branch of libvpx with --enable-experimental > and --enable-vp9-high config options on. But eventually this branch will > be merged into master, whereafter to enable high bitdepth > support you will need to link with libvpx with configure option > --enable-vp9-highbitdepth on. > --- > libavcodec/libvpxdec.c | 64 > ++ > 1 file changed, 60 insertions(+), 4 deletions(-) applied thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB I have never wished to cater to the crowd; for what I know they do not approve, and what they approve I do not know. -- Epicurus signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Adds decode support for formats other than 420
Deb Mukherjee google.com> writes:
> +if (avctx->codec_id == AV_CODEC_ID_VP8) {
> +if (img->fmt != VPX_IMG_FMT_I420)
Wouldn't (avctx->codec_id == ... && img->fmt != ...)
be simpler?
> +return 0;
Shouldn't this be return AVERROR_INVALIDDATA?
Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Adds decode support for formats other than 420
On Fri, Sep 12, 2014 at 04:07:16PM -0700, Deb Mukherjee wrote:
> Handles decoding of new VP9 profiles 1-3 with different color sampling
> and bit-depths.
>
> For high bitdepth (profiles 2 and 3) support, we currently need to link
> with the highbitdepth branch of libvpx with --enable-experimental
> and --enable-vp9-high config options on. But eventually this branch will
> be merged into master, whereafter to enable high bitdepth
> support you will need to link with libvpx with configure option
> --enable-vp9-highbitdepth on.
> ---
[...]
> @@ -104,7 +162,7 @@ static int vp8_decode(AVCodecContext *avctx,
> return avpkt->size;
> }
>
> -static av_cold int vp8_free(AVCodecContext *avctx)
> +static av_cold int vpx_free(AVCodecContext *avctx)
> {
> VP8Context *ctx = avctx->priv_data;
> vpx_codec_destroy(&ctx->decoder);
> @@ -124,8 +182,8 @@ AVCodec ff_libvpx_vp8_decoder = {
> .id = AV_CODEC_ID_VP8,
> .priv_data_size = sizeof(VP8Context),
> .init = vp8_init,
> -.close = vp8_free,
> -.decode = vp8_decode,
> +.close = vpx_free,
> +.decode = vpx_decode,
> .capabilities = CODEC_CAP_AUTO_THREADS | CODEC_CAP_DR1,
> };
> #endif /* CONFIG_LIBVPX_VP8_DECODER */
> @@ -143,8 +201,8 @@ AVCodec ff_libvpx_vp9_decoder = {
> .id = AV_CODEC_ID_VP9,
> .priv_data_size = sizeof(VP8Context),
> .init = vp9_init,
> -.close = vp8_free,
> -.decode = vp8_decode,
> +.close = vpx_free,
> +.decode = vpx_decode,
> .capabilities = CODEC_CAP_AUTO_THREADS | CODEC_CAP_DR1,
> .init_static_data = ff_vp9_init_static,
> };
renaming functions should be in a seperate patch
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Adds decode support for formats other than 420
On 22/08/14 5:31 PM, Deb Mukherjee wrote:
> Handles new VP9 profiles 1-3 with different color sampling and
> bit-depths.
> ---
> libavcodec/libvpxdec.c | 69
> +++---
> 1 file changed, 60 insertions(+), 9 deletions(-)
>
> diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
> index 94e1e4d..7c397fb 100644
> --- a/libavcodec/libvpxdec.c
> +++ b/libavcodec/libvpxdec.c
> @@ -60,7 +60,58 @@ static av_cold int vpx_init(AVCodecContext *avctx,
> return 0;
> }
>
> -static int vp8_decode(AVCodecContext *avctx,
> +// returns 0 on success, 1 on unsupported
0 on success, AVERROR_INVALIDDATA otherwise.
> +static int set_pix_fmt(AVCodecContext *avctx, struct vpx_image *img) {
> +int ret = 0;
No need for this variable. Just return the corresponding value directly.
> +if (avctx->codec_id == AV_CODEC_ID_VP8) {
> +if (img->fmt != VPX_IMG_FMT_I420)
> +return 1;
> +}
> +switch (img->fmt) {
> +case VPX_IMG_FMT_I420:
> +avctx->pix_fmt = AV_PIX_FMT_YUV420P;
> +break;
> +case VPX_IMG_FMT_I422:
> +avctx->pix_fmt = AV_PIX_FMT_YUV422P;
> +break;
> +case VPX_IMG_FMT_I444:
> +avctx->pix_fmt = AV_PIX_FMT_YUV444P;
> +break;
> +case VPX_IMG_FMT_I42016:
Fails to compile with libvpx 1.3.0 and older.
A quick solution would be to check if VPX_IMG_FMT_HIGH is defined and put all
these inside pre-processor guards.
> +if (img->bit_depth == 10) {
> +avctx->pix_fmt = AV_PIX_FMT_YUV420P10LE;
> +} else if (img->bit_depth == 12) {
> +avctx->pix_fmt = AV_PIX_FMT_YUV420P12LE;
> +} else {
> +ret = 1;
> +}
> +break;
> +case VPX_IMG_FMT_I42216:
> +if (img->bit_depth == 10) {
> +avctx->pix_fmt = AV_PIX_FMT_YUV422P10LE;
> +} else if (img->bit_depth == 12) {
> +avctx->pix_fmt = AV_PIX_FMT_YUV422P12LE;
> +} else {
> +ret = 1;
> +}
> +break;
> +case VPX_IMG_FMT_I44416:
> +if (img->bit_depth == 10) {
> +avctx->pix_fmt = AV_PIX_FMT_YUV444P10LE;
> +} else if (img->bit_depth == 12) {
> +avctx->pix_fmt = AV_PIX_FMT_YUV444P12LE;
> +} else {
> +ret = 1;
> +}
> +break;
> +default:
> +ret = 1;
> +break;
> +}
> +return ret;
> +}
> +
> +static int vpx_decode(AVCodecContext *avctx,
>void *data, int *got_frame, AVPacket *avpkt)
> {
> VP8Context *ctx = avctx->priv_data;
> @@ -82,9 +133,9 @@ static int vp8_decode(AVCodecContext *avctx,
> }
>
> if ((img = vpx_codec_get_frame(&ctx->decoder, &iter))) {
> -if (img->fmt != VPX_IMG_FMT_I420) {
> -av_log(avctx, AV_LOG_ERROR, "Unsupported output colorspace
> (%d)\n",
> - img->fmt);
> +if (set_pix_fmt(avctx, img)) {
if ((ret = set_pix_fmt(avctx, img)) < 0)
Then use "return ret;" below to propagate the value returned by set_pix_fmt();
> +av_log(avctx, AV_LOG_ERROR, "Unsupported output colorspace (%d)
> / bit_depth (%d)\n",
> + img->fmt, img->bit_depth);
Same here, bit_depth is not available in the vpx_image struct in libvpx <=
1.3.0.
Either remove it, or hardcode an 8 if VPX_IMG_FMT_HIGH is not defined.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Adds decode support for formats other than 420
On 22.08.2014, at 22:31, Deb Mukherjee wrote: > +// returns 0 on success, 1 on unsupported Please choose one of the standard ways, either 1 for success and 0 failure, or negative failure, 0 or positive success. This variant, at least in the context of FFmpeg, is just confusing/misleading Though potentially just returning the pixfmt and using an invalid value on error might work out even better, but not sure and I don't have a strong opinion on that. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Adds decode support for formats other than 420
On Fri, 22 Aug 2014 13:31:06 -0700 Deb Mukherjee wrote: > Handles new VP9 profiles 1-3 with different color sampling and > bit-depths. > --- > -static int vp8_decode(AVCodecContext *avctx, > +static int vpx_decode(AVCodecContext *avctx, > -static av_cold int vp8_free(AVCodecContext *avctx) > +static av_cold int vpx_free(AVCodecContext *avctx) probably ok, but changing variable names mixed with adding a feature (more colorspaces) is probably better as two separate commits. -compn ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
