Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example

[Jun Zhao]

On 2017/7/11 12:38, 刘歧 wrote:
> From 0e4d230ae4c98949a962c6bbdad31d216b54bb6a Mon Sep 17 00:00:00 2001
> From: Jun Zhao 
> Date: Tue, 21 Mar 2017 11:04:41 +0800
> Subject: [V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example.
> 
> Add a VA-API hwaccel decoding example.
> 
> Signed-off-by: Liu, Kaixuan 
> Signed-off-by: Jun Zhao 
> ---
>  doc/examples/vaapi_dec.c | 266 
> +++
>  1 file changed, 266 insertions(+)
>  create mode 100644 doc/examples/vaapi_dec.c
> 
> diff --git a/doc/examples/vaapi_dec.c b/doc/examples/vaapi_dec.c
> new file mode 100644
> index 00..01320a3b71
> --- /dev/null
> +++ b/doc/examples/vaapi_dec.c
> @@ -0,0 +1,266 @@
> +/*
> + * Video Acceleration API (video decoding) decode sample
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +/**
> + * @file
> + * Intel VAAPI-accelerated decoding example.
> + *
> + * @example vaapi_dec.c
> + * This example shows how to do VAAPI-accelerated decoding with output
> + * frames from the VAAPI video surfaces.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static AVBufferRef *hw_device_ctx = NULL;
> +FILE *output_file = NULL;
> +
> +int decoder_init_vaapi(AVCodecContext *ctx)
> +{
> +int err = 0;
> +const char *dev_name = "/dev/dri/renderD128";
> +
> +if ((err = av_hwdevice_ctx_create(_device_ctx, AV_HWDEVICE_TYPE_VAAPI,
> +  dev_name, NULL, 0)) < 0) {
> +fprintf(stderr, "Failed to create a VAAPI device.\n");
> +return err;
> +}
> +ctx->hw_device_ctx = av_buffer_ref(hw_device_ctx);
> +
> +return err;
> +}
> +
> +static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
> +   const enum AVPixelFormat 
> *pix_fmts)
> +{
> +const enum AVPixelFormat *p;
> +
> +for (p = pix_fmts; *p != -1; p++) {
> +if (*p == AV_PIX_FMT_VAAPI)
> +return *p;
> +}
> +
> +return AV_PIX_FMT_NONE;
> +}
> +
> +int retrieve_data(AVFrame *input)
> +{
> +AVFrame *output = NULL;
> +int err;
> +av_assert0(input->format == AV_PIX_FMT_VAAPI);
> +
> +if (!(output = av_frame_alloc()))
> +return AVERROR(ENOMEM);
> +/* default output nv12 */
> +output->format = AV_PIX_FMT_NV12;
> +if ((err = av_hwframe_transfer_data(output, input, 0)) < 0) {
> +fprintf(stderr, "Failed to transfer data to output frame: %d.\n", 
> err);
> +goto fail;
> +}
> +
> +if ((err = av_frame_copy_props(output, input)) < 0) {
> +av_frame_unref(output);
> +goto fail;
> +}
> +
> +av_frame_unref(input);
> +av_frame_move_ref(input, output);
> +av_frame_free();
> +return 0;
> +
> +fail:
> +av_frame_free();
> +return err;
> +}
> +
> +int write_frame(AVFrame *frame)
> +{
> +int idx, size;
> +int width = frame->width;
> +int height = frame->height;
> +
> +av_assert0(frame && frame->data[0] && output_file);
> +
> +for (idx = 0; idx < height; idx++) {
> +if ((size = fwrite(frame->data[0] + idx*frame->linesize[0],
> +   1, width, output_file)) < 0) {
> +fprintf(stderr, "Dump Y to file error.\n");
> +return -1;
> +}
> +}
> +
> +height >>= 1;
> +for (idx = 0; idx < height; idx++) {
> +if ((size = fwrite(frame->data[1] + idx*frame->linesize[1],
> +   1, width, output_file)) < 0) {
> +fprintf(stderr, "Dump UV to file error.\n");
> +return -1;
> +}
> +}
> +
> +return 0;
> +}
> +
> +int decode_write(AVCodecContext *avctx, AVPacket packet, int flush)
> +{
> +AVFrame *frame = NULL;
> +int ret = 0;
> +
> +ret = avcodec_send_packet(avctx, );
> +if (ret < 0 && ret != AVERROR_EOF)
> +return 0;
> +
> +if (!(frame = av_frame_alloc()))
> +return AVERROR(ENOMEM);
> +
> +ret = avcodec_receive_frame(avctx, frame);
> +if (ret >= 0) {
> +/* retrieve data from GPU to CPU */
> +if ((ret = 

Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add aVA-APIhwaccel decoding example

[liyoubdu]

yes, you.  pls share your test file


 
---Original---
From: "刘歧"
Date: 2017/7/11 13:52:06
To: "and patches development discussions FFmpeg";
Cc: "刘歧";
Subject: Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add aVA-APIhwaccel 
decoding example



> On Jul 11, 2017, at 12:49 PM, liyoubdu  wrote:
> 
> where is your media file for testinfg
me?
> 
> 
> 
> ---Original---
> From: "刘歧"
> Date: 2017/7/11 12:39:31
> To: "and patches development discussions FFmpeg";
> Cc: "wm4";"Mark 
> Thompson";"刘歧";"Liu, 
> Kaixuan";
> Subject: Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a 
> VA-APIhwaccel decoding example
> 
> 
> From 0e4d230ae4c98949a962c6bbdad31d216b54bb6a Mon Sep 17 00:00:00 2001
> From: Jun Zhao 
> Date: Tue, 21 Mar 2017 11:04:41 +0800
> Subject: [V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example.
> 
> Add a VA-API hwaccel decoding example.
> 
> Signed-off-by: Liu, Kaixuan 
> Signed-off-by: Jun Zhao 
> ---
> doc/examples/vaapi_dec.c | 266 +++
> 1 file changed, 266 insertions(+)
> create mode 100644 doc/examples/vaapi_dec.c
> 
> diff --git a/doc/examples/vaapi_dec.c b/doc/examples/vaapi_dec.c
> new file mode 100644
> index 00..01320a3b71
> --- /dev/null
> +++ b/doc/examples/vaapi_dec.c
> @@ -0,0 +1,266 @@
> +/*
> + * Video Acceleration API (video decoding) decode sample
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +/**
> + * @file
> + * Intel VAAPI-accelerated decoding example.
> + *
> + * @example vaapi_dec.c
> + * This example shows how to do VAAPI-accelerated decoding with output
> + * frames from the VAAPI video surfaces.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static AVBufferRef *hw_device_ctx = NULL;
> +FILE *output_file = NULL;
> +
> +int decoder_init_vaapi(AVCodecContext *ctx)
> +{
> +int err = 0;
> +const char *dev_name = "/dev/dri/renderD128";
> +
> +if ((err = av_hwdevice_ctx_create(_device_ctx, AV_HWDEVICE_TYPE_VAAPI,
> +  dev_name, NULL, 0)) < 0) {
> +fprintf(stderr, "Failed to create a VAAPI device.\n");
> +return err;
> +}
> +ctx->hw_device_ctx = av_buffer_ref(hw_device_ctx);
> +
> +return err;
> +}
> +
> +static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
> +   const enum AVPixelFormat 
> *pix_fmts)
> +{
> +const enum AVPixelFormat *p;
> +
> +for (p = pix_fmts; *p != -1; p++) {
> +if (*p == AV_PIX_FMT_VAAPI)
> +return *p;
> +}
> +
> +return AV_PIX_FMT_NONE;
> +}
> +
> +int retrieve_data(AVFrame *input)
> +{
> +AVFrame *output = NULL;
> +int err;
> +av_assert0(input->format == AV_PIX_FMT_VAAPI);
> +
> +if (!(output = av_frame_alloc()))
> +return AVERROR(ENOMEM);
> +/* default output nv12 */
> +output->format = AV_PIX_FMT_NV12;
> +if ((err = av_hwframe_transfer_data(output, input, 0)) < 0) {
> +fprintf(stderr, "Failed to transfer data to output frame: %d.\n", 
> err);
> +goto fail;
> +}
> +
> +if ((err = av_frame_copy_props(output, input)) < 0) {
> +av_frame_unref(output);
> +goto fail;
> +}
> +
> +av_frame_unref(input);
> +av_frame_move_ref(input, output);
> +av_frame_free();
> +return 0;
> +
> +fail:
> +av_frame_free();
> +return err;
> +}
> +
> +int write_frame(AVFrame *frame)
> +{
> +int idx, size;
> +int width = frame->width;
> +int height = frame->height;
> +
> +av_assert0(frame && frame->data[0] && output_file);
> +
> +for (idx = 0; idx < height; idx++) {
> +if ((size = fwrite(frame->data[0] + idx*frame->linesize[0],
> +   1, width, output_file)) < 0) {
> +fprintf(stderr, "Dump Y to file error.\n");
> +return -1;
> +}
> +}
> +
> +  

Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a VA-APIhwaccel decoding example

[刘歧]

> On Jul 11, 2017, at 12:49 PM, liyoubdu  wrote:
> 
> where is your media file for testinfg
me?
> 
> 
> 
> ---Original---
> From: "刘歧"
> Date: 2017/7/11 12:39:31
> To: "and patches development discussions FFmpeg";
> Cc: "wm4";"Mark 
> Thompson";"刘歧";"Liu, 
> Kaixuan";
> Subject: Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a 
> VA-APIhwaccel decoding example
> 
> 
> From 0e4d230ae4c98949a962c6bbdad31d216b54bb6a Mon Sep 17 00:00:00 2001
> From: Jun Zhao 
> Date: Tue, 21 Mar 2017 11:04:41 +0800
> Subject: [V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example.
> 
> Add a VA-API hwaccel decoding example.
> 
> Signed-off-by: Liu, Kaixuan 
> Signed-off-by: Jun Zhao 
> ---
> doc/examples/vaapi_dec.c | 266 +++
> 1 file changed, 266 insertions(+)
> create mode 100644 doc/examples/vaapi_dec.c
> 
> diff --git a/doc/examples/vaapi_dec.c b/doc/examples/vaapi_dec.c
> new file mode 100644
> index 00..01320a3b71
> --- /dev/null
> +++ b/doc/examples/vaapi_dec.c
> @@ -0,0 +1,266 @@
> +/*
> + * Video Acceleration API (video decoding) decode sample
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +
> +/**
> + * @file
> + * Intel VAAPI-accelerated decoding example.
> + *
> + * @example vaapi_dec.c
> + * This example shows how to do VAAPI-accelerated decoding with output
> + * frames from the VAAPI video surfaces.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static AVBufferRef *hw_device_ctx = NULL;
> +FILE *output_file = NULL;
> +
> +int decoder_init_vaapi(AVCodecContext *ctx)
> +{
> +int err = 0;
> +const char *dev_name = "/dev/dri/renderD128";
> +
> +if ((err = av_hwdevice_ctx_create(_device_ctx, AV_HWDEVICE_TYPE_VAAPI,
> +  dev_name, NULL, 0)) < 0) {
> +fprintf(stderr, "Failed to create a VAAPI device.\n");
> +return err;
> +}
> +ctx->hw_device_ctx = av_buffer_ref(hw_device_ctx);
> +
> +return err;
> +}
> +
> +static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
> +   const enum AVPixelFormat 
> *pix_fmts)
> +{
> +const enum AVPixelFormat *p;
> +
> +for (p = pix_fmts; *p != -1; p++) {
> +if (*p == AV_PIX_FMT_VAAPI)
> +return *p;
> +}
> +
> +return AV_PIX_FMT_NONE;
> +}
> +
> +int retrieve_data(AVFrame *input)
> +{
> +AVFrame *output = NULL;
> +int err;
> +av_assert0(input->format == AV_PIX_FMT_VAAPI);
> +
> +if (!(output = av_frame_alloc()))
> +return AVERROR(ENOMEM);
> +/* default output nv12 */
> +output->format = AV_PIX_FMT_NV12;
> +if ((err = av_hwframe_transfer_data(output, input, 0)) < 0) {
> +fprintf(stderr, "Failed to transfer data to output frame: %d.\n", 
> err);
> +goto fail;
> +}
> +
> +if ((err = av_frame_copy_props(output, input)) < 0) {
> +av_frame_unref(output);
> +goto fail;
> +}
> +
> +av_frame_unref(input);
> +av_frame_move_ref(input, output);
> +av_frame_free();
> +return 0;
> +
> +fail:
> +av_frame_free();
> +return err;
> +}
> +
> +int write_frame(AVFrame *frame)
> +{
> +int idx, size;
> +int width = frame->width;
> +int height = frame->height;
> +
> +av_assert0(frame && frame->data[0] && output_file);
> +
> +for (idx = 0; idx < height; idx++) {
> +if ((size = fwrite(frame->data[0] + idx*frame->linesize[0],
> +   1, width, output_file)) < 0) {
> +fprintf(stderr, "Dump Y to file error.\n");
> +return -1;
> +}
> +}
> +
> +height >>= 1;
> +for (idx = 0; idx < height; idx++) {
> +if ((size = fwrite(frame->data[1] + idx*frame->linesize[1],
> +   1, width, output_file)) < 0) {
> +fprintf(stderr, "Dump UV to file error.\n");
> +return -1;
> +}
> +}
> +
> +return 

Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a VA-APIhwaccel decoding example

[liyoubdu]

where is your media file for testinfg


 
---Original---
From: "刘歧"
Date: 2017/7/11 12:39:31
To: "and patches development discussions FFmpeg";
Cc: "wm4";"Mark 
Thompson";"刘歧";"Liu, 
Kaixuan";
Subject: Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a VA-APIhwaccel 
decoding example


From 0e4d230ae4c98949a962c6bbdad31d216b54bb6a Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 21 Mar 2017 11:04:41 +0800
Subject: [V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example.

Add a VA-API hwaccel decoding example.

Signed-off-by: Liu, Kaixuan 
Signed-off-by: Jun Zhao 
---
 doc/examples/vaapi_dec.c | 266 +++
 1 file changed, 266 insertions(+)
 create mode 100644 doc/examples/vaapi_dec.c

diff --git a/doc/examples/vaapi_dec.c b/doc/examples/vaapi_dec.c
new file mode 100644
index 00..01320a3b71
--- /dev/null
+++ b/doc/examples/vaapi_dec.c
@@ -0,0 +1,266 @@
+/*
+ * Video Acceleration API (video decoding) decode sample
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Intel VAAPI-accelerated decoding example.
+ *
+ * @example vaapi_dec.c
+ * This example shows how to do VAAPI-accelerated decoding with output
+ * frames from the VAAPI video surfaces.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static AVBufferRef *hw_device_ctx = NULL;
+FILE *output_file = NULL;
+
+int decoder_init_vaapi(AVCodecContext *ctx)
+{
+int err = 0;
+const char *dev_name = "/dev/dri/renderD128";
+
+if ((err = av_hwdevice_ctx_create(_device_ctx, AV_HWDEVICE_TYPE_VAAPI,
+  dev_name, NULL, 0)) < 0) {
+fprintf(stderr, "Failed to create a VAAPI device.\n");
+return err;
+}
+ctx->hw_device_ctx = av_buffer_ref(hw_device_ctx);
+
+return err;
+}
+
+static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
+   const enum AVPixelFormat *pix_fmts)
+{
+const enum AVPixelFormat *p;
+
+for (p = pix_fmts; *p != -1; p++) {
+if (*p == AV_PIX_FMT_VAAPI)
+return *p;
+}
+
+return AV_PIX_FMT_NONE;
+}
+
+int retrieve_data(AVFrame *input)
+{
+AVFrame *output = NULL;
+int err;
+av_assert0(input->format == AV_PIX_FMT_VAAPI);
+
+if (!(output = av_frame_alloc()))
+return AVERROR(ENOMEM);
+/* default output nv12 */
+output->format = AV_PIX_FMT_NV12;
+if ((err = av_hwframe_transfer_data(output, input, 0)) < 0) {
+fprintf(stderr, "Failed to transfer data to output frame: %d.\n", err);
+goto fail;
+}
+
+if ((err = av_frame_copy_props(output, input)) < 0) {
+av_frame_unref(output);
+goto fail;
+}
+
+av_frame_unref(input);
+av_frame_move_ref(input, output);
+av_frame_free();
+return 0;
+
+fail:
+av_frame_free();
+return err;
+}
+
+int write_frame(AVFrame *frame)
+{
+int idx, size;
+int width = frame->width;
+int height = frame->height;
+
+av_assert0(frame && frame->data[0] && output_file);
+
+for (idx = 0; idx < height; idx++) {
+if ((size = fwrite(frame->data[0] + idx*frame->linesize[0],
+   1, width, output_file)) < 0) {
+fprintf(stderr, "Dump Y to file error.\n");
+return -1;
+}
+}
+
+height >>= 1;
+for (idx = 0; idx < height; idx++) {
+if ((size = fwrite(frame->data[1] + idx*frame->linesize[1],
+   1, width, output_file)) < 0) {
+fprintf(stderr, "Dump UV to file error.\n");
+return -1;
+}
+}
+
+return 0;
+}
+
+int decode_write(AVCodecContext *avctx, AVPacket packet, int flush)
+{
+AVFrame *frame = NULL;
+int ret = 0;
+
+ret = avcodec_send_packet(avctx, );
+if (ret < 0 && ret != AVERROR_EOF)
+return 0;
+
+if (!(frame = av_frame_alloc()))
+return AVERROR(ENOMEM);
+
+ret = avcodec_receive_frame(avctx, frame);
+if (ret >= 0) {
+/* retrieve data 

Re: [FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example

[刘歧]

From 0e4d230ae4c98949a962c6bbdad31d216b54bb6a Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 21 Mar 2017 11:04:41 +0800
Subject: [V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example.

Add a VA-API hwaccel decoding example.

Signed-off-by: Liu, Kaixuan 
Signed-off-by: Jun Zhao 
---
 doc/examples/vaapi_dec.c | 266 +++
 1 file changed, 266 insertions(+)
 create mode 100644 doc/examples/vaapi_dec.c

diff --git a/doc/examples/vaapi_dec.c b/doc/examples/vaapi_dec.c
new file mode 100644
index 00..01320a3b71
--- /dev/null
+++ b/doc/examples/vaapi_dec.c
@@ -0,0 +1,266 @@
+/*
+ * Video Acceleration API (video decoding) decode sample
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Intel VAAPI-accelerated decoding example.
+ *
+ * @example vaapi_dec.c
+ * This example shows how to do VAAPI-accelerated decoding with output
+ * frames from the VAAPI video surfaces.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static AVBufferRef *hw_device_ctx = NULL;
+FILE *output_file = NULL;
+
+int decoder_init_vaapi(AVCodecContext *ctx)
+{
+int err = 0;
+const char *dev_name = "/dev/dri/renderD128";
+
+if ((err = av_hwdevice_ctx_create(_device_ctx, AV_HWDEVICE_TYPE_VAAPI,
+  dev_name, NULL, 0)) < 0) {
+fprintf(stderr, "Failed to create a VAAPI device.\n");
+return err;
+}
+ctx->hw_device_ctx = av_buffer_ref(hw_device_ctx);
+
+return err;
+}
+
+static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
+   const enum AVPixelFormat *pix_fmts)
+{
+const enum AVPixelFormat *p;
+
+for (p = pix_fmts; *p != -1; p++) {
+if (*p == AV_PIX_FMT_VAAPI)
+return *p;
+}
+
+return AV_PIX_FMT_NONE;
+}
+
+int retrieve_data(AVFrame *input)
+{
+AVFrame *output = NULL;
+int err;
+av_assert0(input->format == AV_PIX_FMT_VAAPI);
+
+if (!(output = av_frame_alloc()))
+return AVERROR(ENOMEM);
+/* default output nv12 */
+output->format = AV_PIX_FMT_NV12;
+if ((err = av_hwframe_transfer_data(output, input, 0)) < 0) {
+fprintf(stderr, "Failed to transfer data to output frame: %d.\n", err);
+goto fail;
+}
+
+if ((err = av_frame_copy_props(output, input)) < 0) {
+av_frame_unref(output);
+goto fail;
+}
+
+av_frame_unref(input);
+av_frame_move_ref(input, output);
+av_frame_free();
+return 0;
+
+fail:
+av_frame_free();
+return err;
+}
+
+int write_frame(AVFrame *frame)
+{
+int idx, size;
+int width = frame->width;
+int height = frame->height;
+
+av_assert0(frame && frame->data[0] && output_file);
+
+for (idx = 0; idx < height; idx++) {
+if ((size = fwrite(frame->data[0] + idx*frame->linesize[0],
+   1, width, output_file)) < 0) {
+fprintf(stderr, "Dump Y to file error.\n");
+return -1;
+}
+}
+
+height >>= 1;
+for (idx = 0; idx < height; idx++) {
+if ((size = fwrite(frame->data[1] + idx*frame->linesize[1],
+   1, width, output_file)) < 0) {
+fprintf(stderr, "Dump UV to file error.\n");
+return -1;
+}
+}
+
+return 0;
+}
+
+int decode_write(AVCodecContext *avctx, AVPacket packet, int flush)
+{
+AVFrame *frame = NULL;
+int ret = 0;
+
+ret = avcodec_send_packet(avctx, );
+if (ret < 0 && ret != AVERROR_EOF)
+return 0;
+
+if (!(frame = av_frame_alloc()))
+return AVERROR(ENOMEM);
+
+ret = avcodec_receive_frame(avctx, frame);
+if (ret >= 0) {
+/* retrieve data from GPU to CPU */
+if ((ret = retrieve_data(frame)) < 0)
+goto fail;
+
+if ((ret = write_frame(frame)) < 0)
+goto fail;
+} else if (flush == 0) {
What about !flush? or small than 0?
+ret = 0;
+}
+
+fail:
+av_frame_free();
+return ret;
+}
+
+/* flush the decoder */
+int flush(AVCodecContext *avctx)
+{
+AVPacket packet;
+int ret = 0;
+
+

Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[Muhammad Faiz]

On Tue, Jul 11, 2017 at 4:40 AM, Michael Niedermayer
 wrote:
> On Mon, Jul 10, 2017 at 10:53:42AM +0200, wm4 wrote:
>> On Sat, 8 Jul 2017 01:45:06 +0200
>> Michael Niedermayer  wrote:
>>
>> > On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
>> > > Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
>> > > uses distict mutex/cond. Also let main thread help running jobs.
>> > >
>> > > Benchmark using afir with threads=5 and 4096 taps fir:
>> > > channels=1:
>> > > old:
>> > > 1849650 decicycles in afir_execute,   2 runs,  0 skips
>> > > 1525719 decicycles in afir_execute,1024 runs,  0 skips
>> > > 1546032 decicycles in afir_execute,   16356 runs, 28 skips
>> > > new:
>> > > 1495525 decicycles in afir_execute,   2 runs,  0 skips
>> > >  968897 decicycles in afir_execute,1024 runs,  0 skips
>> > >  941286 decicycles in afir_execute,   16384 runs,  0 skips
>> > >
>> > > channels=2:
>> > > old:
>> > > 3135485 decicycles in afir_execute,   2 runs,  0 skips
>> > > 1967158 decicycles in afir_execute,1024 runs,  0 skips
>> > > 1802430 decicycles in afir_execute,   16364 runs, 20 skips
>> > > new:
>> > > 1864750 decicycles in afir_execute,   2 runs,  0 skips
>> > > 1437792 decicycles in afir_execute,1024 runs,  0 skips
>> > > 1183963 decicycles in afir_execute,   16382 runs,  2 skips
>> > >
>> > > channels=4:
>> > > old:
>> > > 4879925 decicycles in afir_execute,   2 runs,  0 skips
>> > > 3557950 decicycles in afir_execute,1022 runs,  2 skips
>> > > 3206843 decicycles in afir_execute,   16379 runs,  5 skips
>> > > new:
>> > > 2962320 decicycles in afir_execute,   2 runs,  0 skips
>> > > 2450430 decicycles in afir_execute,1024 runs,  0 skips
>> > > 2446219 decicycles in afir_execute,   16383 runs,  1 skips
>> > >
>> > > channels=8:
>> > > old:
>> > > 6032455 decicycles in afir_execute,   2 runs,  0 skips
>> > > 4838614 decicycles in afir_execute,1023 runs,  1 skips
>> > > 4720760 decicycles in afir_execute,   16369 runs, 15 skips
>> > > new:
>> > > 5228150 decicycles in afir_execute,   2 runs,  0 skips
>> > > 4592129 decicycles in afir_execute,1023 runs,  1 skips
>> > > 4469067 decicycles in afir_execute,   16383 runs,  1 skips
>> >
>> > this causes a strange change:
>> >
>> > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf 
>> > scale=80x60 -t 1 file3.nut
>> >
>> > results in different files before and after this patch. Neither plays
>> > i suspect this is not a bug in the patch but something odd elsewhere
>> > but i dont know
>>
>
>> OK so you're saying there's no bug.
>
> no, i didnt say that
>
>
>> Something changed,
>
> yes
>
>
>> and you're too
>> lazy to investigate,
>
> no, i didnt say that either, nor is that true
>
>
>> but I guess he has all time in the world.
>>
>> So why should he care?
>
> You seem to try to imply that i asked Muhammad to look into this
> or take care of or fix the issue.
>
> please read my reply again its just 3 lines, i have not asked
> Muhammad to do anything. Nor was it intended to imply that.
> It is just a report of some odd findings which were triggered by
> the patch but which quite possibly are unrelated to the patch excpt
> by coincidence.

Probably this patch generates race condition. The silence of tsan is
artificial, the similar patch in avcodec cannot make tsan warning
silence. Currently I'm reworking on this.
My plan is to merge implementation of slice threading in avutil.

Thank's.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example

[Jun Zhao]

V2: re-work with new hw decoding API.
From 0e4d230ae4c98949a962c6bbdad31d216b54bb6a Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 21 Mar 2017 11:04:41 +0800
Subject: [V2] examples/vaapi_dec: Add a VA-API hwaccel decoding example.

Add a VA-API hwaccel decoding example.

Signed-off-by: Liu, Kaixuan 
Signed-off-by: Jun Zhao 
---
 doc/examples/vaapi_dec.c | 266 +++
 1 file changed, 266 insertions(+)
 create mode 100644 doc/examples/vaapi_dec.c

diff --git a/doc/examples/vaapi_dec.c b/doc/examples/vaapi_dec.c
new file mode 100644
index 00..01320a3b71
--- /dev/null
+++ b/doc/examples/vaapi_dec.c
@@ -0,0 +1,266 @@
+/*
+ * Video Acceleration API (video decoding) decode sample
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * @file
+ * Intel VAAPI-accelerated decoding example.
+ *
+ * @example vaapi_dec.c
+ * This example shows how to do VAAPI-accelerated decoding with output
+ * frames from the VAAPI video surfaces.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static AVBufferRef *hw_device_ctx = NULL;
+FILE *output_file = NULL;
+
+int decoder_init_vaapi(AVCodecContext *ctx)
+{
+int err = 0;
+const char *dev_name = "/dev/dri/renderD128";
+
+if ((err = av_hwdevice_ctx_create(_device_ctx, AV_HWDEVICE_TYPE_VAAPI,
+  dev_name, NULL, 0)) < 0) {
+fprintf(stderr, "Failed to create a VAAPI device.\n");
+return err;
+}
+ctx->hw_device_ctx = av_buffer_ref(hw_device_ctx);
+
+return err;
+}
+
+static enum AVPixelFormat get_vaapi_format(AVCodecContext *ctx,
+   const enum AVPixelFormat *pix_fmts)
+{
+const enum AVPixelFormat *p;
+
+for (p = pix_fmts; *p != -1; p++) {
+if (*p == AV_PIX_FMT_VAAPI)
+return *p;
+}
+
+return AV_PIX_FMT_NONE;
+}
+
+int retrieve_data(AVFrame *input)
+{
+AVFrame *output = NULL;
+int err;
+av_assert0(input->format == AV_PIX_FMT_VAAPI);
+
+if (!(output = av_frame_alloc()))
+return AVERROR(ENOMEM);
+/* default output nv12 */
+output->format = AV_PIX_FMT_NV12;
+if ((err = av_hwframe_transfer_data(output, input, 0)) < 0) {
+fprintf(stderr, "Failed to transfer data to output frame: %d.\n", err);
+goto fail;
+}
+
+if ((err = av_frame_copy_props(output, input)) < 0) {
+av_frame_unref(output);
+goto fail;
+}
+
+av_frame_unref(input);
+av_frame_move_ref(input, output);
+av_frame_free();
+return 0;
+
+fail:
+av_frame_free();
+return err;
+}
+
+int write_frame(AVFrame *frame)
+{
+int idx, size;
+int width = frame->width;
+int height = frame->height;
+
+av_assert0(frame && frame->data[0] && output_file);
+
+for (idx = 0; idx < height; idx++) {
+if ((size = fwrite(frame->data[0] + idx*frame->linesize[0],
+   1, width, output_file)) < 0) {
+fprintf(stderr, "Dump Y to file error.\n");
+return -1;
+}
+}
+
+height >>= 1;
+for (idx = 0; idx < height; idx++) {
+if ((size = fwrite(frame->data[1] + idx*frame->linesize[1],
+   1, width, output_file)) < 0) {
+fprintf(stderr, "Dump UV to file error.\n");
+return -1;
+}
+}
+
+return 0;
+}
+
+int decode_write(AVCodecContext *avctx, AVPacket packet, int flush)
+{
+AVFrame *frame = NULL;
+int ret = 0;
+
+ret = avcodec_send_packet(avctx, );
+if (ret < 0 && ret != AVERROR_EOF)
+return 0;
+
+if (!(frame = av_frame_alloc()))
+return AVERROR(ENOMEM);
+
+ret = avcodec_receive_frame(avctx, frame);
+if (ret >= 0) {
+/* retrieve data from GPU to CPU */
+if ((ret = retrieve_data(frame)) < 0)
+goto fail;
+
+if ((ret = write_frame(frame)) < 0)
+goto fail;
+} else if (flush == 0) {
+ret = 0;
+}
+
+fail:
+av_frame_free();
+return ret;
+}
+
+/* flush the decoder */
+int flush(AVCodecContext *avctx)
+{
+AVPacket packet;
+int ret = 0;
+
+

[FFmpeg-devel] [PATCH] movenc:adds keywords metadata

[Kieran O Leary]

Hi,

A user mentioned in ffmpeg-user (
http://ffmpeg.org/pipermail/ffmpeg-user/2017-July/036571.html)  that they
couldn't write the 'keywords' metadata tag. I tested this patch and it
appears to add the metadata value when using MOV and MP4 as output. I'm
sure I've messed something up so please let me know if I can improve this.

Patch is attached and posted here too:

>From c474a7f5095fc1a84c4bd2d811546a821f6420f6 Mon Sep 17 00:00:00 2001
From: Kieran O'Leary 
Date: Mon, 10 Jul 2017 22:54:56 +0100
Subject: [PATCH] movenc:adds keywords metadata

---
 libavformat/movenc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 88f2f2c..3989ac1 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -3321,6 +3321,7 @@ static int mov_write_ilst_tag(AVIOContext *pb,
MOVMuxContext *mov,
 mov_write_string_metadata(s, pb, "tvsh","show" , 1);
 mov_write_string_metadata(s, pb, "tven","episode_id",1);
 mov_write_string_metadata(s, pb, "tvnn","network"  , 1);
+mov_write_string_metadata(s, pb, "keyw","keywords"  , 1);
 mov_write_int8_metadata  (s, pb, "tves","episode_sort",4);
 mov_write_int8_metadata  (s, pb, "tvsn","season_number",4);
 mov_write_int8_metadata  (s, pb, "stik","media_type",1);
@@ -3543,6 +3544,7 @@ static int mov_write_udta_tag(AVIOContext *pb,
MOVMuxContext *mov,
 mov_write_string_metadata(s, pb_buf, "\251mak", "make",0);
 mov_write_string_metadata(s, pb_buf, "\251mod", "model",   0);
 mov_write_string_metadata(s, pb_buf, "\251xyz", "location",0);
+mov_write_string_metadata(s, pb_buf, "\251key", "keywords",0);
 mov_write_raw_metadata_tag(s, pb_buf, "XMP_", "xmp");
 } else {
 /* iTunes meta data */
-- 
2.7.4


Best,

Kieran O'Leary
IFI Irish Film Archive
From c474a7f5095fc1a84c4bd2d811546a821f6420f6 Mon Sep 17 00:00:00 2001
From: Kieran O'Leary 
Date: Mon, 10 Jul 2017 22:54:56 +0100
Subject: [PATCH] movenc:adds keywords metadata

---
 libavformat/movenc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 88f2f2c..3989ac1 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -3321,6 +3321,7 @@ static int mov_write_ilst_tag(AVIOContext *pb, MOVMuxContext *mov,
 mov_write_string_metadata(s, pb, "tvsh","show" , 1);
 mov_write_string_metadata(s, pb, "tven","episode_id",1);
 mov_write_string_metadata(s, pb, "tvnn","network"  , 1);
+mov_write_string_metadata(s, pb, "keyw","keywords"  , 1);
 mov_write_int8_metadata  (s, pb, "tves","episode_sort",4);
 mov_write_int8_metadata  (s, pb, "tvsn","season_number",4);
 mov_write_int8_metadata  (s, pb, "stik","media_type",1);
@@ -3543,6 +3544,7 @@ static int mov_write_udta_tag(AVIOContext *pb, MOVMuxContext *mov,
 mov_write_string_metadata(s, pb_buf, "\251mak", "make",0);
 mov_write_string_metadata(s, pb_buf, "\251mod", "model",   0);
 mov_write_string_metadata(s, pb_buf, "\251xyz", "location",0);
+mov_write_string_metadata(s, pb_buf, "\251key", "keywords",0);
 mov_write_raw_metadata_tag(s, pb_buf, "XMP_", "xmp");
 } else {
 /* iTunes meta data */
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] pthread_frame: save the FF_DEBUG_THREADS option in PerThreadContext.

[Ronald S. Bultje]

Hi,

On Mon, Jul 10, 2017 at 1:24 PM, Wan-Teh Chang  wrote:

> Add the debug_threads boolean field to PerThreadContext. For
> PerThreadContext *p, p->debug_threads records whether the
> FF_DEBUG_THREADS bit is set in p->avctx->debug, and p->debug_threads and
> p->avctx->debug are kept in sync. The debug_threads field is defined as
> an atomic_int to allow atomic read by another thread in
> ff_thread_await_progress().
>
> This fixes the tsan warning that
> 2e664b9c1e73c80aab91070c1eb7676f04bdd12d attempted to fix:
>
> WARNING: ThreadSanitizer: data race (pid=452658)
>   Write of size 4 at 0x7b640003f4fc by main thread (mutexes: write
> M248499):
> #0 update_context_from_user [..]/libavcodec/pthread_frame.c:335:19 (
> 5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859)
> [..]
>   Previous read of size 4 at 0x7b640003f4fc by thread T130 (mutexes: write
> M248502, write M248500):
> #0 ff_thread_await_progress [..]/libavcodec/pthread_frame.c:591:26 (
> 5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1)
>
> Signed-off-by: Wan-Teh Chang 
> ---
>  libavcodec/pthread_frame.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)


I think this looks OK, thanks. I'll leave it out for a day or so for others
to review before I merge.

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[James Almer]

On 7/10/2017 5:53 AM, wm4 wrote:
> On Sat, 8 Jul 2017 01:45:06 +0200
> Michael Niedermayer  wrote:
> 
>> On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
>>> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
>>> uses distict mutex/cond. Also let main thread help running jobs.
>>>
>>> Benchmark using afir with threads=5 and 4096 taps fir:
>>> channels=1:
>>> old:
>>> 1849650 decicycles in afir_execute,   2 runs,  0 skips
>>> 1525719 decicycles in afir_execute,1024 runs,  0 skips
>>> 1546032 decicycles in afir_execute,   16356 runs, 28 skips
>>> new:
>>> 1495525 decicycles in afir_execute,   2 runs,  0 skips
>>>  968897 decicycles in afir_execute,1024 runs,  0 skips
>>>  941286 decicycles in afir_execute,   16384 runs,  0 skips
>>>
>>> channels=2:
>>> old:
>>> 3135485 decicycles in afir_execute,   2 runs,  0 skips
>>> 1967158 decicycles in afir_execute,1024 runs,  0 skips
>>> 1802430 decicycles in afir_execute,   16364 runs, 20 skips
>>> new:
>>> 1864750 decicycles in afir_execute,   2 runs,  0 skips
>>> 1437792 decicycles in afir_execute,1024 runs,  0 skips
>>> 1183963 decicycles in afir_execute,   16382 runs,  2 skips
>>>
>>> channels=4:
>>> old:
>>> 4879925 decicycles in afir_execute,   2 runs,  0 skips
>>> 3557950 decicycles in afir_execute,1022 runs,  2 skips
>>> 3206843 decicycles in afir_execute,   16379 runs,  5 skips
>>> new:
>>> 2962320 decicycles in afir_execute,   2 runs,  0 skips
>>> 2450430 decicycles in afir_execute,1024 runs,  0 skips
>>> 2446219 decicycles in afir_execute,   16383 runs,  1 skips
>>>
>>> channels=8:
>>> old:
>>> 6032455 decicycles in afir_execute,   2 runs,  0 skips
>>> 4838614 decicycles in afir_execute,1023 runs,  1 skips
>>> 4720760 decicycles in afir_execute,   16369 runs, 15 skips
>>> new:
>>> 5228150 decicycles in afir_execute,   2 runs,  0 skips
>>> 4592129 decicycles in afir_execute,1023 runs,  1 skips
>>> 4469067 decicycles in afir_execute,   16383 runs,  1 skips  
>>
>> this causes a strange change:
>>
>> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf scale=80x60 
>> -t 1 file3.nut
>>
>> results in different files before and after this patch. Neither plays
>> i suspect this is not a bug in the patch but something odd elsewhere
>> but i dont know
> 
> OK so you're saying there's no bug. Something changed, and you're too
> lazy to investigate, but I guess he has all time in the world.
> 
> So why should he care?

Tone it down already. You're being unnecessarily aggressive in a lot of
your recent emails.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[Michael Niedermayer]

On Mon, Jul 10, 2017 at 10:53:42AM +0200, wm4 wrote:
> On Sat, 8 Jul 2017 01:45:06 +0200
> Michael Niedermayer  wrote:
> 
> > On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
> > > Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> > > uses distict mutex/cond. Also let main thread help running jobs.
> > > 
> > > Benchmark using afir with threads=5 and 4096 taps fir:
> > > channels=1:
> > > old:
> > > 1849650 decicycles in afir_execute,   2 runs,  0 skips
> > > 1525719 decicycles in afir_execute,1024 runs,  0 skips
> > > 1546032 decicycles in afir_execute,   16356 runs, 28 skips
> > > new:
> > > 1495525 decicycles in afir_execute,   2 runs,  0 skips
> > >  968897 decicycles in afir_execute,1024 runs,  0 skips
> > >  941286 decicycles in afir_execute,   16384 runs,  0 skips
> > > 
> > > channels=2:
> > > old:
> > > 3135485 decicycles in afir_execute,   2 runs,  0 skips
> > > 1967158 decicycles in afir_execute,1024 runs,  0 skips
> > > 1802430 decicycles in afir_execute,   16364 runs, 20 skips
> > > new:
> > > 1864750 decicycles in afir_execute,   2 runs,  0 skips
> > > 1437792 decicycles in afir_execute,1024 runs,  0 skips
> > > 1183963 decicycles in afir_execute,   16382 runs,  2 skips
> > > 
> > > channels=4:
> > > old:
> > > 4879925 decicycles in afir_execute,   2 runs,  0 skips
> > > 3557950 decicycles in afir_execute,1022 runs,  2 skips
> > > 3206843 decicycles in afir_execute,   16379 runs,  5 skips
> > > new:
> > > 2962320 decicycles in afir_execute,   2 runs,  0 skips
> > > 2450430 decicycles in afir_execute,1024 runs,  0 skips
> > > 2446219 decicycles in afir_execute,   16383 runs,  1 skips
> > > 
> > > channels=8:
> > > old:
> > > 6032455 decicycles in afir_execute,   2 runs,  0 skips
> > > 4838614 decicycles in afir_execute,1023 runs,  1 skips
> > > 4720760 decicycles in afir_execute,   16369 runs, 15 skips
> > > new:
> > > 5228150 decicycles in afir_execute,   2 runs,  0 skips
> > > 4592129 decicycles in afir_execute,1023 runs,  1 skips
> > > 4469067 decicycles in afir_execute,   16383 runs,  1 skips  
> > 
> > this causes a strange change:
> > 
> > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf 
> > scale=80x60 -t 1 file3.nut
> > 
> > results in different files before and after this patch. Neither plays
> > i suspect this is not a bug in the patch but something odd elsewhere
> > but i dont know
> 

> OK so you're saying there's no bug.

no, i didnt say that


> Something changed,

yes


> and you're too
> lazy to investigate,

no, i didnt say that either, nor is that true


> but I guess he has all time in the world.
> 
> So why should he care?

You seem to try to imply that i asked Muhammad to look into this
or take care of or fix the issue.

please read my reply again its just 3 lines, i have not asked
Muhammad to do anything. Nor was it intended to imply that.
It is just a report of some odd findings which were triggered by
the patch but which quite possibly are unrelated to the patch excpt
by coincidence.

Thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/aacpsdsp_template: Fixes integer overflow in ps_add_squares_c()

[Michael Niedermayer]

On Mon, Jul 10, 2017 at 10:37:46AM +0200, wm4 wrote:
> On Sun, 9 Jul 2017 22:03:21 +0200
> Reimar Döffinger  wrote:
> 
> > On 09.07.2017, at 16:08, "Ronald S. Bultje"  wrote:
> > > Hi,
> > > 
> > > On Sun, Jul 9, 2017 at 4:39 AM, Reimar Döffinger 
> > > 
> > > wrote:
> > >   
> > >> On 09.07.2017, at 02:52, "Ronald S. Bultje"  wrote:  
> > >>> On Sat, Jul 8, 2017 at 5:17 PM, Michael Niedermayer  
> > >>   
> > >>> wrote:
> > >>>   
> >  
> >  Does anyone object to this patch ?
> >  Or does anyone have a better idea on how to fix this ?
> >  if not id like to apply it  
> > >>> 
> > >>> 
> > >>> I think Rostislav's point is: why fix it, if it can only happen with
> > >>> corrupt input? The before and after situation is identical: garbage in,
> > >>> garbage out. If the compiler does funny things that makes the garbage
> > >>> slightly differently bad, is that really so devilishly bad? It's still
> > >>> garbage. Is anything improved by this?  
> > >> 
> > >> The way C works, you MUST assume any undefined behaviour can at any point
> > >> [..] become exploitable.[..] If you don't like that, C is the wrong
> > >> language to use.  
> > > 
> > > 
> > > I think I've read "the boy who cried wolf" a few too many times to my 
> > > kids,
> > > but the form of this discussion is currently too polarizing/political for
> > > my taste.  
> > 
> > That is my reading of the C standard, is that political or even just 
> > controversial?
> > I mean of course you can ignore standards (see MPEG-4 ASP, and in some ways 
> > that was actually fairly reasonable thing to do at the time), and I don't 
> > fix every undefined behaviour case in my code when I can't think of any 
> > reasonable solution.
> > So there is a cost-benefit discussion in principle.
> > I believe the cost of not fixing undefined behaviour, just by virtue of 
> > going outside what the standard guarantees should be considered fairly high.
> > That is an opinion, but is there any disagreement that undefined behaviour 
> > is at least an issue of some degree?
> > If we can agree on that, then the question would only be how much 
> > effort/code ugliness is reasonable.
> > There is also the point (which I hope I mentioned in the parts cut out) 
> > that just making sure that these cases are not already exploitable right 
> > now with the current compiler can in many cases be quite a pain (and does 
> > not have tool support), so I think fixing it would often be the 
> > lowest-effort method.
> 
> The controversial thing is actually the SUINT nonsense. A type is
> either signed or unsigned, but not both incompletely intransparent

> ways. Michael keeps adding them even though many are against it. 

It is extreemly rude from you to make this claim.
When in fact i try my best to respect every maitainer and authors
preferrance and never add it when people object.

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] checkasm: add a g722dsp test

[James Almer]

On 7/5/2017 4:44 PM, James Almer wrote:
> Signed-off-by: James Almer 
> ---
>  tests/checkasm/Makefile   |  1 +
>  tests/checkasm/checkasm.c |  3 +++
>  tests/checkasm/checkasm.h |  1 +
>  tests/checkasm/g722dsp.c  | 66 
> +++
>  tests/fate/checkasm.mak   |  1 +
>  5 files changed, 72 insertions(+)
>  create mode 100644 tests/checkasm/g722dsp.c
> 
> diff --git a/tests/checkasm/Makefile b/tests/checkasm/Makefile
> index 60e80ab738..184e981754 100644
> --- a/tests/checkasm/Makefile
> +++ b/tests/checkasm/Makefile
> @@ -5,6 +5,7 @@ AVCODECOBJS-$(CONFIG_BLOCKDSP)  += blockdsp.o
>  AVCODECOBJS-$(CONFIG_BSWAPDSP)  += bswapdsp.o
>  AVCODECOBJS-$(CONFIG_FLACDSP)   += flacdsp.o
>  AVCODECOBJS-$(CONFIG_FMTCONVERT)+= fmtconvert.o
> +AVCODECOBJS-$(CONFIG_G722DSP)   += g722dsp.o
>  AVCODECOBJS-$(CONFIG_H264DSP)   += h264dsp.o
>  AVCODECOBJS-$(CONFIG_H264PRED)  += h264pred.o
>  AVCODECOBJS-$(CONFIG_H264QPEL)  += h264qpel.o
> diff --git a/tests/checkasm/checkasm.c b/tests/checkasm/checkasm.c
> index 29f201b1b3..9173ed19d9 100644
> --- a/tests/checkasm/checkasm.c
> +++ b/tests/checkasm/checkasm.c
> @@ -90,6 +90,9 @@ static const struct {
>  #if CONFIG_FMTCONVERT
>  { "fmtconvert", checkasm_check_fmtconvert },
>  #endif
> +#if CONFIG_G722DSP
> +{ "g722dsp", checkasm_check_g722dsp },
> +#endif
>  #if CONFIG_H264DSP
>  { "h264dsp", checkasm_check_h264dsp },
>  #endif
> diff --git a/tests/checkasm/checkasm.h b/tests/checkasm/checkasm.h
> index fa51e71e4b..3165b21086 100644
> --- a/tests/checkasm/checkasm.h
> +++ b/tests/checkasm/checkasm.h
> @@ -42,6 +42,7 @@ void checkasm_check_fixed_dsp(void);
>  void checkasm_check_flacdsp(void);
>  void checkasm_check_float_dsp(void);
>  void checkasm_check_fmtconvert(void);
> +void checkasm_check_g722dsp(void);
>  void checkasm_check_h264dsp(void);
>  void checkasm_check_h264pred(void);
>  void checkasm_check_h264qpel(void);
> diff --git a/tests/checkasm/g722dsp.c b/tests/checkasm/g722dsp.c
> new file mode 100644
> index 00..2120c5fd58
> --- /dev/null
> +++ b/tests/checkasm/g722dsp.c
> @@ -0,0 +1,66 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * FFmpeg 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with FFmpeg; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include 
> +#include "checkasm.h"
> +#include "libavcodec/g722.h"
> +#include "libavcodec/g722dsp.h"
> +#include "libavcodec/mathops.h"
> +
> +#define randomize_buffers()   \
> +do {  \
> +int i;\
> +for (i = 0; i < PREV_SAMPLES_BUF_SIZE; i++) { \
> +int16_t r = sign_extend(rnd(), 16);   \
> +src0[i] = r;  \
> +src1[i] = r;  \
> +} \
> +} while (0)
> +
> +static void check_qmf(int16_t *src0, int16_t *src1, int dst0[2], int 
> dst1[2]) {
> +const int16_t *tmp0 = src0;
> +const int16_t *tmp1 = src1;
> +int i;
> +
> +declare_func(void, const int16_t *prev_samples, int xout[2]);
> +
> +randomize_buffers();
> +for (i = 0; i < PREV_SAMPLES_BUF_SIZE - 24; i++) {
> +call_ref(tmp0++, dst0);
> +call_new(tmp1++, dst1);
> +if (memcmp(src0, src1, PREV_SAMPLES_BUF_SIZE * sizeof(int16_t)) ||
> +memcmp(dst0, dst1, sizeof(dst0)))
> +fail();
> +}
> +bench_new(src1, dst1);
> +}
> +
> +void checkasm_check_g722dsp(void)
> +{
> +int16_t src0[PREV_SAMPLES_BUF_SIZE];
> +int16_t src1[PREV_SAMPLES_BUF_SIZE];
> +int dst0[2], dst1[2];
> +G722DSPContext h;
> +
> +ff_g722dsp_init();
> +
> +if (check_func(h.apply_qmf, "g722_apply_qmf"))
> +check_qmf(src0, src1, dst0, dst1);
> +
> +report("apply_qmf");
> +}
> diff --git a/tests/fate/checkasm.mak b/tests/fate/checkasm.mak
> index 61ce6cd74f..e473b3ec0c 100644
> --- a/tests/fate/checkasm.mak
> +++ b/tests/fate/checkasm.mak
> @@ -7,6 +7,7 @@ FATE_CHECKASM = fate-checkasm-aacpsdsp
>   \
>  fate-checkasm-flacdsp   \
>  

Re: [FFmpeg-devel] [PATCH] pthread_frame: save the FF_DEBUG_THREADS option in FrameThreadContext.

[Wan-Teh Chang]

On Sat, Jul 8, 2017 at 3:49 PM, Michael Niedermayer
 wrote:
> On Sat, Jul 08, 2017 at 03:38:11PM -0700, Wan-Teh Chang wrote:
>> Hi Ronald,
>>
>> On Sat, Jul 8, 2017 at 2:33 PM, Ronald S. Bultje  wrote:
>> >
>> > I can see the design from the patch.
>> >
>> > What's missing is a justification for the downside of the design, which is
>> > that updates to this variable by the user are no longer propagated to the
>> > worker threads.
>>
>> My justification is the YAGNI principle.
>>
>> Although the current code allows the FF_DEBUG_THREADS option to be
>> toggled dynamically, I believe that was not intended, and I believe
>> nobody actually does that. In my (admittedly limited) code search, I
>> only see the FF_DEBUG_THREADS option set via the -debug thread_ops
>> command-line option.
>
> ffmpeg (the command line tool) allows changing the AVCodecContext->debug
> value at runtime

Thank you. I abandoned this patch and wrote a new patch. Please see
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213501.html.

Wan-Teh Chang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] pthread_frame: save the FF_DEBUG_THREADS option in PerThreadContext.

[Wan-Teh Chang]

Add the debug_threads boolean field to PerThreadContext. For
PerThreadContext *p, p->debug_threads records whether the
FF_DEBUG_THREADS bit is set in p->avctx->debug, and p->debug_threads and
p->avctx->debug are kept in sync. The debug_threads field is defined as
an atomic_int to allow atomic read by another thread in
ff_thread_await_progress().

This fixes the tsan warning that
2e664b9c1e73c80aab91070c1eb7676f04bdd12d attempted to fix:

WARNING: ThreadSanitizer: data race (pid=452658)
  Write of size 4 at 0x7b640003f4fc by main thread (mutexes: write M248499):
#0 update_context_from_user [..]/libavcodec/pthread_frame.c:335:19 
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe73859)
[..]
  Previous read of size 4 at 0x7b640003f4fc by thread T130 (mutexes: write 
M248502, write M248500):
#0 ff_thread_await_progress [..]/libavcodec/pthread_frame.c:591:26 
(5ab42bb1a6f4b068d7863dabe9b2bacc+0xe749a1)

Signed-off-by: Wan-Teh Chang 
---
 libavcodec/pthread_frame.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 363b139f71..2cea232a36 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -107,6 +107,8 @@ typedef struct PerThreadContext {
 
 int hwaccel_serializing;
 int async_serializing;
+
+atomic_int debug_threads;   ///< Set if the FF_DEBUG_THREADS option is 
set.
 } PerThreadContext;
 
 /**
@@ -398,6 +400,9 @@ static int submit_packet(PerThreadContext *p, 
AVCodecContext *user_avctx,
 pthread_mutex_unlock(>mutex);
 return ret;
 }
+atomic_store_explicit(>debug_threads,
+  (p->avctx->debug & FF_DEBUG_THREADS) != 0,
+  memory_order_relaxed);
 
 release_delayed_buffers(p);
 
@@ -566,7 +571,7 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int 
field)
 p = f->owner[field]->internal->thread_ctx;
 
 pthread_mutex_lock(>progress_mutex);
-if (f->owner[field]->debug_DEBUG_THREADS)
+if (atomic_load_explicit(>debug_threads, memory_order_relaxed))
 av_log(f->owner[field], AV_LOG_DEBUG,
"%p finished %d field %d\n", progress, n, field);
 
@@ -588,7 +593,7 @@ void ff_thread_await_progress(ThreadFrame *f, int n, int 
field)
 p = f->owner[field]->internal->thread_ctx;
 
 pthread_mutex_lock(>progress_mutex);
-if (f->owner[field]->debug_DEBUG_THREADS)
+if (atomic_load_explicit(>debug_threads, memory_order_relaxed))
 av_log(f->owner[field], AV_LOG_DEBUG,
"thread awaiting %d field %d from %p\n", n, field, progress);
 while (atomic_load_explicit([field], memory_order_relaxed) < n)
@@ -823,6 +828,8 @@ int ff_frame_thread_init(AVCodecContext *avctx)
 
 if (err) goto error;
 
+atomic_init(>debug_threads, (copy->debug & FF_DEBUG_THREADS) != 0);
+
 err = AVERROR(pthread_create(>thread, NULL, frame_worker_thread, 
p));
 p->thread_init= !err;
 if(!p->thread_init)
-- 
2.13.2.725.g09c95d1e9-goog

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 10/12] rtspdec: Fix return error

[Derek Buitenhuis]

On 7/6/2017 7:28 PM, Derek Buitenhuis wrote:
> Signed-off-by: Derek Buitenhuis 
> ---
>  libavformat/rtspdec.c | 1 -
>  1 file changed, 1 deletion(-)

Pushing this today if nobody objects.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 10/10] avcodec/dca: avoid using bitstream reader in a non-standard way

[foo86]

Use proper get_bits.h functions instead of directly accessing index.
---
 libavcodec/dca_core.c | 12 +++-
 libavcodec/dca_core.h |  1 +
 libavcodec/dca_xll.c  |  2 +-
 libavcodec/dcadec.h   |  4 ++--
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/libavcodec/dca_core.c b/libavcodec/dca_core.c
index 4a7ea4e3f3..3add9f812b 100644
--- a/libavcodec/dca_core.c
+++ b/libavcodec/dca_core.c
@@ -1804,6 +1804,7 @@ int ff_dca_core_parse(DCACoreDecoder *s, uint8_t *data, 
int size)
 
 if ((ret = init_get_bits8(>gb, data, size)) < 0)
 return ret;
+s->gb_in = s->gb;
 
 if ((ret = parse_frame_header(s)) < 0)
 return ret;
@@ -1831,7 +1832,6 @@ int ff_dca_core_parse_exss(DCACoreDecoder *s, uint8_t 
*data, DCAExssAsset *asset
 {
 AVCodecContext *avctx = s->avctx;
 DCAContext *dca = avctx->priv_data;
-GetBitContext gb = s->gb;
 int exss_mask = asset ? asset->extension_mask : 0;
 int ret = 0, ext = 0;
 
@@ -1843,11 +1843,13 @@ int ff_dca_core_parse_exss(DCACoreDecoder *s, uint8_t 
*data, DCAExssAsset *asset
 ret = parse_xxch_frame(s);
 ext = DCA_EXSS_XXCH;
 } else if (s->xxch_pos) {
-s->gb.index = s->xxch_pos;
+s->gb = s->gb_in;
+skip_bits_long(>gb, s->xxch_pos);
 ret = parse_xxch_frame(s);
 ext = DCA_CSS_XXCH;
 } else if (s->xch_pos) {
-s->gb.index = s->xch_pos;
+s->gb = s->gb_in;
+skip_bits_long(>gb, s->xch_pos);
 ret = parse_xch_frame(s);
 ext = DCA_CSS_XCH;
 }
@@ -1889,8 +1891,8 @@ int ff_dca_core_parse_exss(DCACoreDecoder *s, uint8_t 
*data, DCAExssAsset *asset
 s->ext_audio_mask |= DCA_EXSS_X96;
 }
 } else if (s->x96_pos) {
-s->gb = gb;
-s->gb.index = s->x96_pos;
+s->gb = s->gb_in;
+skip_bits_long(>gb, s->x96_pos);
 if ((ret = parse_x96_frame(s)) < 0) {
 if (ret == AVERROR(ENOMEM) || (avctx->err_recognition & 
AV_EF_EXPLODE))
 return ret;
diff --git a/libavcodec/dca_core.h b/libavcodec/dca_core.h
index cce0ffd7b1..10128d1e32 100644
--- a/libavcodec/dca_core.h
+++ b/libavcodec/dca_core.h
@@ -101,6 +101,7 @@ typedef struct DCADSPData {
 typedef struct DCACoreDecoder {
 AVCodecContext  *avctx;
 GetBitContext   gb;
+GetBitContext   gb_in;
 
 // Bit stream header
 int crc_present;///< CRC present flag
diff --git a/libavcodec/dca_xll.c b/libavcodec/dca_xll.c
index 38a1999fc8..d265cab8df 100644
--- a/libavcodec/dca_xll.c
+++ b/libavcodec/dca_xll.c
@@ -1028,7 +1028,7 @@ static int parse_band_data(DCAXllDecoder *s)
 return ret;
 chs_clear_band_data(s, c, band, seg);
 }
-s->gb.index = navi_pos;
+skip_bits_long(>gb, navi_pos - get_bits_count(>gb));
 }
 navi_ptr++;
 }
diff --git a/libavcodec/dcadec.h b/libavcodec/dcadec.h
index 456f3c433b..9da8d3b444 100644
--- a/libavcodec/dcadec.h
+++ b/libavcodec/dcadec.h
@@ -88,9 +88,9 @@ static inline int ff_dca_check_crc(AVCodecContext *avctx, 
GetBitContext *s,
 
 static inline int ff_dca_seek_bits(GetBitContext *s, int p)
 {
-if (p < s->index || p > s->size_in_bits)
+if (p < get_bits_count(s) || p > s->size_in_bits)
 return -1;
-s->index = p;
+skip_bits_long(s, p - get_bits_count(s));
 return 0;
 }
 
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 08/10] avcodec/dca_parser: avoid use of magic values

[foo86]

Duration computation can be simplified because number of PCM blocks is
only allowed to be a multiple of 8.
---
 libavcodec/dca_parser.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/dca_parser.c b/libavcodec/dca_parser.c
index 390f7975f9..7e99b16bf0 100644
--- a/libavcodec/dca_parser.c
+++ b/libavcodec/dca_parser.c
@@ -25,6 +25,7 @@
 #include "dca.h"
 #include "dca_core.h"
 #include "dca_exss.h"
+#include "dca_lbr.h"
 #include "dca_syncwords.h"
 #include "get_bits.h"
 #include "parser.h"
@@ -214,9 +215,9 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 return AVERROR_INVALIDDATA;
 
 switch (get_bits(, 8)) {
-case 2:
+case DCA_LBR_HEADER_DECODER_INIT:
 pc1->sr_code = get_bits(, 8);
-case 1:
+case DCA_LBR_HEADER_SYNC_ONLY:
 break;
 default:
 return AVERROR_INVALIDDATA;
@@ -267,7 +268,7 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 if (avpriv_dca_parse_core_frame_header(, ) < 0)
 return AVERROR_INVALIDDATA;
 
-*duration = 256 * (h.npcmblocks / 8);
+*duration = h.npcmblocks * DCA_PCMBLOCK_SAMPLES;
 *sample_rate = avpriv_dca_sample_rates[h.sr_code];
 if (*profile != FF_PROFILE_UNKNOWN)
 return 0;
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 03/10] avcodec: add avpriv_dca_parse_core_frame_header()

[foo86]

There are 3 different places where DCA core frame header is parsed:
decoder, parser and demuxer. Each one uses ad-hoc code. Add common core
frame header parsing function that will be used in all places.
---
 libavcodec/dca.c | 60 
 libavcodec/dca.h | 49 ++
 libavcodec/version.h |  4 ++--
 3 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dca.c b/libavcodec/dca.c
index fb796191d6..39f8f3d81c 100644
--- a/libavcodec/dca.c
+++ b/libavcodec/dca.c
@@ -28,7 +28,9 @@
 #include "libavutil/error.h"
 
 #include "dca.h"
+#include "dca_core.h"
 #include "dca_syncwords.h"
+#include "get_bits.h"
 #include "put_bits.h"
 
 const uint32_t avpriv_dca_sample_rates[16] = {
@@ -85,3 +87,61 @@ int avpriv_dca_convert_bitstream(const uint8_t *src, int 
src_size, uint8_t *dst,
 return AVERROR_INVALIDDATA;
 }
 }
+
+int avpriv_dca_parse_core_frame_header(GetBitContext *gb, DCACoreFrameHeader 
*h)
+{
+if (get_bits_long(gb, 32) != DCA_SYNCWORD_CORE_BE)
+return DCA_PARSE_ERROR_SYNC_WORD;
+
+h->normal_frame = get_bits1(gb);
+h->deficit_samples = get_bits(gb, 5) + 1;
+if (h->deficit_samples != DCA_PCMBLOCK_SAMPLES)
+return DCA_PARSE_ERROR_DEFICIT_SAMPLES;
+
+h->crc_present = get_bits1(gb);
+h->npcmblocks = get_bits(gb, 7) + 1;
+if (h->npcmblocks & (DCA_SUBBAND_SAMPLES - 1))
+return DCA_PARSE_ERROR_PCM_BLOCKS;
+
+h->frame_size = get_bits(gb, 14) + 1;
+if (h->frame_size < 96)
+return DCA_PARSE_ERROR_FRAME_SIZE;
+
+h->audio_mode = get_bits(gb, 6);
+if (h->audio_mode >= DCA_AMODE_COUNT)
+return DCA_PARSE_ERROR_AMODE;
+
+h->sr_code = get_bits(gb, 4);
+if (!avpriv_dca_sample_rates[h->sr_code])
+return DCA_PARSE_ERROR_SAMPLE_RATE;
+
+h->br_code = get_bits(gb, 5);
+if (get_bits1(gb))
+return DCA_PARSE_ERROR_RESERVED_BIT;
+
+h->drc_present = get_bits1(gb);
+h->ts_present = get_bits1(gb);
+h->aux_present = get_bits1(gb);
+h->hdcd_master = get_bits1(gb);
+h->ext_audio_type = get_bits(gb, 3);
+h->ext_audio_present = get_bits1(gb);
+h->sync_ssf = get_bits1(gb);
+h->lfe_present = get_bits(gb, 2);
+if (h->lfe_present == DCA_LFE_FLAG_INVALID)
+return DCA_PARSE_ERROR_LFE_FLAG;
+
+h->predictor_history = get_bits1(gb);
+if (h->crc_present)
+skip_bits(gb, 16);
+h->filter_perfect = get_bits1(gb);
+h->encoder_rev = get_bits(gb, 4);
+h->copy_hist = get_bits(gb, 2);
+h->pcmr_code = get_bits(gb, 3);
+if (!ff_dca_bits_per_sample[h->pcmr_code])
+return DCA_PARSE_ERROR_PCM_RES;
+
+h->sumdiff_front = get_bits1(gb);
+h->sumdiff_surround = get_bits1(gb);
+h->dn_code = get_bits(gb, 4);
+return 0;
+}
diff --git a/libavcodec/dca.h b/libavcodec/dca.h
index 1d10de4b94..cf6204e554 100644
--- a/libavcodec/dca.h
+++ b/libavcodec/dca.h
@@ -32,6 +32,49 @@
 #include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 
+#include "get_bits.h"
+
+#define DCA_CORE_FRAME_HEADER_SIZE  18
+
+enum DCAParseError {
+DCA_PARSE_ERROR_SYNC_WORD   = -1,
+DCA_PARSE_ERROR_DEFICIT_SAMPLES = -2,
+DCA_PARSE_ERROR_PCM_BLOCKS  = -3,
+DCA_PARSE_ERROR_FRAME_SIZE  = -4,
+DCA_PARSE_ERROR_AMODE   = -5,
+DCA_PARSE_ERROR_SAMPLE_RATE = -6,
+DCA_PARSE_ERROR_RESERVED_BIT= -7,
+DCA_PARSE_ERROR_LFE_FLAG= -8,
+DCA_PARSE_ERROR_PCM_RES = -9
+};
+
+typedef struct DCACoreFrameHeader {
+uint8_t normal_frame;   ///< Frame type
+uint8_t deficit_samples;///< Deficit sample count
+uint8_t crc_present;///< CRC present flag
+uint8_t npcmblocks; ///< Number of PCM sample blocks
+uint16_tframe_size; ///< Primary frame byte size
+uint8_t audio_mode; ///< Audio channel arrangement
+uint8_t sr_code;///< Core audio sampling frequency
+uint8_t br_code;///< Transmission bit rate
+uint8_t drc_present;///< Embedded dynamic range flag
+uint8_t ts_present; ///< Embedded time stamp flag
+uint8_t aux_present;///< Auxiliary data flag
+uint8_t hdcd_master;///< HDCD mastering flag
+uint8_t ext_audio_type; ///< Extension audio descriptor flag
+uint8_t ext_audio_present;  ///< Extended coding flag
+uint8_t sync_ssf;   ///< Audio sync word insertion flag
+uint8_t lfe_present;///< Low frequency effects flag
+uint8_t predictor_history;  ///< Predictor history flag switch
+uint8_t filter_perfect; ///< Multirate interpolator switch
+uint8_t encoder_rev;///< Encoder software revision
+uint8_t copy_hist;  ///< Copy history
+uint8_t pcmr_code;  ///< Source PCM resolution
+uint8_t 

[FFmpeg-devel] [PATCH 06/10] avformat/dtsdec: switch to common frame header parsing function

[foo86]

This makes probing for regular DTS more strict because more header
fields are checked and values not supported by decoder are now rejected.

Also fixes an issue original code had with 14-bit streams: 96 bits of
header were expected, however only 84 bits were converted, which was not
enough to parse LFE flag.
---
 libavformat/dtsdec.c | 39 +--
 1 file changed, 9 insertions(+), 30 deletions(-)

diff --git a/libavformat/dtsdec.c b/libavformat/dtsdec.c
index 8c985b8877..6e0048f9bc 100644
--- a/libavformat/dtsdec.c
+++ b/libavformat/dtsdec.c
@@ -35,13 +35,13 @@ static int dts_probe(AVProbeData *p)
 uint32_t state = -1;
 int markers[4*16] = {0};
 int exss_markers = 0, exss_nextpos = 0;
-int sum, max, pos, i;
+int sum, max, pos, ret, i;
 int64_t diff = 0;
-uint8_t hdr[12 + AV_INPUT_BUFFER_PADDING_SIZE] = { 0 };
+uint8_t hdr[DCA_CORE_FRAME_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE] = { 
0 };
 
 for (pos = FFMIN(4096, p->buf_size); pos < p->buf_size - 2; pos += 2) {
-int marker, sample_blocks, sample_rate, sr_code, framesize;
-int lfe, wide_hdr, hdr_size;
+int marker, wide_hdr, hdr_size, framesize;
+DCACoreFrameHeader h;
 GetBitContext gb;
 
 bufp = buf = p->buf + pos;
@@ -98,36 +98,15 @@ static int dts_probe(AVProbeData *p)
 else
 continue;
 
-if (avpriv_dca_convert_bitstream(buf-2, 12, hdr, 12) < 0)
+if ((ret = avpriv_dca_convert_bitstream(buf - 2, 
DCA_CORE_FRAME_HEADER_SIZE,
+hdr, 
DCA_CORE_FRAME_HEADER_SIZE)) < 0)
 continue;
-
-init_get_bits(, hdr, 96);
-skip_bits_long(, 39);
-
-sample_blocks = get_bits(, 7) + 1;
-if (sample_blocks < 8)
+if (init_get_bits8(, hdr, ret) < 0)
 continue;
-
-framesize = get_bits(, 14) + 1;
-if (framesize < 95)
-continue;
-
-skip_bits(, 6);
-sr_code = get_bits(, 4);
-sample_rate = avpriv_dca_sample_rates[sr_code];
-if (sample_rate == 0)
-continue;
-
-get_bits(, 5);
-if (get_bits(, 1))
-continue;
-
-skip_bits_long(, 9);
-lfe = get_bits(, 2);
-if (lfe > 2)
+if (avpriv_dca_parse_core_frame_header(, ) < 0)
 continue;
 
-marker += 4* sr_code;
+marker += 4 * h.sr_code;
 
 markers[marker] ++;
 }
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 07/10] avcodec/dca_parser: export profile information

[foo86]

Permits applications to access DTS profile information without having to
decode a frame.
---
 libavcodec/dca_parser.c | 44 +++-
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/libavcodec/dca_parser.c b/libavcodec/dca_parser.c
index 6107358773..390f7975f9 100644
--- a/libavcodec/dca_parser.c
+++ b/libavcodec/dca_parser.c
@@ -23,6 +23,7 @@
  */
 
 #include "dca.h"
+#include "dca_core.h"
 #include "dca_exss.h"
 #include "dca_syncwords.h"
 #include "get_bits.h"
@@ -189,19 +190,19 @@ static av_cold int dca_parse_init(AVCodecParserContext *s)
 }
 
 static int dca_parse_params(DCAParseContext *pc1, const uint8_t *buf,
-int buf_size, int *duration, int *sample_rate)
+int buf_size, int *duration, int *sample_rate,
+int *profile)
 {
+DCAExssAsset *asset = >exss.assets[0];
 GetBitContext gb;
 DCACoreFrameHeader h;
 uint8_t hdr[DCA_CORE_FRAME_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE] = { 
0 };
-int ret;
+int ret, frame_size;
 
 if (buf_size < DCA_CORE_FRAME_HEADER_SIZE)
 return AVERROR_INVALIDDATA;
 
 if (AV_RB32(buf) == DCA_SYNCWORD_SUBSTREAM) {
-DCAExssAsset *asset = >exss.assets[0];
-
 if ((ret = ff_dca_exss_parse(>exss, buf, buf_size)) < 0)
 return ret;
 
@@ -226,6 +227,7 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 
 *sample_rate = ff_dca_sampling_freqs[pc1->sr_code];
 *duration = 1024 << ff_dca_freq_ranges[pc1->sr_code];
+*profile = FF_PROFILE_DTS_EXPRESS;
 return 0;
 }
 
@@ -250,6 +252,7 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 
 *sample_rate = asset->max_sample_rate;
 *duration = (1 + (*sample_rate > 96000)) << nsamples_log2;
+*profile = FF_PROFILE_DTS_HD_MA;
 return 0;
 }
 
@@ -266,6 +269,37 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 
 *duration = 256 * (h.npcmblocks / 8);
 *sample_rate = avpriv_dca_sample_rates[h.sr_code];
+if (*profile != FF_PROFILE_UNKNOWN)
+return 0;
+
+*profile = FF_PROFILE_DTS;
+if (h.ext_audio_present) {
+switch (h.ext_audio_type) {
+case DCA_EXT_AUDIO_XCH:
+case DCA_EXT_AUDIO_XXCH:
+*profile = FF_PROFILE_DTS_ES;
+break;
+case DCA_EXT_AUDIO_X96:
+*profile = FF_PROFILE_DTS_96_24;
+break;
+}
+}
+
+frame_size = FFALIGN(h.frame_size, 4);
+if (buf_size - 4 < frame_size)
+return 0;
+
+buf  += frame_size;
+buf_size -= frame_size;
+if (AV_RB32(buf) != DCA_SYNCWORD_SUBSTREAM)
+return 0;
+if (ff_dca_exss_parse(>exss, buf, buf_size) < 0)
+return 0;
+
+if (asset->extension_mask & DCA_EXSS_XLL)
+*profile = FF_PROFILE_DTS_HD_MA;
+else if (asset->extension_mask & (DCA_EXSS_XBR | DCA_EXSS_XXCH | 
DCA_EXSS_X96))
+*profile = FF_PROFILE_DTS_HD_HRA;
 
 return 0;
 }
@@ -298,7 +332,7 @@ static int dca_parse(AVCodecParserContext *s, 
AVCodecContext *avctx,
 }
 
 /* read the duration and sample rate from the frame header */
-if (!dca_parse_params(pc1, buf, buf_size, , _rate)) {
+if (!dca_parse_params(pc1, buf, buf_size, , _rate, 
>profile)) {
 if (!avctx->sample_rate)
 avctx->sample_rate = sample_rate;
 s->duration = av_rescale(duration, avctx->sample_rate, sample_rate);
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 01/10] avcodec/dca: move some enumeration typedefs into headers

[foo86]

These values will be used by the parser. Prefix them with DCA_
appropriately.
---
 libavcodec/dca_core.c | 68 +++
 libavcodec/dca_core.h | 28 +
 libavcodec/dca_lbr.c  |  9 ++-
 libavcodec/dca_lbr.h  |  5 
 4 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/libavcodec/dca_core.c b/libavcodec/dca_core.c
index 36040f6f9d..16210b89f8 100644
--- a/libavcodec/dca_core.c
+++ b/libavcodec/dca_core.c
@@ -35,35 +35,7 @@ enum HeaderType {
 HEADER_XXCH
 };
 
-enum AudioMode {
-AMODE_MONO, // Mode 0: A (mono)
-AMODE_MONO_DUAL,// Mode 1: A + B (dual mono)
-AMODE_STEREO,   // Mode 2: L + R (stereo)
-AMODE_STEREO_SUMDIFF,   // Mode 3: (L+R) + (L-R) (sum-diff)
-AMODE_STEREO_TOTAL, // Mode 4: LT + RT (left and right total)
-AMODE_3F,   // Mode 5: C + L + R
-AMODE_2F1R, // Mode 6: L + R + S
-AMODE_3F1R, // Mode 7: C + L + R + S
-AMODE_2F2R, // Mode 8: L + R + SL + SR
-AMODE_3F2R, // Mode 9: C + L + R + SL + SR
-
-AMODE_COUNT
-};
-
-enum ExtAudioType {
-EXT_AUDIO_XCH   = 0,
-EXT_AUDIO_X96   = 2,
-EXT_AUDIO_XXCH  = 6
-};
-
-enum LFEFlag {
-LFE_FLAG_NONE,
-LFE_FLAG_128,
-LFE_FLAG_64,
-LFE_FLAG_INVALID
-};
-
-static const int8_t prm_ch_to_spkr_map[AMODE_COUNT][5] = {
+static const int8_t prm_ch_to_spkr_map[DCA_AMODE_COUNT][5] = {
 { DCA_SPEAKER_C,-1, -1, -1,
 -1 },
 { DCA_SPEAKER_L, DCA_SPEAKER_R, -1, -1,
 -1 },
 { DCA_SPEAKER_L, DCA_SPEAKER_R, -1, -1,
 -1 },
@@ -76,7 +48,7 @@ static const int8_t prm_ch_to_spkr_map[AMODE_COUNT][5] = {
 { DCA_SPEAKER_C, DCA_SPEAKER_L, DCA_SPEAKER_R,  DCA_SPEAKER_Ls, 
DCA_SPEAKER_Rs }
 };
 
-static const uint8_t audio_mode_ch_mask[AMODE_COUNT] = {
+static const uint8_t audio_mode_ch_mask[DCA_AMODE_COUNT] = {
 DCA_SPEAKER_LAYOUT_MONO,
 DCA_SPEAKER_LAYOUT_STEREO,
 DCA_SPEAKER_LAYOUT_STEREO,
@@ -139,7 +111,7 @@ static int parse_frame_header(DCACoreDecoder *s)
 
 // Audio channel arrangement
 s->audio_mode = get_bits(>gb, 6);
-if (s->audio_mode >= AMODE_COUNT) {
+if (s->audio_mode >= DCA_AMODE_COUNT) {
 av_log(s->avctx, AV_LOG_ERROR, "Unsupported audio channel arrangement 
(%d)\n", s->audio_mode);
 return AVERROR_PATCHWELCOME;
 }
@@ -180,7 +152,7 @@ static int parse_frame_header(DCACoreDecoder *s)
 
 // Low frequency effects flag
 s->lfe_present = get_bits(>gb, 2);
-if (s->lfe_present == LFE_FLAG_INVALID) {
+if (s->lfe_present == DCA_LFE_FLAG_INVALID) {
 av_log(s->avctx, AV_LOG_ERROR, "Invalid low frequency effects flag\n");
 return AVERROR_INVALIDDATA;
 }
@@ -1783,7 +1755,7 @@ static int parse_optional_info(DCACoreDecoder *s)
 // must be done backwards from the end of core frame to work around
 // sync word aliasing issues.
 switch (s->ext_audio_type) {
-case EXT_AUDIO_XCH:
+case DCA_EXT_AUDIO_XCH:
 if (dca->request_channel_layout)
 break;
 
@@ -1813,7 +1785,7 @@ static int parse_optional_info(DCACoreDecoder *s)
 }
 break;
 
-case EXT_AUDIO_X96:
+case DCA_EXT_AUDIO_X96:
 // The distance between X96 sync word and end of the core frame
 // must be equal to X96 frame size. Minimum X96 frame size is 96
 // bytes.
@@ -1836,7 +1808,7 @@ static int parse_optional_info(DCACoreDecoder *s)
 }
 break;
 
-case EXT_AUDIO_XXCH:
+case DCA_EXT_AUDIO_XXCH:
 if (dca->request_channel_layout)
 break;
 
@@ -2102,7 +2074,7 @@ int ff_dca_core_filter_fixed(DCACoreDecoder *s, int 
x96_synth)
 int nlfesamples = s->npcmblocks >> 1;
 
 // Check LFF
-if (s->lfe_present == LFE_FLAG_128) {
+if (s->lfe_present == DCA_LFE_FLAG_128) {
 av_log(s->avctx, AV_LOG_ERROR, "Fixed point mode doesn't support 
LFF=1\n");
 return AVERROR(EINVAL);
 }
@@ -2152,7 +2124,7 @@ static int filter_frame_fixed(DCACoreDecoder *s, AVFrame 
*frame)
 
 // Undo embedded XCH downmix
 if (s->es_format && (s->ext_audio_mask & DCA_CSS_XCH)
-&& s->audio_mode >= AMODE_2F2R) {
+&& s->audio_mode >= DCA_AMODE_2F2R) {
 s->dcadsp->dmix_sub_xch(s->output_samples[DCA_SPEAKER_Ls],
 s->output_samples[DCA_SPEAKER_Rs],
 s->output_samples[DCA_SPEAKER_Cs],
@@ -2196,15 +2168,15 @@ static int filter_frame_fixed(DCACoreDecoder *s, 
AVFrame *frame)
 
 if (!(s->ext_audio_mask & (DCA_CSS_XXCH | DCA_CSS_XCH | DCA_EXSS_XXCH))) {
 // Front sum/difference decoding
-if ((s->sumdiff_front && s->audio_mode > AMODE_MONO)
-|| 

[FFmpeg-devel] [PATCH 02/10] avcodec/dca: move bits per sample array to dca.c

[foo86]

It will be used by the parser. This change avoids unwanted parser
dependency on dcadata.
---
 libavcodec/dca.c | 4 
 libavcodec/dca.h | 2 ++
 libavcodec/dcadata.c | 4 
 libavcodec/dcadata.h | 2 --
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libavcodec/dca.c b/libavcodec/dca.c
index 58f340e6da..fb796191d6 100644
--- a/libavcodec/dca.c
+++ b/libavcodec/dca.c
@@ -45,6 +45,10 @@ const uint8_t ff_dca_freq_ranges[16] = {
 0, 1, 2, 3, 4, 1, 2, 3, 4, 4, 0, 1, 2, 3, 4, 4
 };
 
+const uint8_t ff_dca_bits_per_sample[8] = {
+16, 16, 20, 20, 0, 24, 24, 0
+};
+
 int avpriv_dca_convert_bitstream(const uint8_t *src, int src_size, uint8_t 
*dst,
  int max_size)
 {
diff --git a/libavcodec/dca.h b/libavcodec/dca.h
index bd96bc9ee3..1d10de4b94 100644
--- a/libavcodec/dca.h
+++ b/libavcodec/dca.h
@@ -156,6 +156,8 @@ extern av_export const uint32_t avpriv_dca_sample_rates[16];
 
 extern const uint32_t ff_dca_sampling_freqs[16];
 extern const uint8_t ff_dca_freq_ranges[16];
+extern const uint8_t ff_dca_bits_per_sample[8];
+
 
 /**
  * Convert bitstream to one representation based on sync marker
diff --git a/libavcodec/dcadata.c b/libavcodec/dcadata.c
index eaef01875a..1b646a7aa6 100644
--- a/libavcodec/dcadata.c
+++ b/libavcodec/dcadata.c
@@ -42,10 +42,6 @@ const uint8_t ff_dca_channels[16] = {
 1, 2, 2, 2, 2, 3, 3, 4, 4, 5, 6, 6, 6, 7, 8, 8
 };
 
-const uint8_t ff_dca_bits_per_sample[8] = {
-16, 16, 20, 20, 0, 24, 24, 0
-};
-
 const uint8_t ff_dca_dmix_primary_nch[8] = {
 1, 2, 2, 3, 3, 4, 4, 0
 };
diff --git a/libavcodec/dcadata.h b/libavcodec/dcadata.h
index 9dd6eba7f1..5aa85b3414 100644
--- a/libavcodec/dcadata.h
+++ b/libavcodec/dcadata.h
@@ -32,8 +32,6 @@ extern const uint32_t ff_dca_bit_rates[32];
 
 extern const uint8_t ff_dca_channels[16];
 
-extern const uint8_t ff_dca_bits_per_sample[8];
-
 extern const uint8_t ff_dca_dmix_primary_nch[8];
 
 extern const uint8_t ff_dca_quant_index_sel_nbits[DCA_CODE_BOOKS];
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 09/10] avcodec/dca_core: probe extension headers directly

[foo86]

Avoid using bitstream reader in a non-standard way by directly accessing
index. Use bit shifting/masking operations instead.
---
 libavcodec/dca_core.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/libavcodec/dca_core.c b/libavcodec/dca_core.c
index 090191dfa9..4a7ea4e3f3 100644
--- a/libavcodec/dca_core.c
+++ b/libavcodec/dca_core.c
@@ -1704,6 +1704,7 @@ static int parse_optional_info(DCACoreDecoder *s)
 int sync_pos = FFMIN(s->frame_size / 4, s->gb.size_in_bits / 32) - 1;
 int last_pos = get_bits_count(>gb) / 32;
 int size, dist;
+uint32_t w1, w2 = 0;
 
 // Search for extension sync words aligned on 4-byte boundary. Search
 // must be done backwards from the end of core frame to work around
@@ -1718,15 +1719,15 @@ static int parse_optional_info(DCACoreDecoder *s)
 // compatibility with legacy bitstreams. Minimum XCH frame size is
 // 96 bytes. AMODE and PCHS are further checked to reduce
 // probability of alias sync detection.
-for (; sync_pos >= last_pos; sync_pos--) {
-if (AV_RB32(s->gb.buffer + sync_pos * 4) == DCA_SYNCWORD_XCH) {
-s->gb.index = (sync_pos + 1) * 32;
-size = get_bits(>gb, 10) + 1;
+for (; sync_pos >= last_pos; sync_pos--, w2 = w1) {
+w1 = AV_RB32(s->gb.buffer + sync_pos * 4);
+if (w1 == DCA_SYNCWORD_XCH) {
+size = (w2 >> 22) + 1;
 dist = s->frame_size - sync_pos * 4;
 if (size >= 96
 && (size == dist || size - 1 == dist)
-&& get_bits(>gb, 7) == 0x08) {
-s->xch_pos = get_bits_count(>gb);
+&& (w2 >> 15 & 0x7f) == 0x08) {
+s->xch_pos = sync_pos * 32 + 49;
 break;
 }
 }
@@ -1743,13 +1744,13 @@ static int parse_optional_info(DCACoreDecoder *s)
 // The distance between X96 sync word and end of the core frame
 // must be equal to X96 frame size. Minimum X96 frame size is 96
 // bytes.
-for (; sync_pos >= last_pos; sync_pos--) {
-if (AV_RB32(s->gb.buffer + sync_pos * 4) == DCA_SYNCWORD_X96) {
-s->gb.index = (sync_pos + 1) * 32;
-size = get_bits(>gb, 12) + 1;
+for (; sync_pos >= last_pos; sync_pos--, w2 = w1) {
+w1 = AV_RB32(s->gb.buffer + sync_pos * 4);
+if (w1 == DCA_SYNCWORD_X96) {
+size = (w2 >> 20) + 1;
 dist = s->frame_size - sync_pos * 4;
 if (size >= 96 && size == dist) {
-s->x96_pos = get_bits_count(>gb);
+s->x96_pos = sync_pos * 32 + 44;
 break;
 }
 }
@@ -1768,10 +1769,10 @@ static int parse_optional_info(DCACoreDecoder *s)
 
 // XXCH frame header CRC must be valid. Minimum XXCH frame header
 // size is 11 bytes.
-for (; sync_pos >= last_pos; sync_pos--) {
-if (AV_RB32(s->gb.buffer + sync_pos * 4) == DCA_SYNCWORD_XXCH) 
{
-s->gb.index = (sync_pos + 1) * 32;
-size = get_bits(>gb, 6) + 1;
+for (; sync_pos >= last_pos; sync_pos--, w2 = w1) {
+w1 = AV_RB32(s->gb.buffer + sync_pos * 4);
+if (w1 == DCA_SYNCWORD_XXCH) {
+size = (w2 >> 26) + 1;
 dist = s->gb.size_in_bits / 8 - sync_pos * 4;
 if (size >= 11 && size <= dist &&
 !av_crc(dca->crctab, 0x, s->gb.buffer +
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 04/10] avcodec/dca_core: switch to common frame header parsing function

[foo86]

---
 libavcodec/dca_core.c | 167 ++
 1 file changed, 60 insertions(+), 107 deletions(-)

diff --git a/libavcodec/dca_core.c b/libavcodec/dca_core.c
index 16210b89f8..090191dfa9 100644
--- a/libavcodec/dca_core.c
+++ b/libavcodec/dca_core.c
@@ -81,114 +81,68 @@ static void get_array(GetBitContext *s, int32_t *array, 
int size, int n)
 // 5.3.1 - Bit stream header
 static int parse_frame_header(DCACoreDecoder *s)
 {
-int normal_frame, pcmr_index;
-
-// Frame type
-normal_frame = get_bits1(>gb);
-
-// Deficit sample count
-if (get_bits(>gb, 5) != DCA_PCMBLOCK_SAMPLES - 1) {
-av_log(s->avctx, AV_LOG_ERROR, "Deficit samples are not supported\n");
-return normal_frame ? AVERROR_INVALIDDATA : AVERROR_PATCHWELCOME;
-}
-
-// CRC present flag
-s->crc_present = get_bits1(>gb);
-
-// Number of PCM sample blocks
-s->npcmblocks = get_bits(>gb, 7) + 1;
-if (s->npcmblocks & (DCA_SUBBAND_SAMPLES - 1)) {
-av_log(s->avctx, AV_LOG_ERROR, "Unsupported number of PCM sample 
blocks (%d)\n", s->npcmblocks);
-return (s->npcmblocks < 6 || normal_frame) ? AVERROR_INVALIDDATA : 
AVERROR_PATCHWELCOME;
-}
-
-// Primary frame byte size
-s->frame_size = get_bits(>gb, 14) + 1;
-if (s->frame_size < 96) {
-av_log(s->avctx, AV_LOG_ERROR, "Invalid core frame size (%d bytes)\n", 
s->frame_size);
-return AVERROR_INVALIDDATA;
-}
-
-// Audio channel arrangement
-s->audio_mode = get_bits(>gb, 6);
-if (s->audio_mode >= DCA_AMODE_COUNT) {
-av_log(s->avctx, AV_LOG_ERROR, "Unsupported audio channel arrangement 
(%d)\n", s->audio_mode);
-return AVERROR_PATCHWELCOME;
-}
-
-// Core audio sampling frequency
-s->sample_rate = avpriv_dca_sample_rates[get_bits(>gb, 4)];
-if (!s->sample_rate) {
-av_log(s->avctx, AV_LOG_ERROR, "Invalid core audio sampling 
frequency\n");
-return AVERROR_INVALIDDATA;
+DCACoreFrameHeader h = { 0 };
+int err = avpriv_dca_parse_core_frame_header(>gb, );
+
+if (err < 0) {
+switch (err) {
+case DCA_PARSE_ERROR_DEFICIT_SAMPLES:
+av_log(s->avctx, AV_LOG_ERROR, "Deficit samples are not 
supported\n");
+return h.normal_frame ? AVERROR_INVALIDDATA : AVERROR_PATCHWELCOME;
+
+case DCA_PARSE_ERROR_PCM_BLOCKS:
+av_log(s->avctx, AV_LOG_ERROR, "Unsupported number of PCM sample 
blocks (%d)\n", h.npcmblocks);
+return (h.npcmblocks < 6 || h.normal_frame) ? AVERROR_INVALIDDATA 
: AVERROR_PATCHWELCOME;
+
+case DCA_PARSE_ERROR_FRAME_SIZE:
+av_log(s->avctx, AV_LOG_ERROR, "Invalid core frame size (%d 
bytes)\n", h.frame_size);
+return AVERROR_INVALIDDATA;
+
+case DCA_PARSE_ERROR_AMODE:
+av_log(s->avctx, AV_LOG_ERROR, "Unsupported audio channel 
arrangement (%d)\n", h.audio_mode);
+return AVERROR_PATCHWELCOME;
+
+case DCA_PARSE_ERROR_SAMPLE_RATE:
+av_log(s->avctx, AV_LOG_ERROR, "Invalid core audio sampling 
frequency\n");
+return AVERROR_INVALIDDATA;
+
+case DCA_PARSE_ERROR_RESERVED_BIT:
+av_log(s->avctx, AV_LOG_ERROR, "Reserved bit set\n");
+return AVERROR_INVALIDDATA;
+
+case DCA_PARSE_ERROR_LFE_FLAG:
+av_log(s->avctx, AV_LOG_ERROR, "Invalid low frequency effects 
flag\n");
+return AVERROR_INVALIDDATA;
+
+case DCA_PARSE_ERROR_PCM_RES:
+av_log(s->avctx, AV_LOG_ERROR, "Invalid source PCM resolution\n");
+return AVERROR_INVALIDDATA;
+
+default:
+av_log(s->avctx, AV_LOG_ERROR, "Unknown core frame header 
error\n");
+return AVERROR_INVALIDDATA;
+}
 }
 
-// Transmission bit rate
-s->bit_rate = ff_dca_bit_rates[get_bits(>gb, 5)];
-
-// Reserved field
-skip_bits1(>gb);
-
-// Embedded dynamic range flag
-s->drc_present = get_bits1(>gb);
-
-// Embedded time stamp flag
-s->ts_present = get_bits1(>gb);
-
-// Auxiliary data flag
-s->aux_present = get_bits1(>gb);
-
-// HDCD mastering flag
-skip_bits1(>gb);
-
-// Extension audio descriptor flag
-s->ext_audio_type = get_bits(>gb, 3);
-
-// Extended coding flag
-s->ext_audio_present = get_bits1(>gb);
-
-// Audio sync word insertion flag
-s->sync_ssf = get_bits1(>gb);
-
-// Low frequency effects flag
-s->lfe_present = get_bits(>gb, 2);
-if (s->lfe_present == DCA_LFE_FLAG_INVALID) {
-av_log(s->avctx, AV_LOG_ERROR, "Invalid low frequency effects flag\n");
-return AVERROR_INVALIDDATA;
-}
-
-// Predictor history flag switch
-s->predictor_history = get_bits1(>gb);
-
-// Header CRC check bytes
-if (s->crc_present)
-skip_bits(>gb, 16);
-
-// Multirate interpolator switch
-s->filter_perfect = get_bits1(>gb);
-
-// Encoder software revision
-

[FFmpeg-devel] [PATCH 05/10] avcodec/dca_parser: switch to common frame header parsing function

[foo86]

---
 libavcodec/dca_parser.c | 26 +++---
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/libavcodec/dca_parser.c b/libavcodec/dca_parser.c
index e5bea3347c..6107358773 100644
--- a/libavcodec/dca_parser.c
+++ b/libavcodec/dca_parser.c
@@ -192,10 +192,11 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 int buf_size, int *duration, int *sample_rate)
 {
 GetBitContext gb;
-uint8_t hdr[12 + AV_INPUT_BUFFER_PADDING_SIZE] = { 0 };
-int ret, sample_blocks;
+DCACoreFrameHeader h;
+uint8_t hdr[DCA_CORE_FRAME_HEADER_SIZE + AV_INPUT_BUFFER_PADDING_SIZE] = { 
0 };
+int ret;
 
-if (buf_size < 12)
+if (buf_size < DCA_CORE_FRAME_HEADER_SIZE)
 return AVERROR_INVALIDDATA;
 
 if (AV_RB32(buf) == DCA_SYNCWORD_SUBSTREAM) {
@@ -255,21 +256,16 @@ static int dca_parse_params(DCAParseContext *pc1, const 
uint8_t *buf,
 return AVERROR_INVALIDDATA;
 }
 
-if ((ret = avpriv_dca_convert_bitstream(buf, 12, hdr, 12)) < 0)
+if ((ret = avpriv_dca_convert_bitstream(buf, DCA_CORE_FRAME_HEADER_SIZE,
+hdr, DCA_CORE_FRAME_HEADER_SIZE)) 
< 0)
 return ret;
-
-init_get_bits(, hdr, 96);
-
-skip_bits_long(, 39);
-sample_blocks = get_bits(, 7) + 1;
-if (sample_blocks < 8)
+if ((ret = init_get_bits8(, hdr, ret)) < 0)
+return ret;
+if (avpriv_dca_parse_core_frame_header(, ) < 0)
 return AVERROR_INVALIDDATA;
-*duration = 256 * (sample_blocks / 8);
 
-skip_bits(, 20);
-*sample_rate = avpriv_dca_sample_rates[get_bits(, 4)];
-if (*sample_rate == 0)
-return AVERROR_INVALIDDATA;
+*duration = 256 * (h.npcmblocks / 8);
+*sample_rate = avpriv_dca_sample_rates[h.sr_code];
 
 return 0;
 }
-- 
2.11.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/aacpsdsp_template: Fixes integer overflow in ps_add_squares_c()

[wm4]

On Mon, 10 Jul 2017 09:29:31 -0300
James Almer  wrote:

> On 7/10/2017 5:37 AM, wm4 wrote:
> > On Sun, 9 Jul 2017 22:03:21 +0200
> > Reimar Döffinger  wrote:
> >   
> >> On 09.07.2017, at 16:08, "Ronald S. Bultje"  wrote:  
> >>> Hi,
> >>>
> >>> On Sun, Jul 9, 2017 at 4:39 AM, Reimar Döffinger 
> >>> 
> >>> wrote:
> >>> 
>  On 09.07.2017, at 02:52, "Ronald S. Bultje"  wrote:  
>    
> > On Sat, Jul 8, 2017 at 5:17 PM, Michael Niedermayer
>  
> > wrote:
> > 
> >>
> >> Does anyone object to this patch ?
> >> Or does anyone have a better idea on how to fix this ?
> >> if not id like to apply it
> >
> >
> > I think Rostislav's point is: why fix it, if it can only happen with
> > corrupt input? The before and after situation is identical: garbage in,
> > garbage out. If the compiler does funny things that makes the garbage
> > slightly differently bad, is that really so devilishly bad? It's still
> > garbage. Is anything improved by this?
> 
>  The way C works, you MUST assume any undefined behaviour can at any point
>  [..] become exploitable.[..] If you don't like that, C is the wrong
>  language to use.
> >>>
> >>>
> >>> I think I've read "the boy who cried wolf" a few too many times to my 
> >>> kids,
> >>> but the form of this discussion is currently too polarizing/political for
> >>> my taste.
> >>
> >> That is my reading of the C standard, is that political or even just 
> >> controversial?
> >> I mean of course you can ignore standards (see MPEG-4 ASP, and in some 
> >> ways that was actually fairly reasonable thing to do at the time), and I 
> >> don't fix every undefined behaviour case in my code when I can't think of 
> >> any reasonable solution.
> >> So there is a cost-benefit discussion in principle.
> >> I believe the cost of not fixing undefined behaviour, just by virtue of 
> >> going outside what the standard guarantees should be considered fairly 
> >> high.
> >> That is an opinion, but is there any disagreement that undefined behaviour 
> >> is at least an issue of some degree?
> >> If we can agree on that, then the question would only be how much 
> >> effort/code ugliness is reasonable.
> >> There is also the point (which I hope I mentioned in the parts cut out) 
> >> that just making sure that these cases are not already exploitable right 
> >> now with the current compiler can in many cases be quite a pain (and does 
> >> not have tool support), so I think fixing it would often be the 
> >> lowest-effort method.  
> > 
> > The controversial thing is actually the SUINT nonsense. A type is
> > either signed or unsigned, but not both incompletely intransparent
> > ways. Michael keeps adding them even though many are against it. 
> > 
> > "SUINTFLOAT" just tops this. Is it a signed int? Unsigned int? A float?
> > Unsigned float???  
> 
> SUINTFLOAT is float or an integer (SUINT) depending on how you include
> the template file.
> 
> Blame the template crap for all the int variants of decoders (aac, ac3,
> etc).

Yeah, I understand how it came to be, but maybe it's not a good idea to
combine the various hacks into more combinations of hacks.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/aacpsdsp_template: Fixes integer overflow in ps_add_squares_c()

[James Almer]

On 7/10/2017 5:37 AM, wm4 wrote:
> On Sun, 9 Jul 2017 22:03:21 +0200
> Reimar Döffinger  wrote:
> 
>> On 09.07.2017, at 16:08, "Ronald S. Bultje"  wrote:
>>> Hi,
>>>
>>> On Sun, Jul 9, 2017 at 4:39 AM, Reimar Döffinger 
>>> wrote:
>>>   
 On 09.07.2017, at 02:52, "Ronald S. Bultje"  wrote:  
> On Sat, Jul 8, 2017 at 5:17 PM, Michael Niedermayer  
   
> wrote:
>   
>>
>> Does anyone object to this patch ?
>> Or does anyone have a better idea on how to fix this ?
>> if not id like to apply it  
>
>
> I think Rostislav's point is: why fix it, if it can only happen with
> corrupt input? The before and after situation is identical: garbage in,
> garbage out. If the compiler does funny things that makes the garbage
> slightly differently bad, is that really so devilishly bad? It's still
> garbage. Is anything improved by this?  

 The way C works, you MUST assume any undefined behaviour can at any point
 [..] become exploitable.[..] If you don't like that, C is the wrong
 language to use.  
>>>
>>>
>>> I think I've read "the boy who cried wolf" a few too many times to my kids,
>>> but the form of this discussion is currently too polarizing/political for
>>> my taste.  
>>
>> That is my reading of the C standard, is that political or even just 
>> controversial?
>> I mean of course you can ignore standards (see MPEG-4 ASP, and in some ways 
>> that was actually fairly reasonable thing to do at the time), and I don't 
>> fix every undefined behaviour case in my code when I can't think of any 
>> reasonable solution.
>> So there is a cost-benefit discussion in principle.
>> I believe the cost of not fixing undefined behaviour, just by virtue of 
>> going outside what the standard guarantees should be considered fairly high.
>> That is an opinion, but is there any disagreement that undefined behaviour 
>> is at least an issue of some degree?
>> If we can agree on that, then the question would only be how much 
>> effort/code ugliness is reasonable.
>> There is also the point (which I hope I mentioned in the parts cut out) that 
>> just making sure that these cases are not already exploitable right now with 
>> the current compiler can in many cases be quite a pain (and does not have 
>> tool support), so I think fixing it would often be the lowest-effort method.
> 
> The controversial thing is actually the SUINT nonsense. A type is
> either signed or unsigned, but not both incompletely intransparent
> ways. Michael keeps adding them even though many are against it. 
> 
> "SUINTFLOAT" just tops this. Is it a signed int? Unsigned int? A float?
> Unsigned float???

SUINTFLOAT is float or an integer (SUINT) depending on how you include
the template file.

Blame the template crap for all the int variants of decoders (aac, ac3,
etc).

> 
> Come on, this is a huge load of bullshit.
> 
>> I'd also like to point out that even ignoring all that, ubsan + fuzzing is a 
>> powerful tool to improve code quality, and I would argue than at least some 
>> amount of code complexity is justified just for having them work well.
>> And it's also that to my mind the patch did not look THAT bad...
> 
> A powerful tool for Michael to churn out patches which make the code
> look worse and more tricky, which without doubt will lead ot more new
> bugs some time in the future. Sure, security is good, but at this
> point I'm even wondering what's the point of this. Realistically,
> you'll have to sandbox ffmpeg anyway if you want some minimal security.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo: Fix all but one tsan warning in fate-vsynth1-mpeg4

[Ronald S. Bultje]

Hi,

On Mon, Jul 10, 2017 at 5:32 AM, wm4  wrote:

> On Sun, 9 Jul 2017 22:37:46 -0400
> "Ronald S. Bultje"  wrote:
>
> > Hi,
> >
> > On Sun, Jul 9, 2017 at 9:19 PM, Michael Niedermayer
> 
> > wrote:
> >
> > > Signed-off-by: Michael Niedermayer 
> > > ---
> > >  libavcodec/mpegvideo.c | 15 ++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > > index e29558b3a2..574b3854e0 100644
> > > --- a/libavcodec/mpegvideo.c
> > > +++ b/libavcodec/mpegvideo.c
> > > @@ -495,7 +495,20 @@ int ff_mpeg_update_thread_context(AVCodecContext
> > > *dst,
> > >  // in that case dst may need a reinit
> > >  if (!s->context_initialized) {
> > >  int err;
> > > -memcpy(s, s1, sizeof(MpegEncContext));
> > > +#define COPY(x) s->x = s1->x;
> > >
> >
> > Unused?
> >
> >
> > > +#define COPYR(a, b) memcpy(>a, >a, ((uint8_t*)>b) -
> > > ((uint8_t*)>a))
> > > +COPYR(h263_aic, qscale);
> > > +COPYR(lambda,   mv_dir);
> > > +COPYR(no_rounding,  dest);
> > > +COPYR(mb_index2xy,  resync_mb_x);
> > > +COPYR(next_p_frame_damaged, h263_aic_dir);
> > > +COPYR(h263_slice_structured,mcsel);
> > > +COPYR(quant_precision,  first_slice_line);
> > > +COPYR(flipflop_rounding,gb);
> > > +COPYR(gop_picture_number,   picture_structure);
> > > +COPYR(picture_structure,pblocks);
> > > +COPYR(decode_mb,er);
> > > +COPYR(error_rate,   noise_reduction);
> >
> >
> > This is very obscure... The obscure part is what happens when fields get
> > (through refactoring) reordered or new fields get inserted...
> >
> > I also appear to remember that we used to use this same pattern for h264
> > also, but we don't anymore. Does anyone remember why?
>
> Do you mean 0ba471d7d864c712f45d7ac6aca4829aba025adc?
>
> h264: eliminate copy_fields
>
> It is very fragile against fields being moved and hides what is
> actually
> being copied. Copy all the fields explicitly instead.
>
> Seems justification enough for me.


Ah, yes, I tried to look for it but couldn't find it, thanks!

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo: Fix all but one tsan warning in fate-vsynth1-mpeg4

[Michael Niedermayer]

On Sun, Jul 09, 2017 at 10:37:46PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Sun, Jul 9, 2017 at 9:19 PM, Michael Niedermayer 
> wrote:
> 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/mpegvideo.c | 15 ++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > index e29558b3a2..574b3854e0 100644
> > --- a/libavcodec/mpegvideo.c
> > +++ b/libavcodec/mpegvideo.c
> > @@ -495,7 +495,20 @@ int ff_mpeg_update_thread_context(AVCodecContext
> > *dst,
> >  // in that case dst may need a reinit
> >  if (!s->context_initialized) {
> >  int err;
> > -memcpy(s, s1, sizeof(MpegEncContext));
> > +#define COPY(x) s->x = s1->x;
> >
> 
> Unused?

yes


> 
> 
> > +#define COPYR(a, b) memcpy(>a, >a, ((uint8_t*)>b) -
> > ((uint8_t*)>a))
> > +COPYR(h263_aic, qscale);
> > +COPYR(lambda,   mv_dir);
> > +COPYR(no_rounding,  dest);
> > +COPYR(mb_index2xy,  resync_mb_x);
> > +COPYR(next_p_frame_damaged, h263_aic_dir);
> > +COPYR(h263_slice_structured,mcsel);
> > +COPYR(quant_precision,  first_slice_line);
> > +COPYR(flipflop_rounding,gb);
> > +COPYR(gop_picture_number,   picture_structure);
> > +COPYR(picture_structure,pblocks);
> > +COPYR(decode_mb,er);
> > +COPYR(error_rate,   noise_reduction);
> 
> 
> This is very obscure... The obscure part is what happens when fields get
> (through refactoring) reordered or new fields get inserted...

yes, its just what you asked me about on IRC

 michaelni: I don’t think we’re asking you to actually fix mpeg4-frame-mt 
(there’s no issues anyway), more to figure out which fields to sync so the 
memcpy is unneeded
 the way I would do that is to copy all fields and remove them until 
frame-mt+fate breaks or tsan stops complaining

Thats a list copying all fields minus what either broke tsan or was
next to one and on first sight looked like it shouldnt be copied.

From this you can make a list of all ~200 fields that are still copied
if that is desired. (seems like a bad idea unless alot of fields can be
omited and only few remain)

or IMHO better, organize the struct members better so that no correct
addition or change can break it.
That is either have section(s) that are copied and documented as such
or have sub structure(s) that are copied.

My plan was to copy the ranges minus what breaks tsan for 
fate-vsynth1-mpeg4

then we can go over the other tests one by one and adjust the copied
ranges (aka remove areas that trigger tsan warnings)
and go over all fields one by one (there are alot) and adjust the
copying (here also the unused COPY macro likely would come in handy)
(easy for all developers to colaborate on if its in git master)

once we think its all ok we wait and see if any bug reorts come in
from the different set of what is copied (costs us 0 work)

and last once we are certain what fields need to be copied we factor
the code to make it clean and maintainable be that through sub
structure or through reordering of fields
(easy for all developers to colaborate on if its in git master)

It seems my first step/patch is not popular as its ugly which i am the
last to deny.

If its preferred to do all the steps outside git master (minus free
user testing as thats not possible), we can do this too. But i wont do
that alone by myself in some private repo, as i never intended to do
the whole work on my own.

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] h264: Support multi-field closed captions by using AVBufferRef and not resetting per field

[wm4]

On Mon, 10 Jul 2017 09:43:09 +
Kieran Kunhya  wrote:

> On Mon, 10 Jul 2017 at 10:39 wm4  wrote:
> 
> >  
> > >  h->frame_recovered   = h1->frame_recovered;
> > > +if (h1->sei.a53_caption.buf_ref) {
> > > +h->sei.a53_caption.buf_ref =  
> > av_buffer_ref(h1->sei.a53_caption.buf_ref);  
> > > +av_buffer_unref(>sei.a53_caption.buf_ref);
> > > +}
> > > +else
> > > +h->sei.a53_caption.buf_ref = NULL;  
> >
> > That seems overly convoluted. Since you only want to move the
> > reference instead of creating a new one, you could just assign the
> > AVBufferRef pointer and assign the source field to NULL.
> >  
> 
> I can't do that, the source is const.

I don't understand. I don't see any const here, except on the src
AVCodecContext, which doesn't matter.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avcodec/pthread_slice: rewrite implementation

[Muhammad Faiz]

On Mon, Jul 10, 2017 at 4:25 PM, wm4  wrote:
> On Sun,  9 Jul 2017 23:26:54 +0700
> Muhammad Faiz  wrote:
>
>> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
>> uses distict mutex/cond. Also let main thread help running jobs, but still
>> allocate thread_count workers. The last worker is currently unused, emulated
>> by main thread.
>> Similar to 'avfilter/pthread: rewrite implementation'
>>
>> Benchmark on x86_64 with 4 cpus (2 cores x 2 hyperthread)
>> ./ffmpeg -threads $threads -thread_type slice -i 10slices.mp4 -f rawvideo -y 
>> /dev/null
>> threads=2:
>> old: 1m15.888s
>> new:  1m5.710s
>> threads=3:
>> old:  1m6.480s
>> new:  1m5.260s
>> threads=4:
>> old:  1m2.292s
>> new:   59.677s
>> threads=5:
>> old:   58.939s
>> new:   55.166s
>>
>> Signed-off-by: Muhammad Faiz 
>> ---
>>  libavcodec/pthread_slice.c | 219 
>> +
>>  1 file changed, 142 insertions(+), 77 deletions(-)
>>
>> diff --git a/libavcodec/pthread_slice.c b/libavcodec/pthread_slice.c
>> index 60f5b78..7223205 100644
>> --- a/libavcodec/pthread_slice.c
>> +++ b/libavcodec/pthread_slice.c
>> @@ -22,6 +22,7 @@
>>   * @see doc/multithreading.txt
>>   */
>>
>> +#include 
>>  #include "config.h"
>>
>>  #include "avcodec.h"
>> @@ -38,21 +39,26 @@
>>  typedef int (action_func)(AVCodecContext *c, void *arg);
>>  typedef int (action_func2)(AVCodecContext *c, void *arg, int jobnr, int 
>> threadnr);
>>
>> +typedef struct WorkerContext WorkerContext;
>> +
>>  typedef struct SliceThreadContext {
>> -pthread_t *workers;
>> +AVCodecContext *avctx;
>> +WorkerContext *workers;
>>  action_func *func;
>>  action_func2 *func2;
>>  void *args;
>>  int *rets;
>> -int job_count;
>> -int job_size;
>> -
>> -pthread_cond_t last_job_cond;
>> -pthread_cond_t current_job_cond;
>> -pthread_mutex_t current_job_lock;
>> -unsigned current_execute;
>> -int current_job;
>> +unsigned job_count;
>> +unsigned job_size;
>> +
>> +pthread_mutex_t mutex_user;
>> +pthread_mutex_t mutex_done;
>> +pthread_cond_t cond_done;
>> +atomic_uint first_job;
>> +atomic_uint current_job;
>> +atomic_uint nb_finished_jobs;
>>  int done;
>> +int worker_done;
>>
>>  int *entries;
>>  int entries_count;
>> @@ -61,42 +67,55 @@ typedef struct SliceThreadContext {
>>  pthread_mutex_t *progress_mutex;
>>  } SliceThreadContext;
>>
>> -static void* attribute_align_arg worker(void *v)
>> +struct WorkerContext {
>> +SliceThreadContext  *ctx;
>> +pthread_t   thread;
>> +pthread_mutex_t mutex;
>> +pthread_cond_t  cond;
>> +int done;
>> +};
>> +
>> +static unsigned run_jobs(SliceThreadContext *c)
>>  {
>> -AVCodecContext *avctx = v;
>> -SliceThreadContext *c = avctx->internal->thread_ctx;
>> -unsigned last_execute = 0;
>> -int our_job = c->job_count;
>> -int thread_count = avctx->thread_count;
>> -int self_id;
>> -
>> -pthread_mutex_lock(>current_job_lock);
>> -self_id = c->current_job++;
>> -for (;;){
>> -int ret;
>> -while (our_job >= c->job_count) {
>> -if (c->current_job == thread_count + c->job_count)
>> -pthread_cond_signal(>last_job_cond);
>> -
>> -while (last_execute == c->current_execute && !c->done)
>> -pthread_cond_wait(>current_job_cond, 
>> >current_job_lock);
>> -last_execute = c->current_execute;
>> -our_job = self_id;
>> -
>> -if (c->done) {
>> -pthread_mutex_unlock(>current_job_lock);
>> -return NULL;
>> -}
>> -}
>> -pthread_mutex_unlock(>current_job_lock);
>> +unsigned current_job, first_job, nb_finished_jobs;
>> +
>> +current_job = first_job = atomic_fetch_add_explicit(>first_job, 1, 
>> memory_order_acq_rel);
>>
>> -ret = c->func ? c->func(avctx, (char*)c->args + 
>> our_job*c->job_size):
>> -c->func2(avctx, c->args, our_job, self_id);
>> +do {
>> +int ret = c->func ? c->func(c->avctx, (char *)c->args + current_job 
>> * (size_t) c->job_size)
>> +  : c->func2(c->avctx, c->args, current_job, 
>> first_job);
>>  if (c->rets)
>> -c->rets[our_job%c->job_count] = ret;
>> +c->rets[current_job] = ret;
>> +nb_finished_jobs = atomic_fetch_add_explicit(>nb_finished_jobs, 
>> 1, memory_order_relaxed) + 1;
>> +} while ((current_job = atomic_fetch_add_explicit(>current_job, 1, 
>> memory_order_acq_rel)) < c->job_count);
>>
>> -pthread_mutex_lock(>current_job_lock);
>> -our_job = c->current_job++;
>> +return nb_finished_jobs;
>> +}
>> +
>> +static void* attribute_align_arg worker(void *v)
>> +{
>> +WorkerContext *w = v;
>> +SliceThreadContext *c = w->ctx;
>> +
>> 

Re: [FFmpeg-devel] [PATCH] h264: Support multi-field closed captions by using AVBufferRef and not resetting per field

[Kieran Kunhya]

On Mon, 10 Jul 2017 at 10:39 wm4  wrote:

>
> >  h->frame_recovered   = h1->frame_recovered;
> > +if (h1->sei.a53_caption.buf_ref) {
> > +h->sei.a53_caption.buf_ref =
> av_buffer_ref(h1->sei.a53_caption.buf_ref);
> > +av_buffer_unref(>sei.a53_caption.buf_ref);
> > +}
> > +else
> > +h->sei.a53_caption.buf_ref = NULL;
>
> That seems overly convoluted. Since you only want to move the
> reference instead of creating a new one, you could just assign the
> AVBufferRef pointer and assign the source field to NULL.
>

I can't do that, the source is const.

Regards,
Kieran Kunhya
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] h264: Support multi-field closed captions by using AVBufferRef and not resetting per field

[wm4]

On Sun, 09 Jul 2017 22:59:41 +
Kieran Kunhya  wrote:

> From e8768677511ae5ae9c62c7182a73993e5132f5b8 Mon Sep 17 00:00:00 2001
> From: Kieran Kunhya 
> Date: Sun, 9 Jul 2017 23:56:14 +0100
> Subject: [PATCH] h264: Support multi-field closed captions by using
>  AVBufferRef and not resetting per field
> 
> ---
>  libavcodec/h264_sei.c   | 15 ---
>  libavcodec/h264_sei.h   |  3 +--
>  libavcodec/h264_slice.c | 15 ++-
>  libavcodec/h264dec.c|  5 +++--
>  4 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index a7e627e..3dcac7e 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -51,8 +51,7 @@ void ff_h264_sei_uninit(H264SEIContext *h)
>  h->display_orientation.present = 0;
>  h->afd.present =  0;
>  
> -h->a53_caption.a53_caption_size = 0;
> -av_freep(>a53_caption.a53_caption);
> +av_buffer_unref(>a53_caption.buf_ref);
>  }
>  
>  static int decode_picture_timing(H264SEIPictureTiming *h, GetBitContext *gb,
> @@ -169,7 +168,8 @@ static int 
> decode_registered_user_data_closed_caption(H264SEIA53Caption *h,
>  size -= 2;
>  
>  if (cc_count && size >= cc_count * 3) {
> -const uint64_t new_size = (h->a53_caption_size + cc_count
> +int old_size = h->buf_ref ? h->buf_ref->size : 0;
> +const uint64_t new_size = (old_size + cc_count
> * UINT64_C(3));
>  int i, ret;
>  
> @@ -177,14 +177,15 @@ static int 
> decode_registered_user_data_closed_caption(H264SEIA53Caption *h,
>  return AVERROR(EINVAL);
>  
>  /* Allow merging of the cc data from two fields. */
> -ret = av_reallocp(>a53_caption, new_size);
> +ret = av_buffer_realloc(>buf_ref, new_size);
>  if (ret < 0)
>  return ret;
>  
> +/* Use of av_buffer_realloc assumes buffer is writeable */
>  for (i = 0; i < cc_count; i++) {
> -h->a53_caption[h->a53_caption_size++] = get_bits(gb, 8);
> -h->a53_caption[h->a53_caption_size++] = get_bits(gb, 8);
> -h->a53_caption[h->a53_caption_size++] = get_bits(gb, 8);
> +h->buf_ref->data[old_size++] = get_bits(gb, 8);
> +h->buf_ref->data[old_size++] = get_bits(gb, 8);
> +h->buf_ref->data[old_size++] = get_bits(gb, 8);
>  }
>  
>  skip_bits(gb, 8);   // marker_bits
> diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h
> index da3b391..7d092e4 100644
> --- a/libavcodec/h264_sei.h
> +++ b/libavcodec/h264_sei.h
> @@ -91,8 +91,7 @@ typedef struct H264SEIAFD {
>  } H264SEIAFD;
>  
>  typedef struct H264SEIA53Caption {
> -int a53_caption_size;
> -uint8_t *a53_caption;
> +AVBufferRef *buf_ref;
>  } H264SEIA53Caption;
>  
>  typedef struct H264SEIUnregistered {
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index 6deb08f..39662a6 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -430,6 +430,12 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
> MAX_DELAYED_PIC_COUNT + 2, h, h1);
>  
>  h->frame_recovered   = h1->frame_recovered;
> +if (h1->sei.a53_caption.buf_ref) {
> +h->sei.a53_caption.buf_ref = 
> av_buffer_ref(h1->sei.a53_caption.buf_ref);
> +av_buffer_unref(>sei.a53_caption.buf_ref);
> +}
> +else
> +h->sei.a53_caption.buf_ref = NULL;

That seems overly convoluted. Since you only want to move the
reference instead of creating a new one, you could just assign the
AVBufferRef pointer and assign the source field to NULL.

>  
>  if (!h->cur_pic_ptr)
>  return 0;
> @@ -1276,15 +1282,14 @@ static int h264_export_frame_props(H264Context *h)
>  }
>  }
>  
> -if (h->sei.a53_caption.a53_caption) {
> +if (h->sei.a53_caption.buf_ref) {
>  H264SEIA53Caption *a53 = >sei.a53_caption;
>  AVFrameSideData *sd = av_frame_new_side_data(cur->f,
>   AV_FRAME_DATA_A53_CC,
> - a53->a53_caption_size);
> + a53->buf_ref->size);
>  if (sd)
> -memcpy(sd->data, a53->a53_caption, a53->a53_caption_size);
> -av_freep(>a53_caption);
> -a53->a53_caption_size = 0;
> +memcpy(sd->data, a53->buf_ref->data, a53->buf_ref->size);
> +av_buffer_unref(>buf_ref);

(In theory you could avoid the memcpy and set the buffer ref on the
side data, but that would probably cause more trouble than it solves.)

>  h->avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
>  }
>  
> diff 

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo: Fix all but one tsan warning in fate-vsynth1-mpeg4

[wm4]

On Sun, 9 Jul 2017 22:37:46 -0400
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Sun, Jul 9, 2017 at 9:19 PM, Michael Niedermayer 
> wrote:
> 
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/mpegvideo.c | 15 ++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> > index e29558b3a2..574b3854e0 100644
> > --- a/libavcodec/mpegvideo.c
> > +++ b/libavcodec/mpegvideo.c
> > @@ -495,7 +495,20 @@ int ff_mpeg_update_thread_context(AVCodecContext
> > *dst,
> >  // in that case dst may need a reinit
> >  if (!s->context_initialized) {
> >  int err;
> > -memcpy(s, s1, sizeof(MpegEncContext));
> > +#define COPY(x) s->x = s1->x;
> >  
> 
> Unused?
> 
> 
> > +#define COPYR(a, b) memcpy(>a, >a, ((uint8_t*)>b) -
> > ((uint8_t*)>a))
> > +COPYR(h263_aic, qscale);
> > +COPYR(lambda,   mv_dir);
> > +COPYR(no_rounding,  dest);
> > +COPYR(mb_index2xy,  resync_mb_x);
> > +COPYR(next_p_frame_damaged, h263_aic_dir);
> > +COPYR(h263_slice_structured,mcsel);
> > +COPYR(quant_precision,  first_slice_line);
> > +COPYR(flipflop_rounding,gb);
> > +COPYR(gop_picture_number,   picture_structure);
> > +COPYR(picture_structure,pblocks);
> > +COPYR(decode_mb,er);
> > +COPYR(error_rate,   noise_reduction);  
> 
> 
> This is very obscure... The obscure part is what happens when fields get
> (through refactoring) reordered or new fields get inserted...
> 
> I also appear to remember that we used to use this same pattern for h264
> also, but we don't anymore. Does anyone remember why?

Do you mean 0ba471d7d864c712f45d7ac6aca4829aba025adc?

h264: eliminate copy_fields

It is very fragile against fields being moved and hides what is actually
being copied. Copy all the fields explicitly instead.

Seems justification enough for me.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo: Fix all but one tsan warning in fate-vsynth1-mpeg4

[wm4]

On Mon, 10 Jul 2017 03:19:51 +0200
Michael Niedermayer  wrote:

> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/mpegvideo.c | 15 ++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index e29558b3a2..574b3854e0 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -495,7 +495,20 @@ int ff_mpeg_update_thread_context(AVCodecContext *dst,
>  // in that case dst may need a reinit
>  if (!s->context_initialized) {
>  int err;
> -memcpy(s, s1, sizeof(MpegEncContext));
> +#define COPY(x) s->x = s1->x;
> +#define COPYR(a, b) memcpy(>a, >a, ((uint8_t*)>b) - 
> ((uint8_t*)>a))
> +COPYR(h263_aic, qscale);
> +COPYR(lambda,   mv_dir);
> +COPYR(no_rounding,  dest);
> +COPYR(mb_index2xy,  resync_mb_x);
> +COPYR(next_p_frame_damaged, h263_aic_dir);
> +COPYR(h263_slice_structured,mcsel);
> +COPYR(quant_precision,  first_slice_line);
> +COPYR(flipflop_rounding,gb);
> +COPYR(gop_picture_number,   picture_structure);
> +COPYR(picture_structure,pblocks);
> +COPYR(decode_mb,er);
> +COPYR(error_rate,   noise_reduction);
>  
>  s->avctx = dst;
>  s->bitstream_buffer  = NULL;

Please copy every member on its own, instead of doing a strange range
copy, that will break in subtle and unobvious ways if someone reorders
or adds fields.

I thought we were in a state where we wouldn't add such tricky and
easily broken code anymore.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] avcodec/pthread_slice: rewrite implementation

[wm4]

On Sun,  9 Jul 2017 23:26:54 +0700
Muhammad Faiz  wrote:

> Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> uses distict mutex/cond. Also let main thread help running jobs, but still
> allocate thread_count workers. The last worker is currently unused, emulated
> by main thread.
> Similar to 'avfilter/pthread: rewrite implementation'
> 
> Benchmark on x86_64 with 4 cpus (2 cores x 2 hyperthread)
> ./ffmpeg -threads $threads -thread_type slice -i 10slices.mp4 -f rawvideo -y 
> /dev/null
> threads=2:
> old: 1m15.888s
> new:  1m5.710s
> threads=3:
> old:  1m6.480s
> new:  1m5.260s
> threads=4:
> old:  1m2.292s
> new:   59.677s
> threads=5:
> old:   58.939s
> new:   55.166s
> 
> Signed-off-by: Muhammad Faiz 
> ---
>  libavcodec/pthread_slice.c | 219 
> +
>  1 file changed, 142 insertions(+), 77 deletions(-)
> 
> diff --git a/libavcodec/pthread_slice.c b/libavcodec/pthread_slice.c
> index 60f5b78..7223205 100644
> --- a/libavcodec/pthread_slice.c
> +++ b/libavcodec/pthread_slice.c
> @@ -22,6 +22,7 @@
>   * @see doc/multithreading.txt
>   */
>  
> +#include 
>  #include "config.h"
>  
>  #include "avcodec.h"
> @@ -38,21 +39,26 @@
>  typedef int (action_func)(AVCodecContext *c, void *arg);
>  typedef int (action_func2)(AVCodecContext *c, void *arg, int jobnr, int 
> threadnr);
>  
> +typedef struct WorkerContext WorkerContext;
> +
>  typedef struct SliceThreadContext {
> -pthread_t *workers;
> +AVCodecContext *avctx;
> +WorkerContext *workers;
>  action_func *func;
>  action_func2 *func2;
>  void *args;
>  int *rets;
> -int job_count;
> -int job_size;
> -
> -pthread_cond_t last_job_cond;
> -pthread_cond_t current_job_cond;
> -pthread_mutex_t current_job_lock;
> -unsigned current_execute;
> -int current_job;
> +unsigned job_count;
> +unsigned job_size;
> +
> +pthread_mutex_t mutex_user;
> +pthread_mutex_t mutex_done;
> +pthread_cond_t cond_done;
> +atomic_uint first_job;
> +atomic_uint current_job;
> +atomic_uint nb_finished_jobs;
>  int done;
> +int worker_done;
>  
>  int *entries;
>  int entries_count;
> @@ -61,42 +67,55 @@ typedef struct SliceThreadContext {
>  pthread_mutex_t *progress_mutex;
>  } SliceThreadContext;
>  
> -static void* attribute_align_arg worker(void *v)
> +struct WorkerContext {
> +SliceThreadContext  *ctx;
> +pthread_t   thread;
> +pthread_mutex_t mutex;
> +pthread_cond_t  cond;
> +int done;
> +};
> +
> +static unsigned run_jobs(SliceThreadContext *c)
>  {
> -AVCodecContext *avctx = v;
> -SliceThreadContext *c = avctx->internal->thread_ctx;
> -unsigned last_execute = 0;
> -int our_job = c->job_count;
> -int thread_count = avctx->thread_count;
> -int self_id;
> -
> -pthread_mutex_lock(>current_job_lock);
> -self_id = c->current_job++;
> -for (;;){
> -int ret;
> -while (our_job >= c->job_count) {
> -if (c->current_job == thread_count + c->job_count)
> -pthread_cond_signal(>last_job_cond);
> -
> -while (last_execute == c->current_execute && !c->done)
> -pthread_cond_wait(>current_job_cond, 
> >current_job_lock);
> -last_execute = c->current_execute;
> -our_job = self_id;
> -
> -if (c->done) {
> -pthread_mutex_unlock(>current_job_lock);
> -return NULL;
> -}
> -}
> -pthread_mutex_unlock(>current_job_lock);
> +unsigned current_job, first_job, nb_finished_jobs;
> +
> +current_job = first_job = atomic_fetch_add_explicit(>first_job, 1, 
> memory_order_acq_rel);
>  
> -ret = c->func ? c->func(avctx, (char*)c->args + our_job*c->job_size):
> -c->func2(avctx, c->args, our_job, self_id);
> +do {
> +int ret = c->func ? c->func(c->avctx, (char *)c->args + current_job 
> * (size_t) c->job_size)
> +  : c->func2(c->avctx, c->args, current_job, 
> first_job);
>  if (c->rets)
> -c->rets[our_job%c->job_count] = ret;
> +c->rets[current_job] = ret;
> +nb_finished_jobs = atomic_fetch_add_explicit(>nb_finished_jobs, 
> 1, memory_order_relaxed) + 1;
> +} while ((current_job = atomic_fetch_add_explicit(>current_job, 1, 
> memory_order_acq_rel)) < c->job_count);
>  
> -pthread_mutex_lock(>current_job_lock);
> -our_job = c->current_job++;
> +return nb_finished_jobs;
> +}
> +
> +static void* attribute_align_arg worker(void *v)
> +{
> +WorkerContext *w = v;
> +SliceThreadContext *c = w->ctx;
> +
> +pthread_mutex_lock(>mutex);
> +pthread_cond_signal(>cond);
> +
> +while (1) {
> +w->done = 1;
> +while (w->done)
> +pthread_cond_wait(>cond, >mutex);
> +
> +if 

Re: [FFmpeg-devel] Ticket 6375, Too many packets buffered for output stream

[wm4]

On Sun, 9 Jul 2017 13:38:10 +0200
Reimar Döffinger  wrote:

> On 09.07.2017, at 13:09, Michael Niedermayer  wrote:
> 
> > Hi all
> > 
> > It does appear this regression affects alot of people, judging from
> > the multiple different people in the ticket.
> > Also people ask me about this, for example baptiste yesterday hit it
> > too.
> > 
> > I belive multiple people where in favour of the change that caused
> > this regression. Does someone who was in favor of this change have
> > time to fix this ticket ?
> > 
> > I belive its important to fix this as it seems affecting many people.
> > 
> > Thanks
> > 
> > For reference:
> > Ticket: https://trac.ffmpeg.org/ticket/6375
> > Regressing Commit: 
> > https://github.com/FFmpeg/FFmpeg/commit/af1761f7b5b1b72197dc40934953b775c2d951cc
> >   
> 
> Huh? I don't know if the commit message is accurate, but if it is the basic 
> concept of this patch is utterly broken and can never work.
> There can be hours of video data before you actually get a frame on one of 
> the "missing" streams (subtitles might be the most obvious case, but there 
> are others), and buffering that much data simply is not possible.

That's a libavfilter issue. In my own code, I do some sort of
stream-with-no-data detection to work this around.

The previous design had exactly the same problem, except it was within
libavformat. ffmpeg.c used the codec parameters which libavformat
"found" to initialize the filter chain. That means, it used
libavformat's guess what the decoded AVFrames are going to look like.

Now if a stream was "missing" data, libavformat just read more data
ahead (sometimes making opening a stream extremely slow), or filled in
the data with pure guesses. In some cases, libavformat would get
parameters merely by opening a decoder, and e.g. using the sample
format it set in the init function.

Often enough, these parameters were incorrect too. Especially if other
decoders than the default were used (typically external decoders, but
also if you explicitly chose float vs. fixed point decoder - although
it's possible that ffmpeg.c's messy option handling tried to force
that as "probing" decoder in libavformat to aboid this problem).

The best example for this is hardware decoding - libavformat will be
completely wrong about the output format, and ffmpeg.c had to configure
the filter chain again on the first decoded frame. The consequence was
that it was really hard to build fully hardware filter chains. Also
remember the awful hacks QSV used, which were needed not because QSV is
bad, but because ffmpeg.c's filter setup was bad.

In conclusion, what ffmpeg.c did before this change was certainly more
broken than after this change.

Also I don't know how often I explained this shit over and over again.

Anyway, there is no real problem for ffmpeg.c. It's not like it
behaved well with missing/sparse streams at any time. And in most cases,
just enlarging the queue size will help. If you really care about the
remaining cases, you could add code to set dummy filter params if no
data is available. 

> You can do something like it if instead of failing when the buffering limit 
> is reached you then force stream processing of what is available, which is 
> kind of a compromise that usually works well but also makes things a bit 
> unpredictable.
> Though since it seems to cause issues with audio files with cover image 
> there's probably also bugs in the implementation itself, since handling those 
> correctly is entirely possible...
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] Ticket 6375, Too many packets buffered for output stream

[wm4]

On Sun, 9 Jul 2017 13:09:28 +0200
Michael Niedermayer  wrote:

> Hi all
> 
> It does appear this regression affects alot of people, judging from
> the multiple different people in the ticket.
> Also people ask me about this, for example baptiste yesterday hit it
> too.
> 
> I belive multiple people where in favour of the change that caused
> this regression. Does someone who was in favor of this change have
> time to fix this ticket ?
> 
> I belive its important to fix this as it seems affecting many people.
> 
> Thanks
> 
> For reference:
> Ticket: https://trac.ffmpeg.org/ticket/6375
> Regressing Commit: 
> https://github.com/FFmpeg/FFmpeg/commit/af1761f7b5b1b72197dc40934953b775c2d951cc
> Request for more time at the time when the change was pushed:
> http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-March/207775.html
> 

There's no bug, just a severe lack of understanding, and apparently an
inability to read.

There was more than enough time for testing. Before the patch got
applied.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avfilter: add ANSNR filter

[Tobias Rapp]

On 09.07.2017 19:42, Ashish Singh wrote:

Hi, added metadata scores and changed multipe string comparisons to few integer
comparisons.

---
 Changelog|   1 +
 doc/filters.texi |  20 +++
 libavfilter/Makefile |   1 +
 libavfilter/allfilters.c |   1 +
 libavfilter/ansnr.h  |  29 
 libavfilter/vf_ansnr.c   | 416 +++
 6 files changed, 468 insertions(+)
 create mode 100644 libavfilter/ansnr.h
 create mode 100644 libavfilter/vf_ansnr.c

diff --git a/Changelog b/Changelog
index 1778980..bfe848a 100644
--- a/Changelog
+++ b/Changelog
@@ -10,6 +10,7 @@ version :
 - config.log and other configuration files moved into ffbuild/ directory
 - update cuvid/nvenc headers to Video Codec SDK 8.0.14
 - afir audio filter
+- ansnr video filter

 version 3.3:
 - CrystalHD decoder moved to new decode API
diff --git a/doc/filters.texi b/doc/filters.texi
index 5985db6..7a0856b 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -4419,6 +4419,26 @@ input reaches end of stream. This will cause problems if 
your encoding
 pipeline drops frames. If you're trying to apply an image as an
 overlay to a video stream, consider the @var{overlay} filter instead.

+@section ansnr
+
+Obtain the average ANSNR (Anti-Noise Signal to Noise
+Ratio) between two input videos.
+
+This filter takes in input two input videos.
+
+Both video inputs must have the same resolution and pixel format for
+this filter to work correctly. Also it assumes that both inputs
+have the same number of frames, which are compared one by one.
+
+The obtained average ANSNR is printed through the logging system.
+
+In the below example the input file @file{main.mpg} being processed is compared
+with the reference file @file{ref.mpg}.
+
+@example
+ffmpeg -i main.mpg -i ref.mpg -lavfi ansnr -f null -
+@end example
+
 @section ass

 Same as the @ref{subtitles} filter, except that it doesn't require libavcodec
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index f7dfe8a..705e5a1 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -124,6 +124,7 @@ OBJS-$(CONFIG_ANULLSINK_FILTER)  += 
asink_anullsink.o
 # video filters
 OBJS-$(CONFIG_ALPHAEXTRACT_FILTER)   += vf_extractplanes.o
 OBJS-$(CONFIG_ALPHAMERGE_FILTER) += vf_alphamerge.o
+OBJS-$(CONFIG_ANSNR_FILTER)  += vf_ansnr.o dualinput.o 
framesync.o
 OBJS-$(CONFIG_ASS_FILTER)+= vf_subtitles.o
 OBJS-$(CONFIG_ATADENOISE_FILTER) += vf_atadenoise.o
 OBJS-$(CONFIG_AVGBLUR_FILTER)+= vf_avgblur.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index cd35ae4..c1f67c4 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -136,6 +136,7 @@ static void register_all(void)

 REGISTER_FILTER(ALPHAEXTRACT,   alphaextract,   vf);
 REGISTER_FILTER(ALPHAMERGE, alphamerge, vf);
+REGISTER_FILTER(ANSNR,  ansnr,  vf);
 REGISTER_FILTER(ASS,ass,vf);
 REGISTER_FILTER(ATADENOISE, atadenoise, vf);
 REGISTER_FILTER(AVGBLUR,avgblur,vf);
diff --git a/libavfilter/ansnr.h b/libavfilter/ansnr.h
new file mode 100644
index 000..44fb3ba
--- /dev/null
+++ b/libavfilter/ansnr.h
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2017 Ronald S. Bultje 
+ * Copyright (c) 2017 Ashish Pratap Singh 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVFILTER_ANSNR_H
+#define AVFILTER_ANSNR_H
+
+static int compute_ansnr(const void *ref, const void *dis, int w,
+ int h, int ref_stride, int dis_stride, double *score,
+ double *score_psnr, double peak, double psnr_max, 
void *ctx);
+
+#endif /* AVFILTER_ANSNR_H */
diff --git a/libavfilter/vf_ansnr.c b/libavfilter/vf_ansnr.c
new file mode 100644
index 000..78c71e1
--- /dev/null
+++ b/libavfilter/vf_ansnr.c
@@ -0,0 +1,416 @@
+/*
+ * Copyright (c) 2017 Ronald S. Bultje 
+ * Copyright (c) 2017 Ashish Pratap Singh 
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU 

Re: [FFmpeg-devel] [PATCH v2] avfilter/pthread: rewrite implementation

[wm4]

On Sat, 8 Jul 2017 01:45:06 +0200
Michael Niedermayer  wrote:

> On Fri, Jul 07, 2017 at 09:04:37PM +0700, Muhammad Faiz wrote:
> > Avoid pthread_cond_broadcast that wakes up all workers. Make each of them
> > uses distict mutex/cond. Also let main thread help running jobs.
> > 
> > Benchmark using afir with threads=5 and 4096 taps fir:
> > channels=1:
> > old:
> > 1849650 decicycles in afir_execute,   2 runs,  0 skips
> > 1525719 decicycles in afir_execute,1024 runs,  0 skips
> > 1546032 decicycles in afir_execute,   16356 runs, 28 skips
> > new:
> > 1495525 decicycles in afir_execute,   2 runs,  0 skips
> >  968897 decicycles in afir_execute,1024 runs,  0 skips
> >  941286 decicycles in afir_execute,   16384 runs,  0 skips
> > 
> > channels=2:
> > old:
> > 3135485 decicycles in afir_execute,   2 runs,  0 skips
> > 1967158 decicycles in afir_execute,1024 runs,  0 skips
> > 1802430 decicycles in afir_execute,   16364 runs, 20 skips
> > new:
> > 1864750 decicycles in afir_execute,   2 runs,  0 skips
> > 1437792 decicycles in afir_execute,1024 runs,  0 skips
> > 1183963 decicycles in afir_execute,   16382 runs,  2 skips
> > 
> > channels=4:
> > old:
> > 4879925 decicycles in afir_execute,   2 runs,  0 skips
> > 3557950 decicycles in afir_execute,1022 runs,  2 skips
> > 3206843 decicycles in afir_execute,   16379 runs,  5 skips
> > new:
> > 2962320 decicycles in afir_execute,   2 runs,  0 skips
> > 2450430 decicycles in afir_execute,1024 runs,  0 skips
> > 2446219 decicycles in afir_execute,   16383 runs,  1 skips
> > 
> > channels=8:
> > old:
> > 6032455 decicycles in afir_execute,   2 runs,  0 skips
> > 4838614 decicycles in afir_execute,1023 runs,  1 skips
> > 4720760 decicycles in afir_execute,   16369 runs, 15 skips
> > new:
> > 5228150 decicycles in afir_execute,   2 runs,  0 skips
> > 4592129 decicycles in afir_execute,1023 runs,  1 skips
> > 4469067 decicycles in afir_execute,   16383 runs,  1 skips  
> 
> this causes a strange change:
> 
> ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg  -vcodec libxavs  -vf scale=80x60 
> -t 1 file3.nut
> 
> results in different files before and after this patch. Neither plays
> i suspect this is not a bug in the patch but something odd elsewhere
> but i dont know

OK so you're saying there's no bug. Something changed, and you're too
lazy to investigate, but I guess he has all time in the world.

So why should he care?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/1] This change adds an encoder for Camera metadata motion. This is a type of sensor data associated with video, such as GPS, acceleration, gyro, and camera orientation. It

[wm4]

On Fri, 7 Jul 2017 15:24:02 -0700
"Louis O'Bryan"  wrote:

> > +static av_cold int encode_init(AVCodecContext *avctx) {  
> > > +// Use dummy values for the height and width.
> > > +avctx->width = DUMMY_ENCODER_SIZE;
> > > +avctx->height = DUMMY_ENCODER_SIZE;
> > > +avctx->max_pixels = DUMMY_ENCODER_SIZE;  
> > What? This makes no sense.  
> 
> Using avcodec_encode_video2() seems to require that the width and height be
> nonzero. What is the recommended way to avoid that?

Well, that API is for video. So I guess it's natural that it errors out
if some basic parameters that would be necessarily always required for
video are not set. There are other APIs for audio and subtitles. Also,
these API functions are deprecated.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] avcodec/aacpsdsp_template: Fixes integer overflow in ps_add_squares_c()

[wm4]

On Sun, 9 Jul 2017 22:03:21 +0200
Reimar Döffinger  wrote:

> On 09.07.2017, at 16:08, "Ronald S. Bultje"  wrote:
> > Hi,
> > 
> > On Sun, Jul 9, 2017 at 4:39 AM, Reimar Döffinger 
> > wrote:
> >   
> >> On 09.07.2017, at 02:52, "Ronald S. Bultje"  wrote:  
> >>> On Sat, Jul 8, 2017 at 5:17 PM, Michael Niedermayer  
> >>   
> >>> wrote:
> >>>   
>  
>  Does anyone object to this patch ?
>  Or does anyone have a better idea on how to fix this ?
>  if not id like to apply it  
> >>> 
> >>> 
> >>> I think Rostislav's point is: why fix it, if it can only happen with
> >>> corrupt input? The before and after situation is identical: garbage in,
> >>> garbage out. If the compiler does funny things that makes the garbage
> >>> slightly differently bad, is that really so devilishly bad? It's still
> >>> garbage. Is anything improved by this?  
> >> 
> >> The way C works, you MUST assume any undefined behaviour can at any point
> >> [..] become exploitable.[..] If you don't like that, C is the wrong
> >> language to use.  
> > 
> > 
> > I think I've read "the boy who cried wolf" a few too many times to my kids,
> > but the form of this discussion is currently too polarizing/political for
> > my taste.  
> 
> That is my reading of the C standard, is that political or even just 
> controversial?
> I mean of course you can ignore standards (see MPEG-4 ASP, and in some ways 
> that was actually fairly reasonable thing to do at the time), and I don't fix 
> every undefined behaviour case in my code when I can't think of any 
> reasonable solution.
> So there is a cost-benefit discussion in principle.
> I believe the cost of not fixing undefined behaviour, just by virtue of going 
> outside what the standard guarantees should be considered fairly high.
> That is an opinion, but is there any disagreement that undefined behaviour is 
> at least an issue of some degree?
> If we can agree on that, then the question would only be how much effort/code 
> ugliness is reasonable.
> There is also the point (which I hope I mentioned in the parts cut out) that 
> just making sure that these cases are not already exploitable right now with 
> the current compiler can in many cases be quite a pain (and does not have 
> tool support), so I think fixing it would often be the lowest-effort method.

The controversial thing is actually the SUINT nonsense. A type is
either signed or unsigned, but not both incompletely intransparent
ways. Michael keeps adding them even though many are against it. 

"SUINTFLOAT" just tops this. Is it a signed int? Unsigned int? A float?
Unsigned float???

Come on, this is a huge load of bullshit.

> I'd also like to point out that even ignoring all that, ubsan + fuzzing is a 
> powerful tool to improve code quality, and I would argue than at least some 
> amount of code complexity is justified just for having them work well.
> And it's also that to my mind the patch did not look THAT bad...

A powerful tool for Michael to churn out patches which make the code
look worse and more tricky, which without doubt will lead ot more new
bugs some time in the future. Sure, security is good, but at this
point I'm even wondering what's the point of this. Realistically,
you'll have to sandbox ffmpeg anyway if you want some minimal security.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel