On Sun, 11 Feb 2018 18:44:44 +0000
Mark Thompson <s...@jkqxz.net> wrote:
> On 10/02/18 16:29, Luca Barbato wrote:
> > 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.
>
> If you feel like tearing down the bikeshed and building a different one: I
> think this would be nicer as a single table with the two functions reading
> it, rather than two functions which duplicate a lot of the information.
_______________________________________________
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel
