On 10/02/2018 16:59, Diego Biurrun wrote:
Looks OK in general.
On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
--- /dev/null
+++ b/libavcodec/libaom.c
@@ -0,0 +1,90 @@
+
+#define HIGH_DEPTH(fmt) \
+ case AOM_IMG_FMT_I ## fmt ## 16: switch (depth) { \
+ case 10: return AV_PIX_FMT_YUV ## fmt ## P10; \
+ case 12: return AV_PIX_FMT_YUV ## fmt ## P12; \
+ default: return AV_PIX_FMT_NONE; \
Move the switch statement to the next line for better readability please.
Sure
+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?
+/* case AOM_IMG_FMT_I42016: return AV_PIX_FMT_YUV420P16;
+ case AOM_IMG_FMT_I42216: return AV_PIX_FMT_YUV422P16;
+ case AOM_IMG_FMT_I44416: return AV_PIX_FMT_YUV444P16; */
Why is this commented out?
I should just remove it, thanks for reminding me.
+aom_img_fmt_t ff_aom_pixfmt_to_imgfmt(enum AVPixelFormat pix)
+{
+ switch (pix) {
+ case AV_PIX_FMT_RGB24: return AOM_IMG_FMT_RGB24;
+ case AV_PIX_FMT_RGB565BE: return AOM_IMG_FMT_RGB565;
+ case AV_PIX_FMT_RGB555BE: return AOM_IMG_FMT_RGB555;
+ case AV_PIX_FMT_UYVY422: return AOM_IMG_FMT_UYVY;
+ case AV_PIX_FMT_YUYV422: return AOM_IMG_FMT_YUY2;
+ case AV_PIX_FMT_YVYU422: return AOM_IMG_FMT_YVYU;
+ case AV_PIX_FMT_BGR24: return AOM_IMG_FMT_BGR24;
+ case AV_PIX_FMT_ARGB: return AOM_IMG_FMT_ARGB;
+ case AV_PIX_FMT_BGRA: return AOM_IMG_FMT_ARGB_LE;
+ case AV_PIX_FMT_RGB565LE: return AOM_IMG_FMT_RGB565_LE;
+ case AV_PIX_FMT_RGB555LE: return AOM_IMG_FMT_RGB555_LE;
+ case AV_PIX_FMT_YUV420P: return AOM_IMG_FMT_I420;
+ case AV_PIX_FMT_YUV422P: return AOM_IMG_FMT_I422;
+ case AV_PIX_FMT_YUV444P: return AOM_IMG_FMT_I444;
+ case AV_PIX_FMT_YUVA444P: return AOM_IMG_FMT_444A;
+ case AV_PIX_FMT_YUV440P: return AOM_IMG_FMT_I440;
+ case AV_PIX_FMT_YUV420P10: return AOM_IMG_FMT_I42016;
+ case AV_PIX_FMT_YUV422P10: return AOM_IMG_FMT_I42216;
+ case AV_PIX_FMT_YUV444P10: return AOM_IMG_FMT_I44416;
+ case AV_PIX_FMT_YUV420P12: return AOM_IMG_FMT_I42016;
+ case AV_PIX_FMT_YUV422P12: return AOM_IMG_FMT_I42216;
+ case AV_PIX_FMT_YUV444P12: return AOM_IMG_FMT_I44416;
same
Ok.
--- /dev/null
+++ b/libavcodec/libaom.h
@@ -0,0 +1,31 @@
+
+#ifndef AVCODEC_LIBAOM_H
+#define AVCODEC_LIBAOM_H
+
+#include <aom/aom_codec.h>
+
+#include "libavutil/pixfmt.h"
+
+enum AVPixelFormat ff_aom_imgfmt_to_pixfmt(aom_img_fmt_t img, int depth);
+aom_img_fmt_t ff_aom_pixfmt_to_imgfmt(enum AVPixelFormat pix);
+
+#endif /* AVCODEC_LIBAOM_H */
Is encoding support planned? Otherwise splitting this into several files
is kind of silly.
I already have it, just I do not want to have people have an easy way to
generate broken files.
--- /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 :)
+static int aom_decode(AVCodecContext *avctx, void *data, int *got_frame,
+ AVPacket *avpkt)
+{
+ *got_frame = 1;
odd spacing
+static av_cold int aom_free(AVCodecContext *avctx)
Why not aom_close()?
can be changed
+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.
lu
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
