Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread Vittorio Giovara 
On Sun, Feb 11, 2018 at 11:50 AM, Diego Biurrun  wrote:

> On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
> >  libavcodec/version.h   |   2 +-
> >
> > --- a/libavcodec/version.h
> > +++ b/libavcodec/version.h
> > @@ -28,7 +28,7 @@
> >
> >  #define LIBAVCODEC_VERSION_MAJOR 58
> > -#define LIBAVCODEC_VERSION_MINOR  8
> > +#define LIBAVCODEC_VERSION_MINOR  9
> >  #define LIBAVCODEC_VERSION_MICRO  0
>
> I guess we should decide what to do about bumping version when there
> is no change to installed headers, e.g. when a decoder is added without
> adding an AV_CODEC_ID. Anton has been saying that minor should not be
> bumped in those cases and I think he is right.
>

IMO nothing wrong with bumping the minor version, but if not at least micro
should be changed.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread wm4 
On Sun, 11 Feb 2018 18:44:44 +
Mark Thompson  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.

+1, this duplication is really not necessary.

Ignore my previous two empty replies, that's due to my mail client
being crap.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread wm4 
On Sun, 11 Feb 2018 18:44:44 +
Mark Thompson  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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread wm4 
On Sun, 11 Feb 2018 18:44:44 +
Mark Thompson  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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread Mark Thompson 
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.

(Or ignore this if you prefer how the bikeshed is currently made...)

- Mark
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread Luca Barbato 

On 11/02/2018 17:50, Diego Biurrun wrote:

On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:

  libavcodec/version.h   |   2 +-

--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@

  #define LIBAVCODEC_VERSION_MAJOR 58
-#define LIBAVCODEC_VERSION_MINOR  8
+#define LIBAVCODEC_VERSION_MINOR  9
  #define LIBAVCODEC_VERSION_MICRO  0


I guess we should decide what to do about bumping version when there
is no change to installed headers, e.g. when a decoder is added without
adding an AV_CODEC_ID. Anton has been saying that minor should not be
bumped in those cases and I think he is right.


I do not have opinion about it, locally amended to avoid the version bump.

___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread Diego Biurrun 
On Fri, Feb 09, 2018 at 10:51:36AM +0100, Luca Barbato wrote:
>  libavcodec/version.h   |   2 +-
> 
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -28,7 +28,7 @@
> 
>  #define LIBAVCODEC_VERSION_MAJOR 58
> -#define LIBAVCODEC_VERSION_MINOR  8
> +#define LIBAVCODEC_VERSION_MINOR  9
>  #define LIBAVCODEC_VERSION_MICRO  0

I guess we should decide what to do about bumping version when there
is no change to installed headers, e.g. when a decoder is added without
adding an AV_CODEC_ID. Anton has been saying that minor should not be
bumped in those cases and I think he is right.

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-11 Thread Luca Barbato 

