On Sat, Feb 10, 2018 at 05:29:52PM +0100, Luca Barbato wrote:
> On 10/02/2018 16:59, Diego Biurrun wrote:
> > On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
> > > --- /dev/null
> > > +++ b/libavcodec/libaom.c
> > > @@ -0,0 +1,90 @@
> > > +enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth)
> > > +{
> > > + switch (img) {
> > > + case AOM_IMG_FMT_RGB24: return AV_PIX_FMT_RGB24;
> > > + case AOM_IMG_FMT_RGB565: return AV_PIX_FMT_RGB565BE;
> > > + case AOM_IMG_FMT_RGB555: return AV_PIX_FMT_RGB555BE;
> > > + case AOM_IMG_FMT_UYVY: return AV_PIX_FMT_UYVY422;
> > > + case AOM_IMG_FMT_YUY2: return AV_PIX_FMT_YUYV422;
> > > + case AOM_IMG_FMT_YVYU: return AV_PIX_FMT_YVYU422;
> > > + case AOM_IMG_FMT_BGR24: return AV_PIX_FMT_BGR24;
> > > + case AOM_IMG_FMT_ARGB: return AV_PIX_FMT_ARGB;
> > > + case AOM_IMG_FMT_ARGB_LE: return AV_PIX_FMT_BGRA;
> > > + case AOM_IMG_FMT_RGB565_LE: return AV_PIX_FMT_RGB565LE;
> > > + case AOM_IMG_FMT_RGB555_LE: return AV_PIX_FMT_RGB555LE;
> > > + case AOM_IMG_FMT_I420: return AV_PIX_FMT_YUV420P;
> > > + case AOM_IMG_FMT_I422: return AV_PIX_FMT_YUV422P;
> > > + case AOM_IMG_FMT_I444: return AV_PIX_FMT_YUV444P;
> > > + case AOM_IMG_FMT_444A: return AV_PIX_FMT_YUVA444P;
> > > + case AOM_IMG_FMT_I440: return AV_PIX_FMT_YUV440P;
> >
> > I'd break those lines.
>
> I can run uncrustify to break it, is that the outcome you'd expect?
That would do the trick, yes.
> > > --- /dev/null
> > > +++ b/libavcodec/libaomdec.c
> > > @@ -0,0 +1,137 @@
> > > +static av_cold int aom_init(AVCodecContext *avctx,
> > > + const struct aom_codec_iface *iface)
> > > +{
> > > + if (aom_codec_dec_init(&ctx->decoder, iface, &deccfg, 0) !=
> > > AOM_CODEC_OK) {
> > > + const char *error = aom_codec_error(&ctx->decoder);
> > > + av_log(avctx, AV_LOG_ERROR, "Failed to initialize decoder: %s\n",
> > > + error);
> > > + return AVERROR(EINVAL);
> >
> > These don't look like user-supplied values, so I think EINVAL is not the
> > right error code.
>
> suggest one :)
Never mind, these values are sort of user-supplied.
> > > +AVCodec ff_libaom_av1_decoder = {
> > > + .init = av1_init,
> > > + .close = aom_free,
> > > + .decode = aom_decode,
> >
> > Why av1_init and not aom_init?
>
> Ideally if we want to support av2 through libaom we'd just have to have a
> av2_init.
That's a little bit too future-proof for my taste. If and when that day
arrives, it's trivial to rename the function. Who knows, maybe there will
not even be two init functions then..
Diego
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
