[FFmpeg-devel] [PATCH] ffmpeg: fix uninitialized return value

[Tobias Rapp]

Signed-off-by: Tobias Rapp 
---
 ffmpeg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ffmpeg.c b/ffmpeg.c
index c47a824..4e58528 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2067,7 +2067,7 @@ static int decode(AVCodecContext *avctx, AVFrame *frame, 
int *got_frame, AVPacke
 
 static int send_frame_to_filters(InputStream *ist, AVFrame *decoded_frame)
 {
-int i, ret;
+int i, ret = 0;
 AVFrame *f;
 
 for (i = 0; i < ist->nb_filters; i++) {
-- 
1.9.1


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

[FFmpeg-devel] [PATCH] lavc/vaapi_hevc: fix scaling list duplicate transfer issue.

[Jun Zhao]

From 4059bc56cba30b26bc194673968c3a51014e315f Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Tue, 22 Nov 2016 15:58:50 +0800
Subject: [PATCH] lavc/vaapi_hevc: fix scaling list duplicate transfer issue.

scaling list is already transfered to raster scan during head parsing,
so no need to transfer it again.

And after this fix, FATE test SLIST_A_Sony_4/SLIST_B_Sony_8/
SLIST_C_Sony_3/SLIST_D_Sony_9 will pass in i965/Skylake.

used the comand:
make HWACCEL='vaapi -vaapi_device /dev/dri/renderD128 \
-hwaccel_output_format yuv420p' fate-hevc -i | \
grep "recipe for target" | wc -l test and verify.

Signed-off-by: Wang, Yi A 
Signed-off-by: Jun Zhao 
---
 libavcodec/vaapi_hevc.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/libavcodec/vaapi_hevc.c b/libavcodec/vaapi_hevc.c
index 62f783e..dd43463 100644
--- a/libavcodec/vaapi_hevc.c
+++ b/libavcodec/vaapi_hevc.c
@@ -265,7 +265,7 @@ static int vaapi_hevc_start_frame(AVCodecContext  
*avctx,
 VAPictureParameterBufferHEVC *pic_param;
 VAIQMatrixBufferHEVC *iq_matrix;
 ScalingList const * scaling_list;
-int i, j, pos;
+int i, j;
 
 ff_dlog(avctx, "vaapi_hevc_start_frame()\n");
 
@@ -293,15 +293,13 @@ static int vaapi_hevc_start_frame(AVCodecContext  
*avctx,
 
 for (i = 0; i < 6; ++i) {
 for (j = 0; j < 16; ++j) {
-pos = 4 * ff_hevc_diag_scan4x4_y[j] + ff_hevc_diag_scan4x4_x[j];
-iq_matrix->ScalingList4x4[i][j] = scaling_list->sl[0][i][pos];
+iq_matrix->ScalingList4x4[i][j] = scaling_list->sl[0][i][j];
 }
 for (j = 0; j < 64; ++j) {
-pos = 8 * ff_hevc_diag_scan8x8_y[j] + ff_hevc_diag_scan8x8_x[j];
-iq_matrix->ScalingList8x8[i][j] = scaling_list->sl[1][i][pos];
-iq_matrix->ScalingList16x16[i][j] = scaling_list->sl[2][i][pos];
+iq_matrix->ScalingList8x8[i][j] = scaling_list->sl[1][i][j];
+iq_matrix->ScalingList16x16[i][j] = scaling_list->sl[2][i][j];
 if (i < 2) {
-iq_matrix->ScalingList32x32[i][j] = scaling_list->sl[3][i * 
3][pos];
+iq_matrix->ScalingList32x32[i][j] = scaling_list->sl[3][i * 
3][j];
 }
 }
 iq_matrix->ScalingListDC16x16[i] = scaling_list->sl_dc[0][i];
-- 
2.9.3

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

[FFmpeg-devel] [PATCH] ffplay -vcodec h264_vda *. error :segmentation fault. bug fix.

[gaochao1]

---
 libavcodec/vda_h264_dec.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/libavcodec/vda_h264_dec.c b/libavcodec/vda_h264_dec.c
index 92839e2..809c2f4 100644
--- a/libavcodec/vda_h264_dec.c
+++ b/libavcodec/vda_h264_dec.c
@@ -226,12 +226,14 @@ static av_cold int vdadec_init(AVCodecContext *avctx)
 ctx->h264_initialized = 1;
 
 for (i = 0; i < MAX_SPS_COUNT; i++) {
-const SPS *sps = (const SPS*)ctx->h264ctx.ps.sps_list[i]->data;
-if (sps && (sps->bit_depth_luma != 8 ||
-sps->chroma_format_idc == 2 ||
-sps->chroma_format_idc == 3)) {
-av_log(avctx, AV_LOG_ERROR, "Format is not supported.\n");
-goto failed;
+if (ctx->h264ctx.ps.sps_list[i] != NULL) {
+const SPS *sps = (const SPS*)ctx->h264ctx.ps.sps_list[i]->data;
+if (sps && (sps->bit_depth_luma != 8 ||
+sps->chroma_format_idc == 2 ||
+sps->chroma_format_idc == 3)) {
+av_log(avctx, AV_LOG_ERROR, "Format is not supported.\n");
+goto failed;
+}
 }
 }
 
-- 
2.3.3

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

Re: [FFmpeg-devel] [PATCH 2/3] avutil/opt: Add AV_OPT_TYPE_UINT64

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 12:29:02AM +0100, Andreas Cadhalpun wrote:
> On 20.11.2016 21:52, Michael Niedermayer wrote:
> > On Sun, Nov 20, 2016 at 08:55:44PM +0100, Andreas Cadhalpun wrote:
> >> On 20.11.2016 12:57, Michael Niedermayer wrote:
> >>> +if (intnum == 1 && d == (double)UINT64_MAX) {
> >>> +*(int64_t *)dst = UINT64_MAX;
> >>
> >> Is there a reason why this uses int64_t, 
> >>
> >>> +} else if (o->max > INT64_MAX + 1ULL && d > INT64_MAX + 1ULL) {
> >>> +*(uint64_t *)dst = (llrint(d - (INT64_MAX + 1ULL)) + 
> >>> (INT64_MAX + 1ULL))*intnum;
> >>
> >> but this uint64_t,
> >>
> >>> +} else {
> >>> +*(int64_t *)dst = llrint(d) * intnum;
> >>
> >> and this again int64_t?
> > 
> > these are left over from the year old patch this is based on
> > fixed locally
> 
> OK. Now I've another question. Why does the check involve o->max?
> Is the out-of-range check at the beginning of write_number not
> sufficient?

removed, this too came from the old patch

thx

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

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato


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

[FFmpeg-devel] Help with adding new adpcm decoder

[James Darnley]

I want to add a decoder for a game's music, specifically Falcom's Xanadu
Next.  I think the audio could be decompressed by adpcm_ms but the
problem comes from the rest of the format.

The file starts with a riff wave header that lies about being pcm and
other values, but I can force the decoder with ffmpeg.  In the data
block (which has an incorrect size) the data is divided into frames and
subframes of constant size (except for the last one).

The frames have an 8 byte header.  The first 4 being a little endian
number of the bytes in the frame, which is always 20480 0x5000 (except
for the last frame).  I am not sure of the meaning of the remaining 4
bytes.  In reality the header is almost always these bytes:
00 50 00 00 20 3e 01 00

The subframes are each 2048 bytes.  Each subframe leads with the
adpcm_ms specifics:
- two 1-byte 'block_predictor' for left and right channels
- two 2-byte 'idelta' values
- four 2-byte starting samples
(14 bytes)

10 subframes in each frame.  A quick and crap diagram:

+---frame
| 8 byte header
+---subframe(s)
|  2048 bytes
| [2048 bytes]

I can probably successfully decode with the existing code in
libavcodec/adpcm.c by adding a new codecand a few new cases to existing
functions.

My trouble is with how the AVPackets are created.  The decoder gets
packets of constant 4096 bytes.  The 8 byte frame header means the
packets drift from the subframes.  As a concrete example, the 2nd
2048-byte subframe would be split between the 1st and 2nd packets (just
8 bytes in the 2nd)

Is it the wave demuxer that splits the packets up like this?  I don't
think I can change that because the riff header lies about the data within.

In that case would I be best by tracking the drift using the adpcm
context structure and decoding what samples I can?  I think I will
attempt that anyway, right now, before I get any feedback.

Thanks,
James



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

[FFmpeg-devel] FFmpeg 3.2.1

[Michael Niedermayer]

Hi all

ill make 3.2.1 soon
if you want to backport something, do so now

thx

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Into a blind darkness they enter who follow after the Ignorance,
they as if into a greater darkness enter who devote themselves
to the Knowledge alone. -- Isha Upanishad


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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[wm4]

On Mon, 21 Nov 2016 15:37:48 -0800
Wan-Teh Chang  wrote:

> Hi,
> 
> This patch makes the one-time initialization in av_get_cpu_flags()
> thread-safe. The data race was reported by ThreadSanitizer.
> 
> Wan-Teh Chang (1):
>   avutil: fix data race in av_get_cpu_flags().
> 
>  libavutil/atomic.c   | 40 
>  libavutil/atomic.h   | 34 --
>  libavutil/atomic_gcc.h   | 33 +
>  libavutil/atomic_suncc.h | 19 +++
>  libavutil/atomic_win32.h | 21 +
>  libavutil/cpu.c  | 41 ++---
>  libavutil/tests/atomic.c | 13 +
>  7 files changed, 180 insertions(+), 21 deletions(-)
> 

Seems like a good idea, but it's probably better until the C11 atomics
from Libav are merged. These can provide relaxed semantics as well.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] ffmpeg: fix uninitialized return value

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 09:16:26AM +0100, Tobias Rapp wrote:
> Signed-off-by: Tobias Rapp 
> ---
>  ffmpeg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

do you have a testcase for this ?

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: fix uninitialized return value

[Tobias Rapp]

On 22.11.2016 14:34, Michael Niedermayer wrote:

On Tue, Nov 22, 2016 at 09:16:26AM +0100, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp 
---
 ffmpeg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


do you have a testcase for this ?


No, I just stumbled over it when reading the code.

Regards,
Tobias

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

Re: [FFmpeg-devel] Help with adding new adpcm decoder

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 12:47:04PM +0100, James Darnley wrote:
> I want to add a decoder for a game's music, specifically Falcom's Xanadu
> Next.  I think the audio could be decompressed by adpcm_ms but the
> problem comes from the rest of the format.
> 
> The file starts with a riff wave header that lies about being pcm and
> other values, but I can force the decoder with ffmpeg.  In the data
> block (which has an incorrect size) the data is divided into frames and
> subframes of constant size (except for the last one).
> 
> The frames have an 8 byte header.  The first 4 being a little endian
> number of the bytes in the frame, which is always 20480 0x5000 (except
> for the last frame).  I am not sure of the meaning of the remaining 4
> bytes.  In reality the header is almost always these bytes:
> 00 50 00 00 20 3e 01 00
> 
> The subframes are each 2048 bytes.  Each subframe leads with the
> adpcm_ms specifics:
> - two 1-byte 'block_predictor' for left and right channels
> - two 2-byte 'idelta' values
> - four 2-byte starting samples
> (14 bytes)
> 
> 10 subframes in each frame.  A quick and crap diagram:
> 
> +---frame
> | 8 byte header
> +---subframe(s)
> |  2048 bytes
> | [2048 bytes]
> 
> I can probably successfully decode with the existing code in
> libavcodec/adpcm.c by adding a new codecand a few new cases to existing
> functions.
> 
> My trouble is with how the AVPackets are created.  The decoder gets
> packets of constant 4096 bytes.  The 8 byte frame header means the
> packets drift from the subframes.  As a concrete example, the 2nd
> 2048-byte subframe would be split between the 1st and 2nd packets (just
> 8 bytes in the 2nd)
> 

> Is it the wave demuxer that splits the packets up like this?  I don't
> think I can change that because the riff header lies about the data within.

block_align controls how the wav demuxer splits packets, setting it
correctly could be one solution

but maybe writing a new demuxer is cleaner, wav is not a format
with packets that contain size values like the 8 byte headers

the 3rd option is a AVParser to take the output from the wav
demuxer and turn it into properly packetized data


[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato


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

Re: [FFmpeg-devel] [PATCH] ffmpeg: fix uninitialized return value

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 02:43:57PM +0100, Tobias Rapp wrote:
> On 22.11.2016 14:34, Michael Niedermayer wrote:
> >On Tue, Nov 22, 2016 at 09:16:26AM +0100, Tobias Rapp wrote:
> >>Signed-off-by: Tobias Rapp 
> >>---
> >> ffmpeg.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >do you have a testcase for this ?
> 
> No, I just stumbled over it when reading the code.

are you sure that the codepath leaving ret uninitialized is possible ?

if not adding a av_assert() may be better

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

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras


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

Re: [FFmpeg-devel] Help with adding new adpcm decoder

[Moritz Barsnick]

On Tue, Nov 22, 2016 at 15:04:46 +0100, Michael Niedermayer wrote:
> On Tue, Nov 22, 2016 at 12:47:04PM +0100, James Darnley wrote:
> > The file starts with a riff wave header that lies about being pcm and
> > other values, but I can force the decoder with ffmpeg.  In the data
> > block (which has an incorrect size) the data is divided into frames and
> > subframes of constant size (except for the last one).
[...]
> but maybe writing a new demuxer is cleaner, wav is not a format
> with packets that contain size values like the 8 byte headers

PCM being encapsulated in packets with "proprietary" header values to
me also sounds like the task for a demuxer. The "codec" underneath,
once the overhead is stripped, looks like plain PCM again, right?

> the 3rd option is a AVParser to take the output from the wav
> demuxer and turn it into properly packetized data

Would that be like an input-bitstream filter?

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

Re: [FFmpeg-devel] [PATCH] ffmpeg: fix uninitialized return value

[Tobias Rapp]

On 22.11.2016 15:06, Michael Niedermayer wrote:

On Tue, Nov 22, 2016 at 02:43:57PM +0100, Tobias Rapp wrote:

On 22.11.2016 14:34, Michael Niedermayer wrote:

On Tue, Nov 22, 2016 at 09:16:26AM +0100, Tobias Rapp wrote:

Signed-off-by: Tobias Rapp 
---
ffmpeg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


do you have a testcase for this ?


No, I just stumbled over it when reading the code.


are you sure that the codepath leaving ret uninitialized is possible ?

if not adding a av_assert() may be better


From my quick glance I assumed that ist->nb_filters could be zero and 
the for-loop is not entered. If that should never be the case I agree 
that an added av_assert(ist->nb_filters > 0) would be better.


Regards,
Tobias

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

[FFmpeg-devel] [PATCH 0/2] cuvid: Add support for P010/P016 output formats

[Philip Langdale]

The latest nvidia drivers support P010/P016 output formats for 10/12bit
content.

Philip Langdale (2):
  libavutil/hwcontext_cuda: Support P010 and P016 formats
  avcodec/cuvid: Add support for P010/P016 as an output surface format

 compat/cuda/dynlink_cuviddec.h |  3 +-
 libavcodec/cuvid.c | 80 +-
 libavutil/hwcontext_cuda.c | 15 +++-
 3 files changed, 80 insertions(+), 18 deletions(-)

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

[FFmpeg-devel] [PATCH 2/2] avcodec/cuvid: Add support for P010/P016 as an output surface format

[Philip Langdale]

The nvidia 375.xx driver introduces support for P016 output surfaces,
for 10bit and 12bit HEVC content (it's also the first driver to support
hardware decoding of 12bit content).

The cuvid api, as far as I can tell, only declares one output format
that they appear to refer to as P016 in the driver strings. Of course,
10bit content in P016 is identical to P010, and it is useful for
compatibility purposes to declare the format to be P010 to work with
other components that only know how to consume P010 (and to avoid
triggering swscale conversions that are lossy when they shouldn't be).

For simplicity, this change does not maintain the previous ability
to output dithered NV12 for 10/12 bit input video - the user will need
to update their driver to decode such videos.

Signed-off-by: Philip Langdale 
---
 compat/cuda/dynlink_cuviddec.h |  3 +-
 libavcodec/cuvid.c | 80 +-
 2 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/compat/cuda/dynlink_cuviddec.h b/compat/cuda/dynlink_cuviddec.h
index 17207bc..9ff2741 100644
--- a/compat/cuda/dynlink_cuviddec.h
+++ b/compat/cuda/dynlink_cuviddec.h
@@ -83,7 +83,8 @@ typedef enum cudaVideoCodec_enum {
  * Video Surface Formats Enums
  */
 typedef enum cudaVideoSurfaceFormat_enum {
-cudaVideoSurfaceFormat_NV12=0   /**< NV12 (currently the only 
supported output format)  */
+cudaVideoSurfaceFormat_NV12=0,  /**< NV12  */
+cudaVideoSurfaceFormat_P016=1   /**< P016  */
 } cudaVideoSurfaceFormat;
 
 /*!
diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
index 65468dd..2b2e8ae 100644
--- a/libavcodec/cuvid.c
+++ b/libavcodec/cuvid.c
@@ -28,6 +28,7 @@
 #include "libavutil/fifo.h"
 #include "libavutil/log.h"
 #include "libavutil/opt.h"
+#include "libavutil/pixdesc.h"
 
 #include "avcodec.h"
 #include "internal.h"
@@ -102,11 +103,53 @@ static int CUDAAPI cuvid_handle_video_sequence(void 
*opaque, CUVIDEOFORMAT* form
 CuvidContext *ctx = avctx->priv_data;
 AVHWFramesContext *hwframe_ctx = (AVHWFramesContext*)ctx->hwframe->data;
 CUVIDDECODECREATEINFO cuinfo;
+int surface_fmt;
+
+enum AVPixelFormat pix_fmts_nv12[3] = { AV_PIX_FMT_CUDA,
+AV_PIX_FMT_NV12,
+AV_PIX_FMT_NONE };
+
+enum AVPixelFormat pix_fmts_p010[3] = { AV_PIX_FMT_CUDA,
+AV_PIX_FMT_P010,
+AV_PIX_FMT_NONE };
+
+enum AVPixelFormat pix_fmts_p016[3] = { AV_PIX_FMT_CUDA,
+AV_PIX_FMT_P016,
+AV_PIX_FMT_NONE };
 
 av_log(avctx, AV_LOG_TRACE, "pfnSequenceCallback, 
progressive_sequence=%d\n", format->progressive_sequence);
 
 ctx->internal_error = 0;
 
+switch (format->bit_depth_luma_minus8) {
+case 0: // 8-bit
+surface_fmt = ff_get_format(avctx, pix_fmts_nv12);
+break;
+case 2: // 10-bit
+surface_fmt = ff_get_format(avctx, pix_fmts_p010);
+break;
+case 4: // 12-bit
+surface_fmt = ff_get_format(avctx, pix_fmts_p016);
+break;
+default:
+av_log(avctx, AV_LOG_ERROR, "unsupported bit depth: %d\n",
+   format->bit_depth_luma_minus8 + 8);
+ctx->internal_error = AVERROR(EINVAL);
+return 0;
+}
+if (surface_fmt < 0) {
+av_log(avctx, AV_LOG_ERROR, "ff_get_format failed: %d\n", surface_fmt);
+ctx->internal_error = AVERROR(EINVAL);
+return 0;
+}
+
+av_log(avctx, AV_LOG_VERBOSE, "Formats: Original: %s | HW: %s | SW: %s\n",
+   av_get_pix_fmt_name(avctx->pix_fmt),
+   av_get_pix_fmt_name(surface_fmt),
+   av_get_pix_fmt_name(avctx->sw_pix_fmt));
+
+avctx->pix_fmt = surface_fmt;
+
 avctx->width = format->display_area.right;
 avctx->height = format->display_area.bottom;
 
@@ -155,7 +198,7 @@ static int CUDAAPI cuvid_handle_video_sequence(void 
*opaque, CUVIDEOFORMAT* form
 hwframe_ctx->width < avctx->width ||
 hwframe_ctx->height < avctx->height ||
 hwframe_ctx->format != AV_PIX_FMT_CUDA ||
-hwframe_ctx->sw_format != AV_PIX_FMT_NV12)) {
+hwframe_ctx->sw_format != avctx->sw_pix_fmt)) {
 av_log(avctx, AV_LOG_ERROR, "AVHWFramesContext is already initialized 
with incompatible parameters\n");
 ctx->internal_error = AVERROR(EINVAL);
 return 0;
@@ -176,7 +219,20 @@ static int CUDAAPI cuvid_handle_video_sequence(void 
*opaque, CUVIDEOFORMAT* form
 
 cuinfo.CodecType = ctx->codec_type = format->codec;
 cuinfo.ChromaFormat = format->chroma_format;
-cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12;
+
+switch (avctx->sw_pix_fmt) {
+case AV_PIX_FMT_NV12:
+cuinfo.OutputFormat = cudaVideoSurfaceFormat_NV12;
+break;
+case AV_PIX_FMT_P010:
+case AV_PIX_FMT_P016:
+cui

[FFmpeg-devel] [PATCH 1/2] libavutil/hwcontext_cuda: Support P010 and P016 formats

[Philip Langdale]

CUVID is now capable of returning 10bit and 12bit decoded content
in P010/P016. Let's support transfering those formats.

Signed-off-by: Philip Langdale 
---
 libavutil/hwcontext_cuda.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/libavutil/hwcontext_cuda.c b/libavutil/hwcontext_cuda.c
index 30de299..2ebf4bc 100644
--- a/libavutil/hwcontext_cuda.c
+++ b/libavutil/hwcontext_cuda.c
@@ -35,6 +35,8 @@ static const enum AVPixelFormat supported_formats[] = {
 AV_PIX_FMT_NV12,
 AV_PIX_FMT_YUV420P,
 AV_PIX_FMT_YUV444P,
+AV_PIX_FMT_P010,
+AV_PIX_FMT_P016,
 };
 
 static void cuda_buffer_free(void *opaque, uint8_t *data)
@@ -111,6 +113,8 @@ static int cuda_frames_init(AVHWFramesContext *ctx)
 size = aligned_width * ctx->height * 3 / 2;
 break;
 case AV_PIX_FMT_YUV444P:
+case AV_PIX_FMT_P010:
+case AV_PIX_FMT_P016:
 size = aligned_width * ctx->height * 3;
 break;
 }
@@ -125,7 +129,14 @@ static int cuda_frames_init(AVHWFramesContext *ctx)
 
 static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame *frame)
 {
-int aligned_width = FFALIGN(ctx->width, CUDA_FRAME_ALIGNMENT);
+int aligned_width;
+int width_in_bytes = ctx->width;
+
+if (ctx->sw_format == AV_PIX_FMT_P010 ||
+ctx->sw_format == AV_PIX_FMT_P016) {
+   width_in_bytes *= 2;
+}
+aligned_width = FFALIGN(width_in_bytes, CUDA_FRAME_ALIGNMENT);
 
 frame->buf[0] = av_buffer_pool_get(ctx->pool);
 if (!frame->buf[0])
@@ -133,6 +144,8 @@ static int cuda_get_buffer(AVHWFramesContext *ctx, AVFrame 
*frame)
 
 switch (ctx->sw_format) {
 case AV_PIX_FMT_NV12:
+case AV_PIX_FMT_P010:
+case AV_PIX_FMT_P016:
 frame->data[0] = frame->buf[0]->data;
 frame->data[1] = frame->data[0] + aligned_width * ctx->height;
 frame->linesize[0] = aligned_width;
-- 
2.9.3
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 0/2] cuvid: Add support for P010/P016 output formats

[Philip Langdale]

On Tue, 22 Nov 2016 08:27:46 -0800
Philip Langdale  wrote:

> The latest nvidia drivers support P010/P016 output formats for
> 10/12bit content.
> 
> Philip Langdale (2):
>   libavutil/hwcontext_cuda: Support P010 and P016 formats
>   avcodec/cuvid: Add support for P010/P016 as an output surface format
> 
>  compat/cuda/dynlink_cuviddec.h |  3 +-
>  libavcodec/cuvid.c | 80
> +-
> libavutil/hwcontext_cuda.c | 15 +++- 3 files changed, 80
> insertions(+), 18 deletions(-)
> 

Pushed after irc-review from Timo.

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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Wan-Teh Chang]

Hi Michael,

On Mon, Nov 21, 2016 at 4:35 PM, Michael Niedermayer
 wrote:
> On Mon, Nov 21, 2016 at 03:37:49PM -0800, Wan-Teh Chang wrote:
>> Make the one-time initialization in av_get_cpu_flags() thread-safe.
>> The fix assumes -1 is an invalid value for cpu flags.
>
> please explain the bug / race that occurs and how it is fixed

Note: I will include the following explanation in the next version of my patch.

The static variables |flags| and |checked| in libavutil/cpu.c are read
and written using normal load and store instructions. These are
considered as data races. The fix is to use atomic load and store
instructions. The fix also eliminates the |checked| variable because
the invalid value of -1 for |flags| can be used to indicate the same
condition.

Our application runs into the data races because two threads call
avformat_find_stream_info() at the same time.
avformat_find_stream_info() calls av_parser_init(), which may
eventually call av_get_cpu_flag(), depending on the codec.

I just wrote a minimal test case (libavutil/tests/cpu_init.c) that
reproduces the data races. The test program creates two threads that
call av_get_cpu_flags(). When built with --toolchain=clang-tsan, the
test program triggers two ThreadSanitizer warnings:

$ libavutil/tests/cpu_init
==
WARNING: ThreadSanitizer: data race (pid=66608)
  Read of size 4 at 0x7f7aa8c15d40 by thread T2:
#0 av_get_cpu_flags
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/cpu.c:78
(exe+0x000a8536)
#1 thread_main
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:32
(exe+0x000a8494)

  Previous write of size 4 at 0x7f7aa8c15d40 by thread T1:
#0 av_get_cpu_flags
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/cpu.c:90
(exe+0x000a8578)
#1 thread_main
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:32
(exe+0x000a8494)

  Thread T2 (tid=66611, running) created by main thread at:
#0 pthread_create ??:0 (exe+0x0004d9bb)
#1 main 
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:51
(exe+0x000a83cb)

  Thread T1 (tid=66610, running) created by main thread at:
#0 pthread_create ??:0 (exe+0x0004d9bb)
#1 main 
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:47
(exe+0x000a83a9)

SUMMARY: ThreadSanitizer: data race
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/cpu.c:78
av_get_cpu_flags
==
==
WARNING: ThreadSanitizer: data race (pid=66608)
  Read of size 4 at 0x7f7aa8c15d3c by thread T2:
#0 av_get_cpu_flags
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/cpu.c:79
(exe+0x000a854b)
#1 thread_main
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:32
(exe+0x000a8494)

  Previous write of size 4 at 0x7f7aa8c15d3c by thread T1:
#0 av_get_cpu_flags
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/cpu.c:88
(exe+0x000a8566)
#1 thread_main
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:32
(exe+0x000a8494)

  Thread T2 (tid=66611, running) created by main thread at:
#0 pthread_create ??:0 (exe+0x0004d9bb)
#1 main 
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:51
(exe+0x000a83cb)

  Thread T1 (tid=66610, running) created by main thread at:
#0 pthread_create ??:0 (exe+0x0004d9bb)
#1 main 
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/tests/cpu_init.c:47
(exe+0x000a83a9)

SUMMARY: ThreadSanitizer: data race
/usr/local/google/home/wtc/ffmpeg-cpu-data-race-before/ffmpeg/libavutil/cpu.c:79
av_get_cpu_flags
==
ThreadSanitizer: reported 2 warnings

>> The fix requires new atomic functions to get, set, and compare-and-swap
>> an integer without a memory barrier.
>
> why ?

The fix needs a compare-and-swap function for an atomic integer. There
is no such function in libavutil/atomic.h.

Although the fix can use avpriv_atomic_int_get() and
avpriv_atomic_int_set(), these two functions execute a memory barrier
with sequentially-consistent ordering, which the fix doesn't need. To
improve performance, I added avpriv_atomic_int_get_relaxed() and
avpriv_atomic_int_set_relaxed()

>> The data race fix is written by Dmitry Vyukov of Google.
>
> Then the author for the git patch should be set accordingly

I will look into this. I may just identify him as a co-author.

>> @@ -44,7 +45,20 @@
>>  #include 
>>  #endif
>>
>> -static int flags, checked;
>> +static int cpu_flags = -1;
>> +
>> +static int get_cpu_flags(void)
>> +{
>> +if (ARCH_AARCH64)
>> +return ff_get_cpu_flags_aarch64();
>> +if (ARCH_ARM)
>> +return ff_get_cpu_flags_arm();
>> +if (ARCH_PPC)
>> + 

Re: [FFmpeg-devel] [PATCH 1/2] libvpxenc: Support targeting a VP9 level

[Alex Converse]

On Fri, Nov 18, 2016 at 3:44 PM, James Zern
 wrote:
>
> On Fri, Nov 18, 2016 at 2:01 PM, Alex Converse  
> wrote:
> > Levels are specified at https://www.webmproject.org/vp9/levels/
> > ---
> >  libavcodec/libvpxenc.c | 10 ++
> >  1 file changed, 10 insertions(+)
> >
>
> lgtm

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

[FFmpeg-devel] [PATCH] Make the one-time initialization in av_get_cpu_flags() thread-safe.

[Wan-Teh Chang]

The static variables |flags| and |checked| in libavutil/cpu.c are read
and written using normal load and store instructions. These are
considered as data races. The fix is to use atomic load and store
instructions. The fix also eliminates the |checked| variable because the
invalid value of -1 for |flags| can be used to indicate the same
condition.

The fix requires a new atomic integer compare and swap function. Since
the fix does not need memory barriers, "relaxed" variants of
avpriv_atomic_int_get() and avpriv_atomic_int_set() are also added.

Add a test program libavutil/tests/cpu_init.c to verify the one-time
initialization in av_get_cpu_flags() is thread-safe.

Co-author: Dmitry Vyukov of Google, which suggested the data race fix.

Signed-off-by: Wan-Teh Chang 
---
 libavutil/Makefile |  1 +
 libavutil/atomic.c | 40 ++
 libavutil/atomic.h | 34 --
 libavutil/atomic_gcc.h | 33 +
 libavutil/atomic_suncc.h   | 19 
 libavutil/atomic_win32.h   | 21 ++
 libavutil/cpu.c| 41 ++
 libavutil/cpu.h|  2 --
 libavutil/tests/atomic.c   | 13 +
 libavutil/tests/cpu_init.c | 72 ++
 10 files changed, 253 insertions(+), 23 deletions(-)
 create mode 100644 libavutil/tests/cpu_init.c

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 0fa90fe..732961a 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -187,6 +187,7 @@ TESTPROGS = adler32 
\
 camellia\
 color_utils \
 cpu \
+cpu_init\
 crc \
 des \
 dict\
diff --git a/libavutil/atomic.c b/libavutil/atomic.c
index 64cff25..7bce013 100644
--- a/libavutil/atomic.c
+++ b/libavutil/atomic.c
@@ -40,6 +40,11 @@ int avpriv_atomic_int_get(volatile int *ptr)
 return res;
 }
 
+int avpriv_atomic_int_get_relaxed(volatile int *ptr)
+{
+return avpriv_atomic_int_get(ptr);
+}
+
 void avpriv_atomic_int_set(volatile int *ptr, int val)
 {
 pthread_mutex_lock(&atomic_lock);
@@ -47,6 +52,11 @@ void avpriv_atomic_int_set(volatile int *ptr, int val)
 pthread_mutex_unlock(&atomic_lock);
 }
 
+void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
+{
+avpriv_atomic_int_set(ptr, val);
+}
+
 int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
 {
 int res;
@@ -59,6 +69,17 @@ int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int 
inc)
 return res;
 }
 
+int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
+{
+int ret;
+pthread_mutex_lock(&atomic_lock);
+ret = *ptr;
+if (ret == oldval)
+*ptr = newval;
+pthread_mutex_unlock(&atomic_lock);
+return ret;
+}
+
 void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
 {
 void *ret;
@@ -77,17 +98,36 @@ int avpriv_atomic_int_get(volatile int *ptr)
 return *ptr;
 }
 
+int avpriv_atomic_int_get_relaxed(volatile int *ptr)
+{
+return avpriv_atomic_int_get(ptr);
+}
+
 void avpriv_atomic_int_set(volatile int *ptr, int val)
 {
 *ptr = val;
 }
 
+void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
+{
+avpriv_atomic_int_set(ptr, val);
+}
+
 int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
 {
 *ptr += inc;
 return *ptr;
 }
 
+int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
+{
+if (*ptr == oldval) {
+*ptr = newval;
+return oldval;
+}
+return *ptr;
+}
+
 void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
 {
 if (*ptr == oldval) {
diff --git a/libavutil/atomic.h b/libavutil/atomic.h
index 15906d2..c5aa1a4 100644
--- a/libavutil/atomic.h
+++ b/libavutil/atomic.h
@@ -40,20 +40,38 @@
  *
  * @param ptr atomic integer
  * @return the current value of the atomic integer
- * @note This acts as a memory barrier.
+ * @note This acts as a memory barrier with sequentially-consistent ordering.
  */
 int avpriv_atomic_int_get(volatile int *ptr);
 
 /**
+ * Load the current value stored in an atomic integer.
+ *
+ * @param ptr atomic integer
+ * @return the current value of the atomic integer
+ * @note This does NOT act as a memory barrier.
+ */
+int avpriv_atomic_int_get_relaxed(volatile int *ptr);
+
+/**
  * Store a new value in an atomic integer.
  *
  * @param ptr atomic integer
  * @param val the value to store in the atomic integer
- * @note This acts as a memory barrier.
+ * @note T

[FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Wan-Teh Chang]

Here is a new version of my patch to address Michael's review comments.

Wan-Teh Chang (1):
  Make the one-time initialization in av_get_cpu_flags() thread-safe.

 libavutil/Makefile |  1 +
 libavutil/atomic.c | 40 ++
 libavutil/atomic.h | 34 --
 libavutil/atomic_gcc.h | 33 +
 libavutil/atomic_suncc.h   | 19 
 libavutil/atomic_win32.h   | 21 ++
 libavutil/cpu.c| 41 ++
 libavutil/cpu.h|  2 --
 libavutil/tests/atomic.c   | 13 +
 libavutil/tests/cpu_init.c | 72 ++
 10 files changed, 253 insertions(+), 23 deletions(-)
 create mode 100644 libavutil/tests/cpu_init.c

-- 
2.8.0.rc3.226.g39d4020

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

Re: [FFmpeg-devel] [PATCH 2/2] libvpxenc: Report encoded VP9 level

[James Zern]

On Fri, Nov 18, 2016 at 2:01 PM, Alex Converse  wrote:
> Report the actual level of the encoded output if a level is
> targeted or the level is passively tracked with a target of 0.
> ---
>  libavcodec/libvpxenc.c | 31 +++
>  1 file changed, 31 insertions(+)
>

lgtm.
I don't know if there's a better way to report this at the stream
level (AV_PKT_DATA_STRINGS_METADATA?), there doesn't seem to be
anything specific right now. This info can be translated quickly if
someone wants to make this more structured or has any opinion now.

> +
>  static av_cold int vpx_free(AVCodecContext *avctx)
>  {
>  VPxContext *ctx = avctx->priv_data;
> +int level_out = 0;

could relocate this to avoid unused warnings against older libs.

> +
> +#if VPX_ENCODER_ABI_VERSION >= 12
> +if (ctx->level >= 0 && !(avctx->flags & AV_CODEC_FLAG_PASS1) &&
> +!codecctl_intp(avctx, VP9E_GET_LEVEL, &level_out))
> +  av_log(avctx, AV_LOG_INFO, "Encoded level %.1f\n", level_out * 0.1);

indent is off.

> +#endif
>
>  vpx_codec_destroy(&ctx->encoder);
>  if (ctx->is_alpha)
> --
> 2.8.0.rc3.226.g39d4020
>
> ___
> 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 2/2] libvpxenc: Report encoded VP9 level

[James Zern]

On Tue, Nov 22, 2016 at 12:04 PM, James Zern  wrote:
> On Fri, Nov 18, 2016 at 2:01 PM, Alex Converse  
> wrote:
>> Report the actual level of the encoded output if a level is
>> targeted or the level is passively tracked with a target of 0.
>> ---
>>  libavcodec/libvpxenc.c | 31 +++
>>  1 file changed, 31 insertions(+)
>>
>
> lgtm.
> I don't know if there's a better way to report this at the stream
> level (AV_PKT_DATA_STRINGS_METADATA?), there doesn't seem to be
> anything specific right now. This info can be translated quickly if
> someone wants to make this more structured or has any opinion now.
>

I forgot that there was some discussion around adding this to the
codec extradata in webm. That could be a followup if there's
documentation on the format for that.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH v2] lavc/vaapi_encode_h264: fix poc incorrect issue after meeting idr frame.

[Andy Furniss]

Mark Thompson wrote:

On 14/11/16 07:11, Jun Zhao wrote:

V2 : - Change the last_idr_frame filed location based on Mark code review.
  - Modify the commit message to actually explain the problem.

 From a1bf2b021effd36f8297b331855a282d775f2a44 Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Fri, 11 Nov 2016 14:53:49 +0800
Subject: [PATCH v2] lavc/vaapi_encode_h264: fix poc incorrect issue after
  meeting idr frame.

when meeting IDR frame, vaapi_encode_h264 poc number don't reset, now fix
this issue based on h264 spec. Some decoder don't care this case, but this
fix will enhance the encoder action. Before this fix, poc number is
negative in some case.

Reviewed-by: Jun Zhao 
Signed-off-by: Wang, Yi A 


Applied as 
.


Oh, nice, and thanks for getting all the other patches in ffmpeg.

On AMD this does fix the negative pocs issue, but there may still be
another, as I mentioned on mesa dev, -g 30 makes an IDR followed by 30
P frames. I assume the IDR should be included in the count?



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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 11:28:48AM -0800, Wan-Teh Chang wrote:
> Hi Michael,
> 
> On Mon, Nov 21, 2016 at 4:35 PM, Michael Niedermayer
>  wrote:
> > On Mon, Nov 21, 2016 at 03:37:49PM -0800, Wan-Teh Chang wrote:
> >> Make the one-time initialization in av_get_cpu_flags() thread-safe.
> >> The fix assumes -1 is an invalid value for cpu flags.
> >
> > please explain the bug / race that occurs and how it is fixed
> 
> Note: I will include the following explanation in the next version of my 
> patch.
> 
> The static variables |flags| and |checked| in libavutil/cpu.c are read
> and written using normal load and store instructions. These are
> considered as data races. The fix is to use atomic load and store
> instructions. The fix also eliminates the |checked| variable because
> the invalid value of -1 for |flags| can be used to indicate the same
> condition.
> 
> Our application runs into the data races because two threads call
> avformat_find_stream_info() at the same time.
> avformat_find_stream_info() calls av_parser_init(), which may
> eventually call av_get_cpu_flag(), depending on the codec.
> 
> I just wrote a minimal test case (libavutil/tests/cpu_init.c) that
> reproduces the data races. The test program creates two threads that
> call av_get_cpu_flags(). When built with --toolchain=clang-tsan, the
> test program triggers two ThreadSanitizer warnings:

ok, i see th race but do you really need the atomic operations ?
isnt merging the 2 variables into 1 as you do enough ?
(i mean the tools might still complain but would there be an actual
 race remaining?)


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

There will always be a question for which you do not know the correct answer.


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

[FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Wan-Teh Chang]

Make the one-time initialization in av_get_cpu_flags() thread-safe. The
static variables |flags| and |checked| in libavutil/cpu.c are read and
written using normal load and store operations. These are considered as
data races. The fix is to use atomic load and store operations. The fix
also eliminates the |checked| variable because the invalid value of -1
for |flags| can be used to indicate the same condition.

The fix requires a new atomic integer compare and swap function. Since
the fix does not need memory barriers, "relaxed" variants of
avpriv_atomic_int_get() and avpriv_atomic_int_set() are also added.

Add a test program libavutil/tests/cpu_init.c to verify the one-time
initialization in av_get_cpu_flags() is thread-safe.

Co-author: Dmitry Vyukov of Google, which suggested the data race fix.

Signed-off-by: Wan-Teh Chang 
---
 libavutil/Makefile |  1 +
 libavutil/atomic.c | 40 ++
 libavutil/atomic.h | 34 --
 libavutil/atomic_gcc.h | 33 +
 libavutil/atomic_suncc.h   | 19 
 libavutil/atomic_win32.h   | 21 ++
 libavutil/cpu.c| 41 ++
 libavutil/cpu.h|  2 --
 libavutil/tests/atomic.c   | 13 +
 libavutil/tests/cpu_init.c | 72 ++
 10 files changed, 253 insertions(+), 23 deletions(-)
 create mode 100644 libavutil/tests/cpu_init.c

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 0fa90fe..732961a 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -187,6 +187,7 @@ TESTPROGS = adler32 
\
 camellia\
 color_utils \
 cpu \
+cpu_init\
 crc \
 des \
 dict\
diff --git a/libavutil/atomic.c b/libavutil/atomic.c
index 64cff25..7bce013 100644
--- a/libavutil/atomic.c
+++ b/libavutil/atomic.c
@@ -40,6 +40,11 @@ int avpriv_atomic_int_get(volatile int *ptr)
 return res;
 }
 
+int avpriv_atomic_int_get_relaxed(volatile int *ptr)
+{
+return avpriv_atomic_int_get(ptr);
+}
+
 void avpriv_atomic_int_set(volatile int *ptr, int val)
 {
 pthread_mutex_lock(&atomic_lock);
@@ -47,6 +52,11 @@ void avpriv_atomic_int_set(volatile int *ptr, int val)
 pthread_mutex_unlock(&atomic_lock);
 }
 
+void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
+{
+avpriv_atomic_int_set(ptr, val);
+}
+
 int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
 {
 int res;
@@ -59,6 +69,17 @@ int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int 
inc)
 return res;
 }
 
+int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
+{
+int ret;
+pthread_mutex_lock(&atomic_lock);
+ret = *ptr;
+if (ret == oldval)
+*ptr = newval;
+pthread_mutex_unlock(&atomic_lock);
+return ret;
+}
+
 void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
 {
 void *ret;
@@ -77,17 +98,36 @@ int avpriv_atomic_int_get(volatile int *ptr)
 return *ptr;
 }
 
+int avpriv_atomic_int_get_relaxed(volatile int *ptr)
+{
+return avpriv_atomic_int_get(ptr);
+}
+
 void avpriv_atomic_int_set(volatile int *ptr, int val)
 {
 *ptr = val;
 }
 
+void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
+{
+avpriv_atomic_int_set(ptr, val);
+}
+
 int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
 {
 *ptr += inc;
 return *ptr;
 }
 
+int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
+{
+if (*ptr == oldval) {
+*ptr = newval;
+return oldval;
+}
+return *ptr;
+}
+
 void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
 {
 if (*ptr == oldval) {
diff --git a/libavutil/atomic.h b/libavutil/atomic.h
index 15906d2..c5aa1a4 100644
--- a/libavutil/atomic.h
+++ b/libavutil/atomic.h
@@ -40,20 +40,38 @@
  *
  * @param ptr atomic integer
  * @return the current value of the atomic integer
- * @note This acts as a memory barrier.
+ * @note This acts as a memory barrier with sequentially-consistent ordering.
  */
 int avpriv_atomic_int_get(volatile int *ptr);
 
 /**
+ * Load the current value stored in an atomic integer.
+ *
+ * @param ptr atomic integer
+ * @return the current value of the atomic integer
+ * @note This does NOT act as a memory barrier.
+ */
+int avpriv_atomic_int_get_relaxed(volatile int *ptr);
+
+/**
  * Store a new value in an atomic integer.
  *
  * @param ptr atomic integer
  * @param val the value to store in the ato

Re: [FFmpeg-devel] [PATCH] Make the one-time initialization in av_get_cpu_flags() thread-safe.

[Wan-Teh Chang]

This patch was generated in error. Please ignore it.

Wan-Teh Chang

On Tue, Nov 22, 2016 at 11:30 AM, Wan-Teh Chang  wrote:
> The static variables |flags| and |checked| in libavutil/cpu.c are read
> and written using normal load and store instructions. These are
> considered as data races. The fix is to use atomic load and store
> instructions. The fix also eliminates the |checked| variable because the
> invalid value of -1 for |flags| can be used to indicate the same
> condition.
>
> The fix requires a new atomic integer compare and swap function. Since
> the fix does not need memory barriers, "relaxed" variants of
> avpriv_atomic_int_get() and avpriv_atomic_int_set() are also added.
>
> Add a test program libavutil/tests/cpu_init.c to verify the one-time
> initialization in av_get_cpu_flags() is thread-safe.
>
> Co-author: Dmitry Vyukov of Google, which suggested the data race fix.
>
> Signed-off-by: Wan-Teh Chang 
> ---
>  libavutil/Makefile |  1 +
>  libavutil/atomic.c | 40 ++
>  libavutil/atomic.h | 34 --
>  libavutil/atomic_gcc.h | 33 +
>  libavutil/atomic_suncc.h   | 19 
>  libavutil/atomic_win32.h   | 21 ++
>  libavutil/cpu.c| 41 ++
>  libavutil/cpu.h|  2 --
>  libavutil/tests/atomic.c   | 13 +
>  libavutil/tests/cpu_init.c | 72 
> ++
>  10 files changed, 253 insertions(+), 23 deletions(-)
>  create mode 100644 libavutil/tests/cpu_init.c
>
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 0fa90fe..732961a 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -187,6 +187,7 @@ TESTPROGS = adler32   
>   \
>  camellia\
>  color_utils \
>  cpu \
> +cpu_init\
>  crc \
>  des \
>  dict\
> diff --git a/libavutil/atomic.c b/libavutil/atomic.c
> index 64cff25..7bce013 100644
> --- a/libavutil/atomic.c
> +++ b/libavutil/atomic.c
> @@ -40,6 +40,11 @@ int avpriv_atomic_int_get(volatile int *ptr)
>  return res;
>  }
>
> +int avpriv_atomic_int_get_relaxed(volatile int *ptr)
> +{
> +return avpriv_atomic_int_get(ptr);
> +}
> +
>  void avpriv_atomic_int_set(volatile int *ptr, int val)
>  {
>  pthread_mutex_lock(&atomic_lock);
> @@ -47,6 +52,11 @@ void avpriv_atomic_int_set(volatile int *ptr, int val)
>  pthread_mutex_unlock(&atomic_lock);
>  }
>
> +void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
> +{
> +avpriv_atomic_int_set(ptr, val);
> +}
> +
>  int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
>  {
>  int res;
> @@ -59,6 +69,17 @@ int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int 
> inc)
>  return res;
>  }
>
> +int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
> +{
> +int ret;
> +pthread_mutex_lock(&atomic_lock);
> +ret = *ptr;
> +if (ret == oldval)
> +*ptr = newval;
> +pthread_mutex_unlock(&atomic_lock);
> +return ret;
> +}
> +
>  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
>  {
>  void *ret;
> @@ -77,17 +98,36 @@ int avpriv_atomic_int_get(volatile int *ptr)
>  return *ptr;
>  }
>
> +int avpriv_atomic_int_get_relaxed(volatile int *ptr)
> +{
> +return avpriv_atomic_int_get(ptr);
> +}
> +
>  void avpriv_atomic_int_set(volatile int *ptr, int val)
>  {
>  *ptr = val;
>  }
>
> +void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
> +{
> +avpriv_atomic_int_set(ptr, val);
> +}
> +
>  int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
>  {
>  *ptr += inc;
>  return *ptr;
>  }
>
> +int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
> +{
> +if (*ptr == oldval) {
> +*ptr = newval;
> +return oldval;
> +}
> +return *ptr;
> +}
> +
>  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
>  {
>  if (*ptr == oldval) {
> diff --git a/libavutil/atomic.h b/libavutil/atomic.h
> index 15906d2..c5aa1a4 100644
> --- a/libavutil/atomic.h
> +++ b/libavutil/atomic.h
> @@ -40,20 +40,38 @@
>   *
>   * @param ptr atomic integer
>   * @return the current value of the atomic integer
> - * @note This acts as a memory barrier.
> + * @note This acts as a memory barrier with sequentially-consistent ordering.
>   */
>  int avpriv_atomic_int_get(volatile int *ptr);
>
>  /**
> + * Load the current value stored in an atomic in

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[wm4]

On Tue, 22 Nov 2016 13:36:11 -0800
Wan-Teh Chang  wrote:

> Make the one-time initialization in av_get_cpu_flags() thread-safe. The
> static variables |flags| and |checked| in libavutil/cpu.c are read and
> written using normal load and store operations. These are considered as
> data races. The fix is to use atomic load and store operations. The fix
> also eliminates the |checked| variable because the invalid value of -1
> for |flags| can be used to indicate the same condition.
> 
> The fix requires a new atomic integer compare and swap function. Since
> the fix does not need memory barriers, "relaxed" variants of
> avpriv_atomic_int_get() and avpriv_atomic_int_set() are also added.
> 
> Add a test program libavutil/tests/cpu_init.c to verify the one-time
> initialization in av_get_cpu_flags() is thread-safe.
> 
> Co-author: Dmitry Vyukov of Google, which suggested the data race fix.
> 
> Signed-off-by: Wan-Teh Chang 
> ---
>  libavutil/Makefile |  1 +
>  libavutil/atomic.c | 40 ++
>  libavutil/atomic.h | 34 --
>  libavutil/atomic_gcc.h | 33 +
>  libavutil/atomic_suncc.h   | 19 
>  libavutil/atomic_win32.h   | 21 ++
>  libavutil/cpu.c| 41 ++
>  libavutil/cpu.h|  2 --
>  libavutil/tests/atomic.c   | 13 +
>  libavutil/tests/cpu_init.c | 72 
> ++
>  10 files changed, 253 insertions(+), 23 deletions(-)
>  create mode 100644 libavutil/tests/cpu_init.c
> 
> diff --git a/libavutil/Makefile b/libavutil/Makefile
> index 0fa90fe..732961a 100644
> --- a/libavutil/Makefile
> +++ b/libavutil/Makefile
> @@ -187,6 +187,7 @@ TESTPROGS = adler32   
>   \
>  camellia\
>  color_utils \
>  cpu \
> +cpu_init\
>  crc \
>  des \
>  dict\
> diff --git a/libavutil/atomic.c b/libavutil/atomic.c
> index 64cff25..7bce013 100644
> --- a/libavutil/atomic.c
> +++ b/libavutil/atomic.c
> @@ -40,6 +40,11 @@ int avpriv_atomic_int_get(volatile int *ptr)
>  return res;
>  }
>  
> +int avpriv_atomic_int_get_relaxed(volatile int *ptr)
> +{
> +return avpriv_atomic_int_get(ptr);
> +}
> +
>  void avpriv_atomic_int_set(volatile int *ptr, int val)
>  {
>  pthread_mutex_lock(&atomic_lock);
> @@ -47,6 +52,11 @@ void avpriv_atomic_int_set(volatile int *ptr, int val)
>  pthread_mutex_unlock(&atomic_lock);
>  }
>  
> +void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
> +{
> +avpriv_atomic_int_set(ptr, val);
> +}
> +
>  int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
>  {
>  int res;
> @@ -59,6 +69,17 @@ int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int 
> inc)
>  return res;
>  }
>  
> +int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
> +{
> +int ret;
> +pthread_mutex_lock(&atomic_lock);
> +ret = *ptr;
> +if (ret == oldval)
> +*ptr = newval;
> +pthread_mutex_unlock(&atomic_lock);
> +return ret;
> +}
> +
>  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
>  {
>  void *ret;
> @@ -77,17 +98,36 @@ int avpriv_atomic_int_get(volatile int *ptr)
>  return *ptr;
>  }
>  
> +int avpriv_atomic_int_get_relaxed(volatile int *ptr)
> +{
> +return avpriv_atomic_int_get(ptr);
> +}
> +
>  void avpriv_atomic_int_set(volatile int *ptr, int val)
>  {
>  *ptr = val;
>  }
>  
> +void avpriv_atomic_int_set_relaxed(volatile int *ptr, int val)
> +{
> +avpriv_atomic_int_set(ptr, val);
> +}
> +
>  int avpriv_atomic_int_add_and_fetch(volatile int *ptr, int inc)
>  {
>  *ptr += inc;
>  return *ptr;
>  }
>  
> +int avpriv_atomic_int_cas_relaxed(volatile int *ptr, int oldval, int newval)
> +{
> +if (*ptr == oldval) {
> +*ptr = newval;
> +return oldval;
> +}
> +return *ptr;
> +}
> +
>  void *avpriv_atomic_ptr_cas(void * volatile *ptr, void *oldval, void *newval)
>  {
>  if (*ptr == oldval) {
> diff --git a/libavutil/atomic.h b/libavutil/atomic.h
> index 15906d2..c5aa1a4 100644
> --- a/libavutil/atomic.h
> +++ b/libavutil/atomic.h
> @@ -40,20 +40,38 @@
>   *
>   * @param ptr atomic integer
>   * @return the current value of the atomic integer
> - * @note This acts as a memory barrier.
> + * @note This acts as a memory barrier with sequentially-consistent ordering.
>   */
>  int avpriv_atomic_int_get(volatile int *ptr);
>  
>  /**
> + * Load the current value stored

Re: [FFmpeg-devel] [PATCH] ffmdec: sanitize codec parameters

[Andreas Cadhalpun]

On 22.11.2016 01:27, Carl Eugen Hoyos wrote:
> 2016-11-22 0:40 GMT+01:00 Andreas Cadhalpun 
> :
> 
>> For example what should be done about overflows, e.g. when
>> calculating the bit rate? Does this count as demuxer failing?
> 
> I don't understand this question:
> There are formats for which we don't know the specification (or
> it may not exist): Of course we always want to fix all undefined
> behaviour, all crashes and similar - this is not related to the
> specifications in question.

The fundamental problem is that multiplying two int32_t variables
does not always fit in an int32_t. If the result of the multiplication
is too large, it wraps around into a negative value, that can cause
problems later on.
In a way this is similar to reading a negative value from a file,
and could be handled with a warning by default, however it could
also indicate a limitation in our framework and just setting the
value to 0 in case of overflow could cause the file to be decoded
incorrectly. Thus it might be better to always error out in such
cases.

> FFmpeg should never by default refuse to decode media files
> that can be decoded and it should never stop reading such files.

'Never' is certainly not correct, since e.g. the ffm muxer is a
special case, as you agreed. Also there are currently many cases
in which FFmpeg errors out, even though it could try to ignore
the problem.

>> And what should be done if the spec says a field is unsigned,
>> but our framework only supports the signed variant?
> 
> Is there a sample for which this makes a difference? If yes, we
> should try to fix it.

There are certainly fuzzed samples, where this is an issue.
How do you propose to fix this?

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Wan-Teh Chang]

Hi Michael,

On Tue, Nov 22, 2016 at 1:22 PM, Michael Niedermayer
 wrote:
>
> ok, i see th race but do you really need the atomic operations ?
> isnt merging the 2 variables into 1 as you do enough ?
> (i mean the tools might still complain but would there be an actual
>  race remaining?)

The atomic operations avoid the "undefined behavior" resulting from
the data races in the C source code. ThreadSanitizer analyzes the C
source code, therefore it must warn about what may be undefined
behavior according to the C standard, even though for a particular
compiler and processor, the generated code is the same.

Here is a small test program that shows gcc generates the same x86_64
assembly code for the normal and atomic (with relaxed memory ordering)
load and store operations:

==
$ gcc --version
gcc (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ cat flags.c
static int flags;

int get_flags_nonatomic(void)
{
return flags;
}

int get_flags_atomic_relaxed(void)
{
return __atomic_load_n(&flags, __ATOMIC_RELAXED);
}

void set_flags_nonatomic(int val)
{
flags = val;
}

void set_flags_atomic_relaxed(int val)
{
__atomic_store_n(&flags, val, __ATOMIC_RELAXED);
}
$ gcc -Wall -O3 -std=c11 -S -c flags.c
$ cat flags.s
.file "flags.c"
.text
.p2align 4,,15
.globl get_flags_nonatomic
.type get_flags_nonatomic, @function
get_flags_nonatomic:
.LFB0:
.cfi_startproc
movl flags(%rip), %eax
ret
.cfi_endproc
.LFE0:
.size get_flags_nonatomic, .-get_flags_nonatomic
.p2align 4,,15
.globl get_flags_atomic_relaxed
.type get_flags_atomic_relaxed, @function
get_flags_atomic_relaxed:
.LFB1:
.cfi_startproc
movl flags(%rip), %eax
ret
.cfi_endproc
.LFE1:
.size get_flags_atomic_relaxed, .-get_flags_atomic_relaxed
.p2align 4,,15
.globl set_flags_nonatomic
.type set_flags_nonatomic, @function
set_flags_nonatomic:
.LFB2:
.cfi_startproc
movl %edi, flags(%rip)
ret
.cfi_endproc
.LFE2:
.size set_flags_nonatomic, .-set_flags_nonatomic
.p2align 4,,15
.globl set_flags_atomic_relaxed
.type set_flags_atomic_relaxed, @function
set_flags_atomic_relaxed:
.LFB3:
.cfi_startproc
movl %edi, flags(%rip)
ret
.cfi_endproc
.LFE3:
.size set_flags_atomic_relaxed, .-set_flags_atomic_relaxed
.local flags
.comm flags,4,16
.ident "GCC: (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4"
.section .note.GNU-stack,"",@progbits
==

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

Re: [FFmpeg-devel] [PATCH] ffmdec: sanitize codec parameters

[Andreas Cadhalpun]

On 22.11.2016 01:35, Carl Eugen Hoyos wrote:
> 2016-11-22 0:06 GMT+01:00 Andreas Cadhalpun 
> :
>> I tend to silently accept zero, because I'd like a
>> consistent solution and in some cases rejecting zero
>> causes FATE failures.
> 
> If there were a bug, you should of course fix fate, fate
> alone is generally no argument in favour or against a patch.

The cases I mean don't look like bugs, just like cases, where it
is expected that some files set certain codec parameters to 0.

>> Because either our demuxer has a bug and should be fixed,
>> or some other tool created broken files, and if ffmpeg
>> informs it's users about that, they can try to get that
>> tool fixed.
> 
> (Träum weiter)
> Seriously: We have an uncountable amount of files that
> do not respect some "specification", many of them apparently
> by intention, many written by applications that do not exist
> since a long time, and some written by applications that have
> fixed the issue meanwhile: Of course it can be a (very) good
> thing to print warnings but in general, we should try to fix a
> few of the thousands of known bugs in FFmpeg and not
> spend infinite time on possible issues in other applications.
> 
> Sorry if I just misunderstand you.

I didn't want to suggest that we fix other applications, just
that users of ffmpeg can realize there is a problem in another
application, if files produced by that application trigger
a warning in ffmpeg.
(Of course this also applies to ffmpeg's muxers.)

Best regards,
Andreas


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

Re: [FFmpeg-devel] [PATCH] configure: reserve a register for gcc before 5 on x86_32 with PIE

[Andreas Cadhalpun]

On 22.11.2016 01:41, Carl Eugen Hoyos wrote:
> 2016-11-22 0:20 GMT+01:00 Andreas Cadhalpun 
> :
>> On 20.11.2016 21:27, Carl Eugen Hoyos wrote:
> 
>>> But --enable-sdl2 made no difference at all, or do I misunderstand?
>>
>> Indeed, I just thought it would. Isn't that a bug?
> 
> It is neither a bug nor not a bug, it is just how FFmpeg's configure
> works for approximately a decade.

Just that a bug existed for a long time does not mean it is not a bug.

> Please remove --enable-sdl2 because it makes people think it
> works the way you expected it to work.

The problem is really not that people think it works that way, but
rather that it actually works in a very confusing way.
Using --enable-sdl2 while libsdl2-dev is not installed does not cause
a configure failure, as it should, but instead causes the build to
fail later. Thus it achieves the desired end result, but in a very
confusing way. I really think this should be fixed.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] avutil/opt: Add AV_OPT_TYPE_UINT64

[Andreas Cadhalpun]

On 22.11.2016 12:06, Michael Niedermayer wrote:
> On Tue, Nov 22, 2016 at 12:29:02AM +0100, Andreas Cadhalpun wrote:
>> OK. Now I've another question. Why does the check involve o->max?
>> Is the out-of-range check at the beginning of write_number not
>> sufficient?
> 
> removed, this too came from the old patch

Thanks. The patch should be OK then.

Best regards,
Andreas

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

[FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[Andreas Cadhalpun]

Signed-off-by: Andreas Cadhalpun 
---
 configure | 4 
 1 file changed, 4 insertions(+)

diff --git a/configure b/configure
index 7d094df..0d85a54 100755
--- a/configure
+++ b/configure
@@ -5846,7 +5846,9 @@ if enabled gcrypt; then
 fi
 
 disabled sdl && disable sdl2
+enabled sdl2 && sdl_requested=yes
 if ! disabled sdl2; then
+disable sdl2
 SDL2_CONFIG="${cross_prefix}sdl2-config"
 if check_pkg_config sdl2 SDL_events.h SDL_PollEvent; then
 check_cpp_condition SDL.h "(SDL_MAJOR_VERSION<<16 | 
SDL_MINOR_VERSION<<8 | SDL_PATCHLEVEL) >= 0x020001" $sdl2_cflags &&
@@ -5867,6 +5869,8 @@ if ! disabled sdl2; then
 fi
 enabled sdl2 && add_cflags $sdl2_cflags && add_extralibs $sdl2_libs
 
+[ "x$sdl_requested" = "xyes" ] && ! enabled sdl2 && die "ERROR: libsdl2 
requested but not found";
+
 disabled securetransport || { check_func SecIdentityCreate 
"-Wl,-framework,CoreFoundation -Wl,-framework,Security" &&
 check_lib2 "Security/SecureTransport.h Security/Security.h" 
"SSLCreateContext SecItemImport" "-Wl,-framework,CoreFoundation 
-Wl,-framework,Security" &&
 enable securetransport; }
-- 
2.10.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] oggparsetheora: set need_context_update when changing codec id

[Andreas Cadhalpun]

On 04.11.2016 22:27, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/oggparsetheora.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/oggparsetheora.c b/libavformat/oggparsetheora.c
> index b14f9f0..078d827 100644
> --- a/libavformat/oggparsetheora.c
> +++ b/libavformat/oggparsetheora.c
> @@ -111,6 +111,8 @@ static int theora_header(AVFormatContext *s, int idx)
>  thp->gpmask  = (1U << thp->gpshift) - 1;
>  
>  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
> +if (st->codecpar->codec_id != AV_CODEC_ID_THEORA)
> +st->internal->need_context_update = 1;
>  st->codecpar->codec_id   = AV_CODEC_ID_THEORA;
>  st->need_parsing  = AVSTREAM_PARSE_HEADERS;
>  }
> 

Ping. I intend to apply this in a few days.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] flvdec: set need_context_update when changing codec id

[Andreas Cadhalpun]

On 04.11.2016 22:28, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/flvdec.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index e53c345..4ba7fc8 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -289,7 +289,9 @@ static int flv_same_video_codec(AVCodecParameters *vpar, 
> int flags)
>  static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
> int flv_codecid, int read)
>  {
> +int ret = 0;
>  AVCodecParameters *par = vstream->codecpar;
> +enum AVCodecID old_codec_id = vstream->codecpar->codec_id;
>  switch (flv_codecid) {
>  case FLV_CODECID_H263:
>  par->codec_id = AV_CODEC_ID_FLV1;
> @@ -317,20 +319,26 @@ static int flv_set_video_codec(AVFormatContext *s, 
> AVStream *vstream,
>  else
>  avio_skip(s->pb, 1);
>  }
> -return 1; // 1 byte body size adjustment for flv_read_packet()
> +ret = 1; // 1 byte body size adjustment for flv_read_packet()
> +break;
>  case FLV_CODECID_H264:
>  par->codec_id = AV_CODEC_ID_H264;
>  vstream->need_parsing = AVSTREAM_PARSE_HEADERS;
> -return 3; // not 4, reading packet type will consume one byte
> +ret = 3; // not 4, reading packet type will consume one byte
> +break;
>  case FLV_CODECID_MPEG4:
>  par->codec_id = AV_CODEC_ID_MPEG4;
> -return 3;
> +ret = 3;
> +break;
>  default:
>  avpriv_request_sample(s, "Video codec (%x)", flv_codecid);
>  par->codec_tag = flv_codecid;
>  }
>  
> -return 0;
> +if (par->codec_id != old_codec_id)
> +vstream->internal->need_context_update = 1;
> +
> +return ret;
>  }
>  
>  static int amf_get_string(AVIOContext *ioc, char *buffer, int buffsize)
> 

Ping. I intend to apply this in a few days.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mpegpicture: use coded_width/coded_height to allocate frame

[Andreas Cadhalpun]

On 07.11.2016 22:32, Andreas Cadhalpun wrote:
> This fixes a heap-buffer-overflow in ff_er_frame_end when decoding mss2 with
> coded_width/coded_height larger than width/height.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/mpegpicture.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
> index 6748fc2..70b4d3c 100644
> --- a/libavcodec/mpegpicture.c
> +++ b/libavcodec/mpegpicture.c
> @@ -108,15 +108,15 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  
> Picture *pic,
>  avctx->codec_id != AV_CODEC_ID_VC1IMAGE  &&
>  avctx->codec_id != AV_CODEC_ID_MSS2) {
>  if (edges_needed) {
> -pic->f->width  = avctx->width  + 2 * EDGE_WIDTH;
> -pic->f->height = avctx->height + 2 * EDGE_WIDTH;
> +pic->f->width  = avctx->coded_width  + 2 * EDGE_WIDTH;
> +pic->f->height = avctx->coded_height + 2 * EDGE_WIDTH;
>  }
>  
>  r = ff_thread_get_buffer(avctx, &pic->tf,
>   pic->reference ? AV_GET_BUFFER_FLAG_REF : 
> 0);
>  } else {
> -pic->f->width  = avctx->width;
> -pic->f->height = avctx->height;
> +pic->f->width  = avctx->coded_width;
> +pic->f->height = avctx->coded_height;
>  pic->f->format = avctx->pix_fmt;
>  r = avcodec_default_get_buffer2(avctx, pic->f, 0);
>  }
> @@ -135,8 +135,8 @@ static int alloc_frame_buffer(AVCodecContext *avctx,  
> Picture *pic,
>   (EDGE_WIDTH >> (i ? chroma_x_shift : 0));
>  pic->f->data[i] += offset;
>  }
> -pic->f->width  = avctx->width;
> -pic->f->height = avctx->height;
> +pic->f->width  = avctx->coded_width;
> +pic->f->height = avctx->coded_height;
>  }
>  
>  if (avctx->hwaccel) {
> 

Ping. It would be good to have this fixed in 3.2.1.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] sbgdec: prevent NULL pointer access

[Andreas Cadhalpun]

On 10.11.2016 22:24, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/sbgdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
> index bb020d7..cbedd12 100644
> --- a/libavformat/sbgdec.c
> +++ b/libavformat/sbgdec.c
> @@ -927,7 +927,7 @@ static void expand_timestamps(void *log, struct 
> sbg_script *s)
>  }
>  }
>  if (s->start_ts == AV_NOPTS_VALUE)
> -s->start_ts = s->opt_start_at_first ? s->tseq[0].ts.t : now;
> +s->start_ts = (s->opt_start_at_first && s->tseq) ? s->tseq[0].ts.t : 
> now;
>  s->end_ts = s->opt_duration ? s->start_ts + s->opt_duration :
>  AV_NOPTS_VALUE; /* may be overridden later by -E option */
>  cur_ts = now;
> 

Ping. It would be good to have this fixed in 3.2.1.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] aacsbr_fixed: prevent sbr gain from being zero

[Andreas Cadhalpun]

On 13.11.2016 21:29, Andreas Cadhalpun wrote:
> It causes division by zero crashes.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/aacsbr_fixed.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/aacsbr_fixed.c b/libavcodec/aacsbr_fixed.c
> index b26314a..d2a7027 100644
> --- a/libavcodec/aacsbr_fixed.c
> +++ b/libavcodec/aacsbr_fixed.c
> @@ -429,6 +429,10 @@ static void sbr_gain_calc(AACContext *ac, 
> SpectralBandReplication *sbr,
>  av_add_sf(FLOAT_1, 
> sbr->e_curr[e][m]),
>  av_add_sf(FLOAT_1, 
> sbr->q_mapped[e][m];
>  }
> +if (sbr->gain[e][m].mant == 0) {
> +sbr->gain[e][m] = FLOAT_1;
> +sbr->gain[e][m].exp = MIN_EXP;
> +}
>  }
>  for (m = sbr->f_tablelim[k] - sbr->kx[1]; m < sbr->f_tablelim[k 
> + 1] - sbr->kx[1]; m++) {
>  sum[0] = av_add_sf(sum[0], sbr->e_origmapped[e][m]);
> 

Ping. It would be good to have this fixed in 3.2.1.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [libav-devel] [PATCH] libopusdec: fix out-of-bounds read

[Andreas Cadhalpun]

On 14.11.2016 21:55, Andreas Cadhalpun wrote:
> From d33ded293d15e8ceab666bea834d436f3a225bcc Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun 
> Date: Mon, 14 Nov 2016 21:41:45 +0100
> Subject: [PATCH] libopusdec: default to stereo for invalid number of channels
> 
> This fixes an out-of-bounds read if avc->channels is 0.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/libopusdec.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
> index acc62f1..61f68ed 100644
> --- a/libavcodec/libopusdec.c
> +++ b/libavcodec/libopusdec.c
> @@ -47,6 +47,12 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
>  int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled;
>  uint8_t mapping_arr[8] = { 0, 1 }, *mapping;
>  
> +if (avc->channels <= 0) {
> +av_log(avc, AV_LOG_WARNING,
> +   "Invalid number of channels %d, defaulting to stereo\n", 
> avc->channels);
> +avc->channels = 2;
> +}
> +
>  avc->sample_rate= 48000;
>  avc->sample_fmt = avc->request_sample_fmt == AV_SAMPLE_FMT_FLT ?
>AV_SAMPLE_FMT_FLT : AV_SAMPLE_FMT_S16;

Ping. It would be good to have this fixed in 3.2.1.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[Carl Eugen Hoyos]

2016-11-22 23:15 GMT+01:00 Andreas Cadhalpun :

>  disabled sdl && disable sdl2
> +enabled sdl2 && sdl_requested=yes

I consider this ugly and it makes sdl different to all other system
libraries: Why?

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

Re: [FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[Andreas Cadhalpun]

On 22.11.2016 23:30, Carl Eugen Hoyos wrote:
> 2016-11-22 23:15 GMT+01:00 Andreas Cadhalpun 
> :
> 
>>  disabled sdl && disable sdl2
>> +enabled sdl2 && sdl_requested=yes
> 
> I consider this ugly

That is very subjective.

> and it makes sdl different to all other system libraries: Why?

Because it avoids confusing users. And the checks for other system
libraries should do this, too.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] pgssubdec: only set w/h/linesize when allocating data

[Andreas Cadhalpun]

On 09.11.2016 23:27, Andreas Cadhalpun wrote:
> Rects with positive w/h/linesize but no data are invalid.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/pgssubdec.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c
> index cef477d..b50b37b 100644
> --- a/libavcodec/pgssubdec.c
> +++ b/libavcodec/pgssubdec.c
> @@ -556,12 +556,13 @@ static int display_end_segment(AVCodecContext *avctx, 
> void *data,
>  
>  sub->rects[i]->x= ctx->presentation.objects[i].x;
>  sub->rects[i]->y= ctx->presentation.objects[i].y;
> -sub->rects[i]->w= object->w;
> -sub->rects[i]->h= object->h;
> -
> -sub->rects[i]->linesize[0] = object->w;
>  
>  if (object->rle) {
> +sub->rects[i]->w= object->w;
> +sub->rects[i]->h= object->h;
> +
> +sub->rects[i]->linesize[0] = object->w;
> +
>  if (object->rle_remaining_len) {
>  av_log(avctx, AV_LOG_ERROR, "RLE data length %u is %u bytes 
> shorter than expected\n",
> object->rle_data_len, object->rle_remaining_len);
> 

Ping. I intend to apply this in a few days.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] oggparsevp8: set need_context_update when changing codec id

[Andreas Cadhalpun]

On 15.11.2016 22:51, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/oggparsevp8.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
> index c534ab1..86495ae 100644
> --- a/libavformat/oggparsevp8.c
> +++ b/libavformat/oggparsevp8.c
> @@ -62,6 +62,7 @@ static int vp8_header(AVFormatContext *s, int idx)
>  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>  st->codecpar->codec_id   = AV_CODEC_ID_VP8;
>  st->need_parsing  = AVSTREAM_PARSE_HEADERS;
> +st->internal->need_context_update = 1;
>  break;
>  case 0x02:
>  if (p[6] != 0x20)
> 

Ping. I intend to apply this in a few days.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] mxfdec: fix NULL pointer dereference in mxf_read_packet_old

[Andreas Cadhalpun]

On 17.11.2016 22:55, Andreas Cadhalpun wrote:
> Metadata streams have priv_data set to NULL.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/mxfdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index a1a79ce..2ad0c28 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -3135,7 +3135,7 @@ static int mxf_read_packet_old(AVFormatContext *s, 
> AVPacket *pkt)
>  if (mxf->nb_index_tables >= 1 && mxf->current_edit_unit < 
> t->nb_ptses) {
>  pkt->dts = mxf->current_edit_unit + t->first_dts;
>  pkt->pts = t->ptses[mxf->current_edit_unit];
> -} else if (track->intra_only) {
> +} else if (track && track->intra_only) {
>  /* intra-only -> PTS = EditUnit.
>   * let utils.c figure out DTS since it can be < PTS if 
> low_delay = 0 (Sony IMX30) */
>  pkt->pts = mxf->current_edit_unit;
> 

Ping. It would be good to have this fixed in 3.2.1.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[Josh de Kock]

On 22/11/2016 22:33, Andreas Cadhalpun wrote:

On 22.11.2016 23:30, Carl Eugen Hoyos wrote:

2016-11-22 23:15 GMT+01:00 Andreas Cadhalpun :


 disabled sdl && disable sdl2
+enabled sdl2 && sdl_requested=yes


I consider this ugly


That is very subjective.



This goes both ways.


and it makes sdl different to all other system libraries: Why?


Because it avoids confusing users. And the checks for other system
libraries should do this, too.



I see no real reason to add this, but neither a reason to block it 
either. I personally think that it should stay as-is, or all other 
system libraries should have the same behavior as well. Consistency is key.


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

Re: [FFmpeg-devel] [PATCH] oggparsevp8: set need_context_update when changing codec id

[James Almer]

On 11/15/2016 6:51 PM, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/oggparsevp8.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
> index c534ab1..86495ae 100644
> --- a/libavformat/oggparsevp8.c
> +++ b/libavformat/oggparsevp8.c
> @@ -62,6 +62,7 @@ static int vp8_header(AVFormatContext *s, int idx)
>  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>  st->codecpar->codec_id   = AV_CODEC_ID_VP8;
>  st->need_parsing  = AVSTREAM_PARSE_HEADERS;
> +st->internal->need_context_update = 1;

I'm not sure i understand what's the use for this or how codec_id can be
anything else, but in any case why is this implemented in a different way
than in your oggparsetheora patch?

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

Re: [FFmpeg-devel] [PATCH] mxfdec: fix NULL pointer dereference in mxf_read_packet_old

[Josh de Kock]

On 22/11/2016 22:43, Andreas Cadhalpun wrote:

On 17.11.2016 22:55, Andreas Cadhalpun wrote:

Metadata streams have priv_data set to NULL.

Signed-off-by: Andreas Cadhalpun 
---
 libavformat/mxfdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index a1a79ce..2ad0c28 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -3135,7 +3135,7 @@ static int mxf_read_packet_old(AVFormatContext *s, 
AVPacket *pkt)
 if (mxf->nb_index_tables >= 1 && mxf->current_edit_unit < 
t->nb_ptses) {
 pkt->dts = mxf->current_edit_unit + t->first_dts;
 pkt->pts = t->ptses[mxf->current_edit_unit];
-} else if (track->intra_only) {
+} else if (track && track->intra_only) {
 /* intra-only -> PTS = EditUnit.
  * let utils.c figure out DTS since it can be < PTS if 
low_delay = 0 (Sony IMX30) */
 pkt->pts = mxf->current_edit_unit;



Ping. It would be good to have this fixed in 3.2.1.

Best regards,
Andreas


LGTM.

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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 02:00:12PM -0800, Wan-Teh Chang wrote:
> Hi Michael,
> 
> On Tue, Nov 22, 2016 at 1:22 PM, Michael Niedermayer
>  wrote:
> >
> > ok, i see th race but do you really need the atomic operations ?
> > isnt merging the 2 variables into 1 as you do enough ?
> > (i mean the tools might still complain but would there be an actual
> >  race remaining?)
> 
> The atomic operations avoid the "undefined behavior" resulting from
> the data races in the C source code. ThreadSanitizer analyzes the C
> source code, therefore it must warn about what may be undefined
> behavior according to the C standard, even though for a particular
> compiler and processor, the generated code is the same.
> 
> Here is a small test program that shows gcc generates the same x86_64
> assembly code for the normal and atomic (with relaxed memory ordering)
> load and store operations:

we have plenty of code that accesses fields without the extra layer
of weak atomics.
For example the progress code in the frame threading.

Can you just merge the varible ?

this also would be easier to backport if anyone wants to backport

If you still want to add weak atomics as in the patch, afterwards on
top iam not against but that is me, i wont apply it if theres no
consensus or clear majority in favour though

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact


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

Re: [FFmpeg-devel] [PATCH] sbgdec: prevent NULL pointer access

[Josh de Kock]

On 22/11/2016 22:22, Andreas Cadhalpun wrote:

On 10.11.2016 22:24, Andreas Cadhalpun wrote:

Signed-off-by: Andreas Cadhalpun 
---
 libavformat/sbgdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
index bb020d7..cbedd12 100644
--- a/libavformat/sbgdec.c
+++ b/libavformat/sbgdec.c
@@ -927,7 +927,7 @@ static void expand_timestamps(void *log, struct sbg_script 
*s)
 }
 }
 if (s->start_ts == AV_NOPTS_VALUE)
-s->start_ts = s->opt_start_at_first ? s->tseq[0].ts.t : now;
+s->start_ts = (s->opt_start_at_first && s->tseq) ? s->tseq[0].ts.t : 
now;
 s->end_ts = s->opt_duration ? s->start_ts + s->opt_duration :
 AV_NOPTS_VALUE; /* may be overridden later by -E option */
 cur_ts = now;



Ping. It would be good to have this fixed in 3.2.1.



I don't see how s->tseq can be NULL unless the functions are externally 
invoked without a proper state (which they shouldn't be because they're 
static).


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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Wan-Teh Chang]

Hi wm4,

On Tue, Nov 22, 2016 at 1:41 PM, wm4  wrote:
>
> Again, once the atomics changes in Libav are merged, the avpriv_atomic_
> additions will have to be deleted, and code using it rewritten.

Thanks for the heads-up. Is there an atomics patch for libav being
reviewed right now?

I inspected the libav code this morning after I read your earlier
message. The current atomics code in libav ignores the memory order
argument, so the "relaxed" memory order isn't implemented in the
current libav.

For example, the current revision of libav/compat/atomics/gcc/stdatomic.h has:

==
#define atomic_store(object, desired)   \
do {\
*(object) = (desired);  \
__sync_synchronize();   \
} while (0)

#define atomic_store_explicit(object, desired, order) \
atomic_store(object, desired)

#define atomic_load(object) \
(__sync_synchronize(), *(object))

#define atomic_load_explicit(object, order) \
atomic_load(object)
==

So I am wondering if there is a libav patch that implements the
|order| argument for atomic_store_explicit() and
atomic_load_explicit().

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

Re: [FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[Andreas Cadhalpun]

On 22.11.2016 23:43, Josh de Kock wrote:
> On 22/11/2016 22:33, Andreas Cadhalpun wrote:
>> Because it avoids confusing users. And the checks for other system
>> libraries should do this, too.
>>
> 
> I see no real reason to add this, but neither a reason to block it either.
> I personally think that it should stay as-is, or all other system libraries
> should have the same behavior as well.

Yes, I thinkg the other system library checks should be changed as well.

> Consistency is key.

However, the current behavior is already not consistent:
If a non-system library is enabled, but not found, configure fails.
If a system library is enabled, but not found, configure succeeds,
but later the build fails.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] libvpxenc: Report encoded VP9 level

[James Zern]

On Tue, Nov 22, 2016 at 12:08 PM, James Zern  wrote:
> On Tue, Nov 22, 2016 at 12:04 PM, James Zern  wrote:
>> On Fri, Nov 18, 2016 at 2:01 PM, Alex Converse  
>> wrote:
>>> Report the actual level of the encoded output if a level is
>>> targeted or the level is passively tracked with a target of 0.
>>> ---
>>>  libavcodec/libvpxenc.c | 31 +++
>>>  1 file changed, 31 insertions(+)
>>>
>>
>> lgtm.
>> I don't know if there's a better way to report this at the stream
>> level (AV_PKT_DATA_STRINGS_METADATA?), there doesn't seem to be
>> anything specific right now. This info can be translated quickly if
>> someone wants to make this more structured or has any opinion now.
>>
>
> I forgot that there was some discussion around adding this to the
> codec extradata in webm. That could be a followup if there's
> documentation on the format for that.

http://wiki.webmproject.org/vp9-codecprivate
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[Wan-Teh Chang]

Hi Michael,

On Tue, Nov 22, 2016 at 2:57 PM, Michael Niedermayer
 wrote:
>
> we have plenty of code that accesses fields without the extra layer
> of weak atomics.
> For example the progress code in the frame threading.

The progress code in the frame threading also has data races as
defined by the C standard, so ThreadSanitizer also warns about it. I
submitted patches for that code to use atomic operations before.

> Can you just merge the varible ?

Yes, I will create a patch to just merge the variables. My goal is to
fix the data races, so I still need to create another patch on top to
use atomic operations.

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

Re: [FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[Josh de Kock]

On 22/11/2016 23:11, Andreas Cadhalpun wrote:

On 22.11.2016 23:43, Josh de Kock wrote:

On 22/11/2016 22:33, Andreas Cadhalpun wrote:

Because it avoids confusing users. And the checks for other system
libraries should do this, too.



I see no real reason to add this, but neither a reason to block it either.
I personally think that it should stay as-is, or all other system libraries
should have the same behavior as well.


Yes, I thinkg the other system library checks should be changed as well.

Consistency is key.


I meant within the two types of libraries we have, system and 
non-system. (Which are different things, and therefore have been handled 
as such).


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

Re: [FFmpeg-devel] [PATCH] oggparsevp8: set need_context_update when changing codec id

[Andreas Cadhalpun]

On 22.11.2016 23:45, James Almer wrote:
> On 11/15/2016 6:51 PM, Andreas Cadhalpun wrote:
>> Otherwise the codec context and codecpar might disagree on the codec id,
>> triggering asserts in av_parser_parse2.
>>
>> Signed-off-by: Andreas Cadhalpun 
>> ---
>>  libavformat/oggparsevp8.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
>> index c534ab1..86495ae 100644
>> --- a/libavformat/oggparsevp8.c
>> +++ b/libavformat/oggparsevp8.c
>> @@ -62,6 +62,7 @@ static int vp8_header(AVFormatContext *s, int idx)
>>  st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>>  st->codecpar->codec_id   = AV_CODEC_ID_VP8;
>>  st->need_parsing  = AVSTREAM_PARSE_HEADERS;
>> +st->internal->need_context_update = 1;
> 
> I'm not sure i understand what's the use for this

Since the introduction of codecpar, the codec_id of a stream is saved in
two places: the codecpar struct and the internal->avctx context.
These have to agree to avoid undefined behavior.

> or how codec_id can be anything else,

The codec_id can be AV_CODEC_ID_NONE before it is set here.

> but in any case why is this implemented in a different way
> than in your oggparsetheora patch?

The way I used there avoids updating the internal context if
it is not strictly required. I'm not sure which way is better,
as these parsers also set width/height and so on, which should
also match between codecpar and internal context.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[wm4]

On Tue, 22 Nov 2016 23:57:15 +0100
Michael Niedermayer  wrote:

> On Tue, Nov 22, 2016 at 02:00:12PM -0800, Wan-Teh Chang wrote:
> > Hi Michael,
> > 
> > On Tue, Nov 22, 2016 at 1:22 PM, Michael Niedermayer
> >  wrote:  
> > >
> > > ok, i see th race but do you really need the atomic operations ?
> > > isnt merging the 2 variables into 1 as you do enough ?
> > > (i mean the tools might still complain but would there be an actual
> > >  race remaining?)  
> > 
> > The atomic operations avoid the "undefined behavior" resulting from
> > the data races in the C source code. ThreadSanitizer analyzes the C
> > source code, therefore it must warn about what may be undefined
> > behavior according to the C standard, even though for a particular
> > compiler and processor, the generated code is the same.
> > 
> > Here is a small test program that shows gcc generates the same x86_64
> > assembly code for the normal and atomic (with relaxed memory ordering)
> > load and store operations:  
> 
> we have plenty of code that accesses fields without the extra layer
> of weak atomics.

> For example the progress code in the frame threading.

Which was recently fixed in Libav AFAIR...

> 
> Can you just merge the varible ?
> 
> this also would be easier to backport if anyone wants to backport
> 
> If you still want to add weak atomics as in the patch, afterwards on
> top iam not against but that is me, i wont apply it if theres no
> consensus or clear majority in favour though
> 
> [...]
> 

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

Re: [FFmpeg-devel] [PATCH] avutil: fix data race in av_get_cpu_flags().

[wm4]

On Tue, 22 Nov 2016 15:05:36 -0800
Wan-Teh Chang  wrote:

> Hi wm4,
> 
> On Tue, Nov 22, 2016 at 1:41 PM, wm4  wrote:
> >
> > Again, once the atomics changes in Libav are merged, the avpriv_atomic_
> > additions will have to be deleted, and code using it rewritten.  
> 
> Thanks for the heads-up. Is there an atomics patch for libav being
> reviewed right now?
> 
> I inspected the libav code this morning after I read your earlier
> message. The current atomics code in libav ignores the memory order
> argument, so the "relaxed" memory order isn't implemented in the
> current libav.
> 
> For example, the current revision of libav/compat/atomics/gcc/stdatomic.h has:
> 
> ==
> #define atomic_store(object, desired)   \
> do {\
> *(object) = (desired);  \
> __sync_synchronize();   \
> } while (0)
> 
> #define atomic_store_explicit(object, desired, order) \
> atomic_store(object, desired)
> 
> #define atomic_load(object) \
> (__sync_synchronize(), *(object))
> 
> #define atomic_load_explicit(object, order) \
> atomic_load(object)
> ==
> 
> So I am wondering if there is a libav patch that implements the
> |order| argument for atomic_store_explicit() and
> atomic_load_explicit().
> 

This is emulation code for compilers which don't provide C11 atomics.
All relevant compilers provide them natively (and presumably implement
the weaker semantics). Except MSVC I guess, but it's well known that
libavcodec runs slower when compiled with MSVC anyway.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] sbgdec: prevent NULL pointer access

[Andreas Cadhalpun]

On 23.11.2016 00:01, Josh de Kock wrote:
> On 22/11/2016 22:22, Andreas Cadhalpun wrote:
>> On 10.11.2016 22:24, Andreas Cadhalpun wrote:
>>> Signed-off-by: Andreas Cadhalpun 
>>> ---
>>>  libavformat/sbgdec.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
>>> index bb020d7..cbedd12 100644
>>> --- a/libavformat/sbgdec.c
>>> +++ b/libavformat/sbgdec.c
>>> @@ -927,7 +927,7 @@ static void expand_timestamps(void *log, struct 
>>> sbg_script *s)
>>>  }
>>>  }
>>>  if (s->start_ts == AV_NOPTS_VALUE)
>>> -s->start_ts = s->opt_start_at_first ? s->tseq[0].ts.t : now;
>>> +s->start_ts = (s->opt_start_at_first && s->tseq) ? s->tseq[0].ts.t 
>>> : now;
>>>  s->end_ts = s->opt_duration ? s->start_ts + s->opt_duration :
>>>  AV_NOPTS_VALUE; /* may be overridden later by -E option */
>>>  cur_ts = now;
>>>
>>
>> Ping. It would be good to have this fixed in 3.2.1.
>>
> 
> I don't see how s->tseq can be NULL unless the functions are externally 
> invoked without
> a proper state (which they shouldn't be because they're static).

It happens with simply using ffprobe on the sample.
The problem is that tseq is only allocated in parse_time_sequence, but
that function is not necessarily called.

Best regards,
Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] mxfdec: fix NULL pointer dereference in mxf_read_packet_old

[Andreas Cadhalpun]

On 22.11.2016 23:50, Josh de Kock wrote:
> On 22/11/2016 22:43, Andreas Cadhalpun wrote:
>> On 17.11.2016 22:55, Andreas Cadhalpun wrote:
>>> Metadata streams have priv_data set to NULL.
>>>
>>> Signed-off-by: Andreas Cadhalpun 
>>> ---
>>>  libavformat/mxfdec.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>>> index a1a79ce..2ad0c28 100644
>>> --- a/libavformat/mxfdec.c
>>> +++ b/libavformat/mxfdec.c
>>> @@ -3135,7 +3135,7 @@ static int mxf_read_packet_old(AVFormatContext *s, 
>>> AVPacket *pkt)
>>>  if (mxf->nb_index_tables >= 1 && mxf->current_edit_unit < 
>>> t->nb_ptses) {
>>>  pkt->dts = mxf->current_edit_unit + t->first_dts;
>>>  pkt->pts = t->ptses[mxf->current_edit_unit];
>>> -} else if (track->intra_only) {
>>> +} else if (track && track->intra_only) {
>>>  /* intra-only -> PTS = EditUnit.
>>>   * let utils.c figure out DTS since it can be < PTS if 
>>> low_delay = 0 (Sony IMX30) */
>>>  pkt->pts = mxf->current_edit_unit;
>>>
>>
>> Ping. It would be good to have this fixed in 3.2.1.
>>
>> Best regards,
>> Andreas
> 
> LGTM.

Pushed.

Best regards,
Andreas
 

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

Re: [FFmpeg-devel] [PATCH] rmdec: validate block alignment

[Michael Niedermayer]

On Fri, Nov 18, 2016 at 10:37:36PM +0100, Andreas Cadhalpun wrote:
> On 18.11.2016 02:44, Michael Niedermayer wrote:
> > On Thu, Nov 17, 2016 at 10:52:30PM +0100, Andreas Cadhalpun wrote:
> >> This fixes division by zero crashes.
> >>
> >> Signed-off-by: Andreas Cadhalpun 
> >> ---
> >>  libavformat/rmdec.c | 4 
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> >> index d175862..4d56529 100644
> >> --- a/libavformat/rmdec.c
> >> +++ b/libavformat/rmdec.c
> >> @@ -934,6 +934,10 @@ ff_rm_parse_packet (AVFormatContext *s, AVIOContext 
> >> *pb,
> >>  
> >>   ast->sub_packet_cnt = 0;
> >>   rm->audio_stream_num = st->index;
> >> +if (st->codecpar->block_align <= 0) {
> >> +av_log(s, AV_LOG_ERROR, "Invalid block alignment %d\n", 
> >> st->codecpar->block_align);
> >> +return AVERROR_INVALIDDATA;
> >> +}
> >>   rm->audio_pkt_cnt = h * w / st->codecpar->block_align;
> > 
> > indention looks odd
> 
> That's because the two lines above and the line below are indented one
> space too much.
> 
> > also i think this is already checked in rm_read_audio_stream_info() is
> > that check somehow broken ?
> 
> That check is fine, but the codecpar can be changed by the API user,
> e.g. when forcing a particular codec_id.

patch probably ok

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

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire


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

Re: [FFmpeg-devel] [PATCH] doc/examples/muxing: Fix av_frame_make_writable usage

[Sam Hocevar]

On Mon, Oct 24, 2016, Sam Hocevar wrote:

> This fixes data corruption issues in codecs that keep references on
> one or several previous frames.

   This doesn't seem to have gotten much attention. Any comments maybe?

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

Re: [FFmpeg-devel] [PATCH] avformat/mpeg: Adjust vid probe threshold to correct mis-detection

[Michael Niedermayer]

On Tue, Nov 15, 2016 at 08:06:42PM +0100, Michael Niedermayer wrote:
> Fixes: _ij.mp3
> 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/mpeg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

applied

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


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

Re: [FFmpeg-devel] [PATCH] sbgdec: prevent NULL pointer access

[Josh de Kock]

On 22/11/2016 23:37, Andreas Cadhalpun wrote:
> On 23.11.2016 00:01, Josh de Kock wrote:
>> On 22/11/2016 22:22, Andreas Cadhalpun wrote:
>>> On 10.11.2016 22:24, Andreas Cadhalpun wrote:
 Signed-off-by: Andreas Cadhalpun 
 ---
  libavformat/sbgdec.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
 index bb020d7..cbedd12 100644
 --- a/libavformat/sbgdec.c
 +++ b/libavformat/sbgdec.c
 @@ -927,7 +927,7 @@ static void expand_timestamps(void *log, struct 
 sbg_script *s)
  }
  }
  if (s->start_ts == AV_NOPTS_VALUE)
 -s->start_ts = s->opt_start_at_first ? s->tseq[0].ts.t : now;
 +s->start_ts = (s->opt_start_at_first && s->tseq) ? 
 s->tseq[0].ts.t : now;
  s->end_ts = s->opt_duration ? s->start_ts + s->opt_duration :
  AV_NOPTS_VALUE; /* may be overridden later by -E option */
  cur_ts = now;

>>>
>>> Ping. It would be good to have this fixed in 3.2.1.
>>>
>>
>> I don't see how s->tseq can be NULL unless the functions are externally 
>> invoked without
>> a proper state (which they shouldn't be because they're static).
> 
> It happens with simply using ffprobe on the sample.
> The problem is that tseq is only allocated in parse_time_sequence, but
> that function is not necessarily called.
>  

Ok. I see that now, at the very least this patch shouldn't have any
adverse effects--LGTM.

-- 
Josh



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

Re: [FFmpeg-devel] [PATCH] smacker: limit recursion depth of smacker_decode_bigtree

[Andreas Cadhalpun]

On 20.11.2016 00:44, Andreas Cadhalpun wrote:
> On 19.11.2016 23:34, Michael Niedermayer wrote:
>> On Sat, Nov 19, 2016 at 05:27:19PM +0100, Andreas Cadhalpun wrote:
>>> diff --git a/libavcodec/smacker.c b/libavcodec/smacker.c
>>> index b8a0c55..be3914b 100644
>>> --- a/libavcodec/smacker.c
>>> +++ b/libavcodec/smacker.c
>>> @@ -129,8 +129,12 @@ static int smacker_decode_tree(GetBitContext *gb, 
>>> HuffContext *hc, uint32_t pref
>>>  /**
>>>   * Decode header tree
>>>   */
>>> -static int smacker_decode_bigtree(GetBitContext *gb, HuffContext *hc, 
>>> DBCtx *ctx)
>>> +static int smacker_decode_bigtree(GetBitContext *gb, HuffContext *hc, 
>>> DBCtx *ctx, int length)
>>>  {
>>> +if(length > 5000) { // Larger length can cause segmentation faults due 
>>> to too deep recursion.
>>> +av_log(NULL, AV_LOG_ERROR, "length too long\n");
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>
>> are you sure this is not too large for some platforms ?
> 
> I don't think it's even possible to make this small enough for all cases,
> as the stack size can be arbitrarily changed with 'ulimit -s'.
> 
> This value was chosen so that it works with the default stack size of 8 MB,
> but if you think that's too much, it can be made smaller.
> 
> Attached is a variant reducing the 5000 to 500 and thus still working
> with a stack size of only 0.8 MB.

I've now pushed this variant with reduced stack size needs.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] rmdec: validate block alignment

[Andreas Cadhalpun]

On 23.11.2016 00:47, Michael Niedermayer wrote:
> On Fri, Nov 18, 2016 at 10:37:36PM +0100, Andreas Cadhalpun wrote:
>> On 18.11.2016 02:44, Michael Niedermayer wrote:
>>> On Thu, Nov 17, 2016 at 10:52:30PM +0100, Andreas Cadhalpun wrote:
 This fixes division by zero crashes.

 Signed-off-by: Andreas Cadhalpun 
 ---
  libavformat/rmdec.c | 4 
  1 file changed, 4 insertions(+)

 diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
 index d175862..4d56529 100644
 --- a/libavformat/rmdec.c
 +++ b/libavformat/rmdec.c
 @@ -934,6 +934,10 @@ ff_rm_parse_packet (AVFormatContext *s, AVIOContext 
 *pb,
  
   ast->sub_packet_cnt = 0;
   rm->audio_stream_num = st->index;
 +if (st->codecpar->block_align <= 0) {
 +av_log(s, AV_LOG_ERROR, "Invalid block alignment %d\n", 
 st->codecpar->block_align);
 +return AVERROR_INVALIDDATA;
 +}
   rm->audio_pkt_cnt = h * w / st->codecpar->block_align;
>>>
>>> indention looks odd
>>
>> That's because the two lines above and the line below are indented one
>> space too much.
>>
>>> also i think this is already checked in rm_read_audio_stream_info() is
>>> that check somehow broken ?
>>
>> That check is fine, but the codecpar can be changed by the API user,
>> e.g. when forcing a particular codec_id.
> 
> patch probably ok

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH] sbgdec: prevent NULL pointer access

[Andreas Cadhalpun]

On 23.11.2016 01:06, Josh de Kock wrote:
> On 22/11/2016 23:37, Andreas Cadhalpun wrote:
>> On 23.11.2016 00:01, Josh de Kock wrote:
>>> On 22/11/2016 22:22, Andreas Cadhalpun wrote:
 On 10.11.2016 22:24, Andreas Cadhalpun wrote:
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/sbgdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/sbgdec.c b/libavformat/sbgdec.c
> index bb020d7..cbedd12 100644
> --- a/libavformat/sbgdec.c
> +++ b/libavformat/sbgdec.c
> @@ -927,7 +927,7 @@ static void expand_timestamps(void *log, struct 
> sbg_script *s)
>  }
>  }
>  if (s->start_ts == AV_NOPTS_VALUE)
> -s->start_ts = s->opt_start_at_first ? s->tseq[0].ts.t : now;
> +s->start_ts = (s->opt_start_at_first && s->tseq) ? 
> s->tseq[0].ts.t : now;
>  s->end_ts = s->opt_duration ? s->start_ts + s->opt_duration :
>  AV_NOPTS_VALUE; /* may be overridden later by -E option 
> */
>  cur_ts = now;
>

 Ping. It would be good to have this fixed in 3.2.1.

>>>
>>> I don't see how s->tseq can be NULL unless the functions are externally 
>>> invoked without
>>> a proper state (which they shouldn't be because they're static).
>>
>> It happens with simply using ffprobe on the sample.
>> The problem is that tseq is only allocated in parse_time_sequence, but
>> that function is not necessarily called.
>>  
> 
> Ok. I see that now, at the very least this patch shouldn't have any
> adverse effects--LGTM.

Pushed.

Best regards,
Andreas

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

Re: [FFmpeg-devel] [PATCH 2/3] avutil/opt: Add AV_OPT_TYPE_UINT64

[Michael Niedermayer]

On Tue, Nov 22, 2016 at 11:06:49PM +0100, Andreas Cadhalpun wrote:
> On 22.11.2016 12:06, Michael Niedermayer wrote:
> > On Tue, Nov 22, 2016 at 12:29:02AM +0100, Andreas Cadhalpun wrote:
> >> OK. Now I've another question. Why does the check involve o->max?
> >> Is the out-of-range check at the beginning of write_number not
> >> sufficient?
> > 
> > removed, this too came from the old patch
> 
> Thanks. The patch should be OK then.

applied

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 3/3] avcodec/options_table: make channel_layouts uint64

[Michael Niedermayer]

On Sun, Nov 20, 2016 at 08:56:18PM +0100, Andreas Cadhalpun wrote:
> On 20.11.2016 12:57, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/options_table.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
> > index 48de667..ee79859 100644
> > --- a/libavcodec/options_table.h
> > +++ b/libavcodec/options_table.h
> > @@ -447,8 +447,8 @@ static const AVOption avcodec_options[] = {
> >  {"timecode_frame_start", "GOP timecode frame start number, in 
> > non-drop-frame format", OFFSET(timecode_frame_start), AV_OPT_TYPE_INT64, 
> > {.i64 = -1 }, -1, INT64_MAX, V|E},
> >  #endif
> >  {"bits_per_raw_sample", NULL, OFFSET(bits_per_raw_sample), 
> > AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX},
> > -{"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_INT64, {.i64 
> > = DEFAULT }, 0, INT64_MAX, A|E|D, "channel_layout"},
> > -{"request_channel_layout", NULL, OFFSET(request_channel_layout), 
> > AV_OPT_TYPE_INT64, {.i64 = DEFAULT }, 0, INT64_MAX, A|D, 
> > "request_channel_layout"},
> > +{"channel_layout", NULL, OFFSET(channel_layout), AV_OPT_TYPE_UINT64, {.i64 
> > = DEFAULT }, 0, UINT64_MAX, A|E|D, "channel_layout"},
> > +{"request_channel_layout", NULL, OFFSET(request_channel_layout), 
> > AV_OPT_TYPE_UINT64, {.i64 = DEFAULT }, 0, UINT64_MAX, A|D, 
> > "request_channel_layout"},
> >  {"rc_max_vbv_use", NULL, OFFSET(rc_max_available_vbv_use), 
> > AV_OPT_TYPE_FLOAT, {.dbl = 0 }, 0.0, FLT_MAX, V|E},
> >  {"rc_min_vbv_use", NULL, OFFSET(rc_min_vbv_overflow_use),  
> > AV_OPT_TYPE_FLOAT, {.dbl = 3 }, 0.0, FLT_MAX, V|E},
> >  {"ticks_per_frame", NULL, OFFSET(ticks_per_frame), AV_OPT_TYPE_INT, {.i64 
> > = 1 }, 1, INT_MAX, A|V|E|D},
> > 
> 
> Probably OK.

applied

thx

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

There will always be a question for which you do not know the correct answer.


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

Re: [FFmpeg-devel] [PATCH] doc/examples/muxing: Fix av_frame_make_writable usage

[wm4]

On Mon, 24 Oct 2016 00:12:00 +0200
Sam Hocevar  wrote:

> This patch moves the av_frame_make_writable() call from fill_yuv_image
> to get_video_frame so that its argument can be the actual frame that
> will be sent to the encoder.
> 
> This fixes data corruption issues in codecs that keep references on
> one or several previous frames.
> 
> Signed-off-by: Sam Hocevar 
> ---
>  doc/examples/muxing.c | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/examples/muxing.c b/doc/examples/muxing.c
> index f1f5bb8..1df5912 100644
> --- a/doc/examples/muxing.c
> +++ b/doc/examples/muxing.c
> @@ -440,15 +440,7 @@ static void open_video(AVFormatContext *oc, AVCodec 
> *codec, OutputStream *ost, A
>  static void fill_yuv_image(AVFrame *pict, int frame_index,
> int width, int height)
>  {
> -int x, y, i, ret;
> -
> -/* when we pass a frame to the encoder, it may keep a reference to it
> - * internally;
> - * make sure we do not overwrite it here
> - */
> -ret = av_frame_make_writable(pict);
> -if (ret < 0)
> -exit(1);
> +int x, y, i;
>  
>  i = frame_index;
>  
> @@ -475,6 +467,11 @@ static AVFrame *get_video_frame(OutputStream *ost)
>STREAM_DURATION, (AVRational){ 1, 1 }) >= 0)
>  return NULL;
>  
> +/* when we pass a frame to the encoder, it may keep a reference to it
> + * internally; make sure we do not overwrite it here */
> +if (av_frame_make_writable(ost->frame) < 0)
> +exit(1);
> +
>  if (c->pix_fmt != AV_PIX_FMT_YUV420P) {
>  /* as we only generate a YUV420P picture, we must convert it
>   * to the codec pixel format if needed */

LGTM, but I'm not in a position to push now.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: fail if sdl2 was requested but not found

[wm4]

On Tue, 22 Nov 2016 23:30:38 +0100
Carl Eugen Hoyos  wrote:

> 2016-11-22 23:15 GMT+01:00 Andreas Cadhalpun 
> :
> 
> >  disabled sdl && disable sdl2
> > +enabled sdl2 && sdl_requested=yes  
> 
> I consider this ugly and it makes sdl different to all other system
> libraries: Why?

Wait a moment, SDL is not a system library. Like, not at all.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [libav-devel] [PATCH] libopusdec: fix out-of-bounds read

[Michael Niedermayer]

On Mon, Nov 14, 2016 at 09:55:15PM +0100, Andreas Cadhalpun wrote:
> On 14.11.2016 20:54, Anton Khirnov wrote:
> > Quoting Andreas Cadhalpun (2016-11-14 20:30:10)
> >> On 14.11.2016 00:01, Luca Barbato wrote:
> >>> On 13/11/2016 19:23, Andreas Cadhalpun wrote:
>  avc->channels can be 0.
> >>>
> >>> 0 and less than zero shouldn't be an error?
> >>
> >> Such values should be rejected, wherever they are set.
> >> However, ensuring that is a larger change I'm currently
> >> working on.
> >> Meanwhile, this patch is a trivial fix for the potential
> >> security problem that can easily be backported.
> > 
> > channels being zero is perfectly valid, it means the caller does not
> > know the channel count and expects the decoder to read it from the
> > bitstream.
> 
> In general code this is correct, however if e.g. the matroska demuxer
> reads an audio stream which claims to have 0 channels, it should
> be rejected as broken.
> 
> > This should fail for codecs that do not store this
> > information in the bitstream, but work fine otherwise.
> > 
> > In the case of opus, the channel count is always known -- when the
> > extradata is present, the channel count is stored there. Otherwise the
> > stream is simple and can be decoded either as mono or stereo, as we
> > want.
> > 
> > The patch does not seem to be doing the right thing -- I think it will
> > simply fail on the opus_multistream_decoder_create() call.
> 
> Correct.
> 
> > What it should do instead is just default to stereo.
> 
> OK, patch doing that is attached.
> 
> > Even better, you could replace the whole extradata parsing block with
> > a call to ff_opus_parse_extradata(), though that would require some
> > refactoring.
> 
> The fix should be easily backportable, which excludes refactoring.
> 
> Best regards,
> Andreas

>  libopusdec.c |6 ++
>  1 file changed, 6 insertions(+)
> 0b663c14f4a6dae3e1da453239dbe429aef7886e  
> 0001-libopusdec-default-to-stereo-for-invalid-number-of-c.patch
> From d33ded293d15e8ceab666bea834d436f3a225bcc Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun 
> Date: Mon, 14 Nov 2016 21:41:45 +0100
> Subject: [PATCH] libopusdec: default to stereo for invalid number of channels
> 
> This fixes an out-of-bounds read if avc->channels is 0.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/libopusdec.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavcodec/libopusdec.c b/libavcodec/libopusdec.c
> index acc62f1..61f68ed 100644
> --- a/libavcodec/libopusdec.c
> +++ b/libavcodec/libopusdec.c
> @@ -47,6 +47,12 @@ static av_cold int libopus_decode_init(AVCodecContext *avc)
>  int ret, channel_map = 0, gain_db = 0, nb_streams, nb_coupled;
>  uint8_t mapping_arr[8] = { 0, 1 }, *mapping;
>  
> +if (avc->channels <= 0) {
> +av_log(avc, AV_LOG_WARNING,
> +   "Invalid number of channels %d, defaulting to stereo\n", 
> avc->channels);
> +avc->channels = 2;
> +}

This looks wrong

opusdec uses ff_opus_parse_extradata() to set the number of channels
from extradata.

The value provided by the demuxer if any should not matter

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Asymptotically faster algorithms should always be preferred if you have
asymptotical amounts of data


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

Re: [FFmpeg-devel] [PATCH] aacsbr_fixed: prevent sbr gain from being zero

[Michael Niedermayer]

On Sun, Nov 13, 2016 at 09:29:11PM +0100, Andreas Cadhalpun wrote:
> It causes division by zero crashes.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavcodec/aacsbr_fixed.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavcodec/aacsbr_fixed.c b/libavcodec/aacsbr_fixed.c
> index b26314a..d2a7027 100644
> --- a/libavcodec/aacsbr_fixed.c
> +++ b/libavcodec/aacsbr_fixed.c
> @@ -429,6 +429,10 @@ static void sbr_gain_calc(AACContext *ac, 
> SpectralBandReplication *sbr,
>  av_add_sf(FLOAT_1, 
> sbr->e_curr[e][m]),
>  av_add_sf(FLOAT_1, 
> sbr->q_mapped[e][m];
>  }
> +if (sbr->gain[e][m].mant == 0) {
> +sbr->gain[e][m] = FLOAT_1;
> +sbr->gain[e][m].exp = MIN_EXP;
> +}

Why is this not not needed for the float code ?
is there a shortcomming in the SoftFloat code ?

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

I have never wished to cater to the crowd; for what I know they do not
approve, and what they approve I do not know. -- Epicurus


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

Re: [FFmpeg-devel] [PATCH] flvdec: set need_context_update when changing codec id

[Michael Niedermayer]

On Fri, Nov 04, 2016 at 10:28:20PM +0100, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun 
> ---
>  libavformat/flvdec.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> index e53c345..4ba7fc8 100644
> --- a/libavformat/flvdec.c
> +++ b/libavformat/flvdec.c
> @@ -289,7 +289,9 @@ static int flv_same_video_codec(AVCodecParameters *vpar, 
> int flags)
>  static int flv_set_video_codec(AVFormatContext *s, AVStream *vstream,
> int flv_codecid, int read)
>  {
> +int ret = 0;
>  AVCodecParameters *par = vstream->codecpar;
> +enum AVCodecID old_codec_id = vstream->codecpar->codec_id;
>  switch (flv_codecid) {
>  case FLV_CODECID_H263:
>  par->codec_id = AV_CODEC_ID_FLV1;
> @@ -317,20 +319,26 @@ static int flv_set_video_codec(AVFormatContext *s, 
> AVStream *vstream,
>  else
>  avio_skip(s->pb, 1);
>  }
> -return 1; // 1 byte body size adjustment for flv_read_packet()
> +ret = 1; // 1 byte body size adjustment for flv_read_packet()
> +break;
>  case FLV_CODECID_H264:
>  par->codec_id = AV_CODEC_ID_H264;
>  vstream->need_parsing = AVSTREAM_PARSE_HEADERS;
> -return 3; // not 4, reading packet type will consume one byte
> +ret = 3; // not 4, reading packet type will consume one byte
> +break;
>  case FLV_CODECID_MPEG4:
>  par->codec_id = AV_CODEC_ID_MPEG4;
> -return 3;
> +ret = 3;
> +break;
>  default:
>  avpriv_request_sample(s, "Video codec (%x)", flv_codecid);
>  par->codec_tag = flv_codecid;
>  }
>  
> -return 0;
> +if (par->codec_id != old_codec_id)
> +vstream->internal->need_context_update = 1;

If this occurs only for fuzzed samples then rejecting the change
with a request for a sample seems better

changing teh codec_id midstream like this could, seems problematic
changing at at header time might be ok if that works better than not
changing it for some real sample

but maybe iam missing something

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

I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates


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

Re: [FFmpeg-devel] [PATCH] doc/examples/muxing: Fix av_frame_make_writable usage

[Michael Niedermayer]

On Wed, Nov 23, 2016 at 02:09:14AM +0100, wm4 wrote:
> On Mon, 24 Oct 2016 00:12:00 +0200
> Sam Hocevar  wrote:
> 
> > This patch moves the av_frame_make_writable() call from fill_yuv_image
> > to get_video_frame so that its argument can be the actual frame that
> > will be sent to the encoder.
> > 
> > This fixes data corruption issues in codecs that keep references on
> > one or several previous frames.
> > 
> > Signed-off-by: Sam Hocevar 
> > ---
> >  doc/examples/muxing.c | 15 ++-
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/doc/examples/muxing.c b/doc/examples/muxing.c
> > index f1f5bb8..1df5912 100644
> > --- a/doc/examples/muxing.c
> > +++ b/doc/examples/muxing.c
> > @@ -440,15 +440,7 @@ static void open_video(AVFormatContext *oc, AVCodec 
> > *codec, OutputStream *ost, A
> >  static void fill_yuv_image(AVFrame *pict, int frame_index,
> > int width, int height)
> >  {
> > -int x, y, i, ret;
> > -
> > -/* when we pass a frame to the encoder, it may keep a reference to it
> > - * internally;
> > - * make sure we do not overwrite it here
> > - */
> > -ret = av_frame_make_writable(pict);
> > -if (ret < 0)
> > -exit(1);
> > +int x, y, i;
> >  
> >  i = frame_index;
> >  
> > @@ -475,6 +467,11 @@ static AVFrame *get_video_frame(OutputStream *ost)
> >STREAM_DURATION, (AVRational){ 1, 1 }) >= 0)
> >  return NULL;
> >  
> > +/* when we pass a frame to the encoder, it may keep a reference to it
> > + * internally; make sure we do not overwrite it here */
> > +if (av_frame_make_writable(ost->frame) < 0)
> > +exit(1);
> > +
> >  if (c->pix_fmt != AV_PIX_FMT_YUV420P) {
> >  /* as we only generate a YUV420P picture, we must convert it
> >   * to the codec pixel format if needed */
> 
> LGTM, but I'm not in a position to push now.

applied

thx

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

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato


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

[FFmpeg-devel] [PATCH] web/consulting: add myself

[Steven Liu]

Been getting occassional consulting help emails for a while, so I might
as well add myself here.

Signed-off-by: Steven Liu 
---
 src/consulting |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/src/consulting b/src/consulting
index b18ad22..c04b7b4 100644
--- a/src/consulting
+++ b/src/consulting
@@ -144,4 +144,17 @@ E.g.:
   
  

+  
+
+  Steven Liu
+  
+Steven is located in BeiJing, China. He works with FFmpeg since
+2010 and is a maintainer since 2016. The focus of his work is on 
network
+protocols and all areas of audio/video streaming. He has special 
expertise
+with HTTP, RTMP, HLS, MP4, FLV, MPEGTS, libavformat, ff* tools usage 
and usability issues.
+You can contact him by email at
+mailto:l...@chinaffmpeg.org";>l...@chinaffmpeg.org.
+  
+ 
+   
  
-- 
1.7.1



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

[FFmpeg-devel] About swr_convert_frame of FFmpeg 3.2 Release

[He Lei]

I convert audio frame from s16p to s16 used swr_convert_frame; I found the 
out_frame ->linesize[0] is more then in_frame;
After view the code,  I don’t get it ; At lines 144 of file 
swr_convert_frame.c, Why to add 3 when calculated nb_samples?



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

[FFmpeg-devel] 答复: About swr_convert_frame of FFmpeg 3.2 Release

[He Lei]

Sorry, File is swresample_frame.c;


发件人: He Lei
发送时间: 2016年11月23日 11:20
收件人: ffmpeg-devel@ffmpeg.org
主题: [FFmpeg-devel] About swr_convert_frame of FFmpeg 3.2 Release

I convert audio frame from s16p to s16 used swr_convert_frame; I found the 
out_frame ->linesize[0] is more then in_frame;
After view the code,  I don’t get it ; At lines 144 of file swresample_frame.c, 
Why to add 3 when calculated nb_samples?



___
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