On 10/02/2018 18:15, Diego Biurrun wrote:

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(>decoder, iface, , 0) != AOM_CODEC_OK) {
+const char *error = aom_codec_error(>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..


Comments addressed, I'll push it tomorrow.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-10 Thread Diego Biurrun 
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(>decoder, iface, , 0) != 
> > > AOM_CODEC_OK) {
> > > +const char *error = aom_codec_error(>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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-10 Thread Luca Barbato 

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 
+
+#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(>decoder, iface, , 0) != AOM_CODEC_OK) {
+const char *error = aom_codec_error(>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

Re: [libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-10 Thread Diego Biurrun 
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.

> +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.

> +/*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?

> +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

> --- /dev/null
> +++ b/libavcodec/libaom.h
> @@ -0,0 +1,31 @@
> +
> +#ifndef AVCODEC_LIBAOM_H
> +#define AVCODEC_LIBAOM_H
> +
> +#include 
> +
> +#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.

> --- /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(>decoder, iface, , 0) != 
> AOM_CODEC_OK) {
> +const char *error = aom_codec_error(>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.

> +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()?

> +AVCodec ff_libaom_av1_decoder = {
> +.init   = av1_init,
> +.close  = aom_free,
> +.decode = aom_decode,

Why av1_init and not aom_init?

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] Add AV1 video decoding support through libaom

2018-02-09 Thread Luca Barbato 
Signed-off-by: Diego Biurrun 
Signed-off-by: Luca Barbato 
---

Freshen to work with the current libaom.

 Changelog  |   1 +
 configure  |   4 ++
 doc/general.texi   |  10 
 libavcodec/Makefile|   2 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/libaom.c|  90 
 libavcodec/libaom.h|  31 +++
 libavcodec/libaomdec.c | 137 +
 libavcodec/version.h   |   2 +-
 9 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/libaom.c
 create mode 100644 libavcodec/libaom.h
 create mode 100644 libavcodec/libaomdec.c

diff --git a/Changelog b/Changelog
index 51c3f85a28..0d20cd47df 100644
--- a/Changelog
+++ b/Changelog
@@ -20,6 +20,7 @@ version :
 - Intel QSV-accelerated MJPEG encoding
 - NVIDIA CUVID-accelerated H.264 and HEVC decoding
 - Intel QSV-accelerated overlay filter
+- AV1 Support through libaom


 version 12:
diff --git a/configure b/configure
index 9f84f88014..1bb9072da4 100755
--- a/configure
+++ b/configure
@@ -187,6 +187,7 @@ External library support:
   --enable-bzlib bzip2 compression [autodetect]
   --enable-frei0rvideo filtering plugins
   --enable-gnutlscrypto
+  --enable-libaomAV1 video encoding/decoding
   --enable-libbs2b   Bauer stereophonic-to-binaural DSP
   --enable-libcdio   audio CD input
   --enable-libdc1394 IEEE 1394/Firewire camera input
@@ -1297,6 +1298,7 @@ EXTERNAL_LIBRARY_LIST="
 avxsynth
 frei0r
 gnutls
+libaom
 libbs2b
 libdc1394
 libdcadec
@@ -2311,6 +2313,7 @@ avisynth_deps="LoadLibrary"
 avxsynth_deps="libdl"
 avisynth_demuxer_deps_any="avisynth avxsynth"
 avisynth_demuxer_select="riffdec"
+libaom_av1_decoder_deps="libaom"
 libdcadec_decoder_deps="libdcadec"
 libfaac_encoder_deps="libfaac"
 libfaac_encoder_select="audio_frame_queue"
@@ -4602,6 +4605,7 @@ enabled cuvid && require cuvid cuviddec.h 
cuvidCreateDecoder -lnvcuv
 enabled frei0r&& require_header frei0r.h
 enabled gnutls&& require_pkg_config gnutls gnutls gnutls/gnutls.h 
gnutls_global_init &&
  check_lib gmp gmp.h mpz_export -lgmp
+enabled libaom&& require_pkg_config libaom "aom >= 0.1.0" 
aom/aom_codec.h aom_codec_version
 enabled libbs2b   && require_pkg_config libbs2b libbs2b bs2b.h 
bs2b_open
 enabled libdc1394 && require_pkg_config libdc1394 libdc1394-2 
dc1394/dc1394.h dc1394_new
 enabled libdcadec && require libdcadec libdcadec/dca_context.h 
dcadec_context_create -ldcadec
diff --git a/doc/general.texi b/doc/general.texi
index 0c92761a49..e066b42187 100644
--- a/doc/general.texi
+++ b/doc/general.texi
@@ -16,6 +16,14 @@ for more formats. None of them are used by default, their 
use has to be
 explicitly requested by passing the appropriate flags to
 @command{./configure}.

+@section Alliance for Open Media libaom
+
+Libav can make use of the libaom library for AV1 decoding.
+
+Go to @url{http://aomedia.org/} and follow the instructions for
+installing the library. Then pass @code{--enable-libaom} to configure to
+enable it.
+
 @section OpenCORE and VisualOn libraries

 Spun off Google Android sources, OpenCore, VisualOn and Fraunhofer
@@ -617,6 +625,8 @@ following image formats are supported:
 @item Autodesk Animator Flic video  @tab @tab  X
 @item Autodesk RLE   @tab @tab  X
 @tab fourcc: AASC
+@item AV1@tab @tab  E
+@tab Supported through external library libaom
 @item AVS (Audio Video Standard) video  @tab @tab  X
 @tab Video encoding used by the Creature Shock game.
 @item Beam Software VB   @tab @tab  X
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 99969ac779..0b50a839bc 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -687,6 +687,7 @@ OBJS-$(CONFIG_TAK_DEMUXER) += tak.o
 OBJS-$(CONFIG_WEBM_MUXER)  += mpeg4audio.o

 # external codec libraries
+OBJS-$(CONFIG_LIBAOM_AV1_DECODER) += libaomdec.o libaom.o
 OBJS-$(CONFIG_LIBDCADEC_DECODER)  += libdcadec.o dca.o
 OBJS-$(CONFIG_LIBFAAC_ENCODER)+= libfaac.o
 OBJS-$(CONFIG_LIBFDK_AAC_DECODER) += libfdk-aacdec.o
@@ -814,6 +815,7 @@ SKIPHEADERS-$(CONFIG_CUVID)+= cuvid.h
 SKIPHEADERS-$(CONFIG_AMF)  += amfenc.h
 SKIPHEADERS-$(CONFIG_D3D11VA)  += d3d11va.h dxva2_internal.h
 SKIPHEADERS-$(CONFIG_DXVA2)+= dxva2.h dxva2_internal.h
+SKIPHEADERS-$(CONFIG_LIBAOM)   += libaom.h
 SKIPHEADERS-$(CONFIG_LIBSCHROEDINGER)  += libschroedinger.h
 SKIPHEADERS-$(CONFIG_LIBVPX)   += libvpx.h
 SKIPHEADERS-$(CONFIG_NVENC)+= nvenc.h
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index efde5a2b0e..ec923cd511 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@