Re: [libav-devel] [PATCH 3/3] Support AV1 encoding using libaom

2018-03-12 Thread Diego Biurrun 
On Mon, Mar 12, 2018 at 11:30:34AM +0100, Hendrik Leppkes wrote:
> On Mon, Mar 12, 2018 at 11:27 AM, Diego Biurrun  wrote:
> >> --- a/libavcodec/avcodec.h
> >> +++ b/libavcodec/avcodec.h
> >> @@ -2551,6 +2551,10 @@ typedef struct AVCodecContext {
> >>  #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE  3
> >>  #define FF_PROFILE_HEVC_REXT4
> >>
> >> +#define FF_PROFILE_AV1_00
> >> +#define FF_PROFILE_AV1_11
> >> +#define FF_PROFILE_AV1_22
> >
> > I think it's a mistake to add more FF_-prefixed stuff into avcodec.h
> > and I don't see why these defines have to be in a public header. They
> > are only used in libaomenc.c, I don't see a reason to place them
> > outside of that file.
> 
> All codec profiles are listed right there with a FF prefix, its used
> by the user to set avctx->profile (or check the profile from a
> decoder), so the symbols make sense to have public.

Good point, but then this thing needs a version bump as it adds public
API.

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

Re: [libav-devel] [PATCH] configure: Don't assume an aligned stack on clang on windows

2018-03-12 Thread Luca Barbato 

On 12/03/2018 23:38, Martin Storsjö wrote:

If we'd enable a 16 byte aligned stack, clang/llvm would also assume
that alignment everywhere and produce code that strictly requires it.
That would require adding realignment (via attribute_align_arg) on every
single public library function or enable -mstackrealign (which does the
same on every single function).

Also relatedly; the parameter currently tested (-mllvm
-stack-alignment=16) hasn't actually been supported for quite some
time; current clang versions use -mstack-alignment=16 for the same.
Actually testing for that parameter would be a different change
though, since it has a real risk of changing behaviour on any other
platform where clang is used.
---
  configure | 11 ++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index b91be32..7042635 100755
--- a/configure
+++ b/configure
@@ -4955,7 +4955,16 @@ elif enabled gcc; then
  elif enabled llvm_gcc; then
  check_cflags -mllvm -stack-alignment=16
  elif enabled clang; then
-check_cflags -mllvm -stack-alignment=16
+if [ "$target_os" = "mingw32" -o "$target_os" = "win32" ] && enabled 
x86_32; then
+# Clang doesn't support maintaining alignment without assuming the
+# same alignment in every function. If 16 byte alignment would be
+# enabled, one would also have to either add attribute_align_arg on
+# every single entry point into the libraries or enable -mstackrealign
+# (doing stack realignment in every single function).
+disable aligned_stack
+else
+check_cflags -mllvm -stack-alignment=16
+fi
  check_cflags -Qunused-arguments
  check_cflags -Werror=implicit-function-declaration
  check_cflags -Werror=missing-prototypes



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

[libav-devel] [PATCH] configure: Don't assume an aligned stack on clang on windows

2018-03-12 Thread Martin Storsjö 
If we'd enable a 16 byte aligned stack, clang/llvm would also assume
that alignment everywhere and produce code that strictly requires it.
That would require adding realignment (via attribute_align_arg) on every
single public library function or enable -mstackrealign (which does the
same on every single function).

Also relatedly; the parameter currently tested (-mllvm
-stack-alignment=16) hasn't actually been supported for quite some
time; current clang versions use -mstack-alignment=16 for the same.
Actually testing for that parameter would be a different change
though, since it has a real risk of changing behaviour on any other
platform where clang is used.
---
 configure | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index b91be32..7042635 100755
--- a/configure
+++ b/configure
@@ -4955,7 +4955,16 @@ elif enabled gcc; then
 elif enabled llvm_gcc; then
 check_cflags -mllvm -stack-alignment=16
 elif enabled clang; then
-check_cflags -mllvm -stack-alignment=16
+if [ "$target_os" = "mingw32" -o "$target_os" = "win32" ] && enabled 
x86_32; then
+# Clang doesn't support maintaining alignment without assuming the
+# same alignment in every function. If 16 byte alignment would be
+# enabled, one would also have to either add attribute_align_arg on
+# every single entry point into the libraries or enable -mstackrealign
+# (doing stack realignment in every single function).
+disable aligned_stack
+else
+check_cflags -mllvm -stack-alignment=16
+fi
 check_cflags -Qunused-arguments
 check_cflags -Werror=implicit-function-declaration
 check_cflags -Werror=missing-prototypes
-- 
2.7.4

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

Re: [libav-devel] [PATCH] Support AV1 encoding using libaom

2018-03-12 Thread Luca Barbato 

On 12/03/2018 14:00, Diego Biurrun wrote:

On Mon, Mar 12, 2018 at 12:11:22PM +0100, Luca Barbato wrote:

--- /dev/null
+++ b/libavcodec/libaomenc.c
@@ -0,0 +1,583 @@
+// provide dummy value to initialize wrapper, values will be updated each 
_encode()
+aom_img_wrap(>rawimg, ff_aom_pixfmt_to_imgfmt(avctx->pix_fmt),
+ avctx->width, avctx->height, 1, (unsigned char *)1);


What's with the strange cast?


What the comment says, a dummy value that gets updated with the correct 
values once the frame is available.


You might notice that the same code is in libvpxenc.

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

Re: [libav-devel] [PATCH] Support AV1 encoding using libaom

2018-03-12 Thread Diego Biurrun 
On Mon, Mar 12, 2018 at 12:11:22PM +0100, Luca Barbato wrote:
> --- /dev/null
> +++ b/libavcodec/libaomenc.c
> @@ -0,0 +1,583 @@
> +// provide dummy value to initialize wrapper, values will be updated 
> each _encode()
> +aom_img_wrap(>rawimg, ff_aom_pixfmt_to_imgfmt(avctx->pix_fmt),
> + avctx->width, avctx->height, 1, (unsigned char *)1);

What's with the strange cast?


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

Re: [libav-devel] [PATCH 3/3] Support AV1 encoding using libaom

2018-03-12 Thread Diego Biurrun 
On Mon, Mar 12, 2018 at 12:10:10PM +0100, Luca Barbato wrote:
> On 12/03/2018 11:27, Diego Biurrun wrote:
> > On Thu, Mar 08, 2018 at 09:50:40PM +0100, Luca Barbato wrote:
> > > ---
> > >   configure  |   1 +
> > >   libavcodec/Makefile|   1 +
> > >   libavcodec/allcodecs.c |   2 +-
> > >   libavcodec/avcodec.h   |   4 +
> > >   libavcodec/libaomenc.c | 584 
> > > +
> > >   5 files changed, 591 insertions(+), 1 deletion(-)
> > >   create mode 100644 libavcodec/libaomenc.c
> > 
> > missing docs update and changelog entry
> 
> Patch 2/3 contains both already.

No, it only adds a mention of decoding support to docs/general.texi.
I'll take care of it.

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

[libav-devel] [PATCH] Support AV1 encoding using libaom

2018-03-12 Thread Luca Barbato 
---
 configure  |   1 +
 libavcodec/Makefile|   1 +
 libavcodec/allcodecs.c |   2 +-
 libavcodec/avcodec.h   |   4 +
 libavcodec/libaomenc.c | 583 +
 5 files changed, 590 insertions(+), 1 deletion(-)
 create mode 100644 libavcodec/libaomenc.c

diff --git a/configure b/configure
index a43fb93cdf..42465edf94 100755
--- a/configure
+++ b/configure
@@ -2366,6 +2366,7 @@ avxsynth_deps="libdl"
 avisynth_demuxer_deps_any="avisynth avxsynth"
 avisynth_demuxer_select="riffdec"
 libaom_av1_decoder_deps="libaom"
+libaom_av1_encoder_deps="libaom"
 libdcadec_decoder_deps="libdcadec"
 libfaac_encoder_deps="libfaac"
 libfaac_encoder_select="audio_frame_queue"
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 0b50a839bc..ea0c9dceae 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -688,6 +688,7 @@ OBJS-$(CONFIG_WEBM_MUXER)  += mpeg4audio.o
 
 # external codec libraries
 OBJS-$(CONFIG_LIBAOM_AV1_DECODER) += libaomdec.o libaom.o
+OBJS-$(CONFIG_LIBAOM_AV1_ENCODER) += libaomenc.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
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index ec923cd511..4aee65762e 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -421,7 +421,7 @@ void avcodec_register_all(void)
 REGISTER_ENCDEC (XSUB,  xsub);
 
 /* external libraries */
-REGISTER_DECODER(LIBAOM_AV1,libaom_av1);
+REGISTER_ENCDEC (LIBAOM_AV1,libaom_av1);
 REGISTER_DECODER(LIBDCADEC, libdcadec)
 REGISTER_ENCODER(LIBFAAC,   libfaac);
 REGISTER_ENCDEC (LIBFDK_AAC,libfdk_aac);
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 03a3d5bd6d..ac0915328c 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2551,6 +2551,10 @@ typedef struct AVCodecContext {
 #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE  3
 #define FF_PROFILE_HEVC_REXT4
 
+#define FF_PROFILE_AV1_00
+#define FF_PROFILE_AV1_11
+#define FF_PROFILE_AV1_22
+
 /**
  * level
  * - encoding: Set by user.
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
new file mode 100644
index 00..94b3ddd326
--- /dev/null
+++ b/libavcodec/libaomenc.c
@@ -0,0 +1,583 @@
+/*
+ * Copyright (c) 2010, Google, Inc.
+ *
+ * This file is part of Libav.
+ *
+ * Libav is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * Libav is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Libav; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#define AOM_DISABLE_CTRL_TYPECHECKS 1
+#include 
+#include 
+
+#include "libavutil/base64.h"
+#include "libavutil/common.h"
+#include "libavutil/mathematics.h"
+#include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
+
+#include "avcodec.h"
+#include "internal.h"
+#include "libaom.h"
+
+/*
+ * Portion of struct aom_codec_cx_pkt from aom_encoder.h.
+ * One encoded frame returned from the library.
+ */
+struct FrameListData {
+void *buf;   /* compressed data buffer */
+size_t sz;   /* length of compressed data */
+int64_t pts; /* time stamp to show frame
+  * (in timebase units) */
+unsigned long duration;  /* duration to show frame
+  * (in timebase units) */
+uint32_t flags;  /* flags for this frame */
+struct FrameListData *next;
+};
+
+typedef struct AOMEncoderContext {
+AVClass *class;
+struct aom_codec_ctx encoder;
+struct aom_image rawimg;
+struct aom_fixed_buf twopass_stats;
+struct FrameListData *coded_frame_list;
+int cpu_used;
+int auto_alt_ref;
+int lag_in_frames;
+int error_resilient;
+int crf;
+int static_thresh;
+int drop_threshold;
+int noise_sensitivity;
+} AOMContext;
+
+static const char *const ctlidstr[] = {
+[AOME_SET_CPUUSED]  = "AOME_SET_CPUUSED",
+[AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL",
+[AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF",
+[AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD",
+};
+
+static

Re: [libav-devel] [PATCH 3/3] Support AV1 encoding using libaom

2018-03-12 Thread Luca Barbato 

On 12/03/2018 11:27, Diego Biurrun wrote:

On Thu, Mar 08, 2018 at 09:50:40PM +0100, Luca Barbato wrote:

---
  configure  |   1 +
  libavcodec/Makefile|   1 +
  libavcodec/allcodecs.c |   2 +-
  libavcodec/avcodec.h   |   4 +
  libavcodec/libaomenc.c | 584 +
  5 files changed, 591 insertions(+), 1 deletion(-)
  create mode 100644 libavcodec/libaomenc.c


missing docs update and changelog entry


Patch 2/3 contains both already.




--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2551,6 +2551,10 @@ typedef struct AVCodecContext {
  #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE  3
  #define FF_PROFILE_HEVC_REXT4
  
+#define FF_PROFILE_AV1_00

+#define FF_PROFILE_AV1_11
+#define FF_PROFILE_AV1_22


I think it's a mistake to add more FF_-prefixed stuff into avcodec.h
and I don't see why these defines have to be in a public header. They
are only used in libaomenc.c, I don't see a reason to place them
outside of that file.


I'd move them all to AV_PROFILE in another patch.


--- /dev/null
+++ b/libavcodec/libaomenc.c
@@ -0,0 +1,584 @@
+static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
+  AVPacket *pkt)
+{
+int ret = ff_alloc_packet(pkt, cx_frame->sz);
+if (ret >= 0) {
+memcpy(pkt->data, cx_frame->buf, pkt->size);
+pkt->pts = pkt->dts = cx_frame->pts;
+
+if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
+pkt->flags |= AV_PKT_FLAG_KEY;
+}
+} else {
+av_log(avctx, AV_LOG_ERROR,
+   "Error getting output packet of size %zu.\n", cx_frame->sz);
+return ret;
+}
+return pkt->size;
+}


simpler logic:



Updated



to look ahead at for


+{ "error-resilient", "Error resilience configuration", OFFSET(error_resilient), 
AV_OPT_TYPE_FLAGS, {.i64 = 0}, INT_MIN, INT_MAX, VE, "er"},


I think this should be named error-resilience.


Amended

Sending the resulting file now.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 3/3] Support AV1 encoding using libaom

2018-03-12 Thread Hendrik Leppkes 
On Mon, Mar 12, 2018 at 11:27 AM, Diego Biurrun  wrote:
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -2551,6 +2551,10 @@ typedef struct AVCodecContext {
>>  #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE  3
>>  #define FF_PROFILE_HEVC_REXT4
>>
>> +#define FF_PROFILE_AV1_00
>> +#define FF_PROFILE_AV1_11
>> +#define FF_PROFILE_AV1_22
>
> I think it's a mistake to add more FF_-prefixed stuff into avcodec.h
> and I don't see why these defines have to be in a public header. They
> are only used in libaomenc.c, I don't see a reason to place them
> outside of that file.
>

All codec profiles are listed right there with a FF prefix, its used
by the user to set avctx->profile (or check the profile from a
decoder), so the symbols make sense to have public.

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

Re: [libav-devel] [PATCH 3/3] Support AV1 encoding using libaom

2018-03-12 Thread Diego Biurrun 
On Thu, Mar 08, 2018 at 09:50:40PM +0100, Luca Barbato wrote:
> ---
>  configure  |   1 +
>  libavcodec/Makefile|   1 +
>  libavcodec/allcodecs.c |   2 +-
>  libavcodec/avcodec.h   |   4 +
>  libavcodec/libaomenc.c | 584 
> +
>  5 files changed, 591 insertions(+), 1 deletion(-)
>  create mode 100644 libavcodec/libaomenc.c

missing docs update and changelog entry

> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -2551,6 +2551,10 @@ typedef struct AVCodecContext {
>  #define FF_PROFILE_HEVC_MAIN_STILL_PICTURE  3
>  #define FF_PROFILE_HEVC_REXT4
>  
> +#define FF_PROFILE_AV1_00
> +#define FF_PROFILE_AV1_11
> +#define FF_PROFILE_AV1_22

I think it's a mistake to add more FF_-prefixed stuff into avcodec.h
and I don't see why these defines have to be in a public header. They
are only used in libaomenc.c, I don't see a reason to place them
outside of that file.

> --- /dev/null
> +++ b/libavcodec/libaomenc.c
> @@ -0,0 +1,584 @@
> +static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
> +  AVPacket *pkt)
> +{
> +int ret = ff_alloc_packet(pkt, cx_frame->sz);
> +if (ret >= 0) {
> +memcpy(pkt->data, cx_frame->buf, pkt->size);
> +pkt->pts = pkt->dts = cx_frame->pts;
> +
> +if (!!(cx_frame->flags & AOM_FRAME_IS_KEY)) {
> +pkt->flags |= AV_PKT_FLAG_KEY;
> +}
> +} else {
> +av_log(avctx, AV_LOG_ERROR,
> +   "Error getting output packet of size %zu.\n", cx_frame->sz);
> +return ret;
> +}
> +return pkt->size;
> +}

simpler logic:


  int ret = ff_alloc_packet(pkt, cx_frame->sz);
  if (ret < 0) {
  av_log(avctx, AV_LOG_ERROR,
 "Error getting output packet of size %zu.\n", cx_frame->sz);
  return ret;
  }
  memcpy(pkt->data, cx_frame->buf, pkt->size);
  pkt->pts = pkt->dts = cx_frame->pts;

  if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
  pkt->flags |= AV_PKT_FLAG_KEY;
  return pkt->size;

> +static const AVOption options[] = {
> +{ "cpu-used","Quality/Speed ratio modifier",   
> OFFSET(cpu_used),AV_OPT_TYPE_INT, {.i64 = 1}, INT_MIN, INT_MAX, VE},
> +{ "auto-alt-ref","Enable use of alternate reference "
> + "frames (2-pass only)",   
> OFFSET(auto_alt_ref),AV_OPT_TYPE_INT, {.i64 = -1},  -1,  1,   
> VE},
> +{ "lag-in-frames",   "Number of frames to look ahead for "
> + "alternate reference frame selection",
> OFFSET(lag_in_frames),   AV_OPT_TYPE_INT, {.i64 = -1},  -1,  INT_MAX, 
> VE},

to look ahead at for

> +{ "error-resilient", "Error resilience configuration", 
> OFFSET(error_resilient), AV_OPT_TYPE_FLAGS, {.i64 = 0}, INT_MIN, INT_MAX, VE, 
> "er"},

I think this should be named error-resilience.

nit: spaces inside {} inside the whole block

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