[FFmpeg-devel] [PATCHv3 2/3] ffplay: convert to new decode API

[Marton Balint]

Signed-off-by: Marton Balint 
---
 ffplay.c | 101 ++-
 1 file changed, 54 insertions(+), 47 deletions(-)

diff --git a/ffplay.c b/ffplay.c
index cf138dc..3d48daa 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -187,7 +187,6 @@ enum {
 
 typedef struct Decoder {
 AVPacket pkt;
-AVPacket pkt_temp;
 PacketQueue *queue;
 AVCodecContext *avctx;
 int pkt_serial;
@@ -551,40 +550,24 @@ static void decoder_init(Decoder *d, AVCodecContext 
*avctx, PacketQueue *queue,
 d->queue = queue;
 d->empty_queue_cond = empty_queue_cond;
 d->start_pts = AV_NOPTS_VALUE;
+d->pkt_serial = -1;
 }
 
 static int decoder_decode_frame(Decoder *d, AVFrame *frame, AVSubtitle *sub) {
-int got_frame = 0;
+int ret = AVERROR(EAGAIN);
 
-do {
-int ret = -1;
+for (;;) {
+AVPacket pkt;
 
+if (d->queue->serial == d->pkt_serial) {
+do {
 if (d->queue->abort_request)
 return -1;
 
-if (!d->packet_pending || d->queue->serial != d->pkt_serial) {
-AVPacket pkt;
-do {
-if (d->queue->nb_packets == 0)
-SDL_CondSignal(d->empty_queue_cond);
-if (packet_queue_get(d->queue, , 1, >pkt_serial) < 0)
-return -1;
-if (pkt.data == flush_pkt.data) {
-avcodec_flush_buffers(d->avctx);
-d->finished = 0;
-d->next_pts = d->start_pts;
-d->next_pts_tb = d->start_pts_tb;
-}
-} while (pkt.data == flush_pkt.data || d->queue->serial != 
d->pkt_serial);
-av_packet_unref(>pkt);
-d->pkt_temp = d->pkt = pkt;
-d->packet_pending = 1;
-}
-
 switch (d->avctx->codec_type) {
 case AVMEDIA_TYPE_VIDEO:
-ret = avcodec_decode_video2(d->avctx, frame, _frame, 
>pkt_temp);
-if (got_frame) {
+ret = avcodec_receive_frame(d->avctx, frame);
+if (ret >= 0) {
 if (decoder_reorder_pts == -1) {
 frame->pts = av_frame_get_best_effort_timestamp(frame);
 } else if (!decoder_reorder_pts) {
@@ -593,8 +576,8 @@ static int decoder_decode_frame(Decoder *d, AVFrame *frame, 
AVSubtitle *sub) {
 }
 break;
 case AVMEDIA_TYPE_AUDIO:
-ret = avcodec_decode_audio4(d->avctx, frame, _frame, 
>pkt_temp);
-if (got_frame) {
+ret = avcodec_receive_frame(d->avctx, frame);
+if (ret >= 0) {
 AVRational tb = (AVRational){1, frame->sample_rate};
 if (frame->pts != AV_NOPTS_VALUE)
 frame->pts = av_rescale_q(frame->pts, 
av_codec_get_pkt_timebase(d->avctx), tb);
@@ -606,33 +589,57 @@ static int decoder_decode_frame(Decoder *d, AVFrame 
*frame, AVSubtitle *sub) {
 }
 }
 break;
-case AVMEDIA_TYPE_SUBTITLE:
-ret = avcodec_decode_subtitle2(d->avctx, sub, _frame, 
>pkt_temp);
-break;
+}
+if (ret == AVERROR_EOF) {
+d->finished = d->pkt_serial;
+avcodec_flush_buffers(d->avctx);
+return 0;
+}
+if (ret >= 0)
+return 1;
+} while (ret != AVERROR(EAGAIN));
 }
 
-if (ret < 0) {
-d->packet_pending = 0;
+do {
+if (d->queue->nb_packets == 0)
+SDL_CondSignal(d->empty_queue_cond);
+if (d->packet_pending) {
+av_packet_move_ref(, >pkt);
+d->packet_pending = 0;
+} else {
+if (packet_queue_get(d->queue, , 1, >pkt_serial) < 0)
+return -1;
+}
+} while (d->queue->serial != d->pkt_serial);
+
+if (pkt.data == flush_pkt.data) {
+avcodec_flush_buffers(d->avctx);
+d->finished = 0;
+d->next_pts = d->start_pts;
+d->next_pts_tb = d->start_pts_tb;
 } else {
-d->pkt_temp.dts =
-d->pkt_temp.pts = AV_NOPTS_VALUE;
-if (d->pkt_temp.data) {
-if (d->avctx->codec_type != AVMEDIA_TYPE_AUDIO)
-ret = d->pkt_temp.size;
-d->pkt_temp.data += ret;
-d->pkt_temp.size -= ret;
-if (d->pkt_temp.size <= 0)
-d->packet_pending = 0;
+if (d->avctx->codec_type == AVMEDIA_TYPE_SUBTITLE) {
+int got_frame = 0;
+ret = avcodec_decode_subtitle2(d->avctx, sub, _frame, 
);
+if (ret < 0) {
+ret = AVERROR(EAGAIN);
+} else {
+if (got_frame && !pkt.data) {
+   

[FFmpeg-devel] [PATCHv2] avfilter/vf_framerate: always request input if no output is provided in request_frame

[Marton Balint]

Fixes ticket #6285.

Signed-off-by: Marton Balint 
---
 libavfilter/vf_framerate.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavfilter/vf_framerate.c b/libavfilter/vf_framerate.c
index b4a74f7..0093f78 100644
--- a/libavfilter/vf_framerate.c
+++ b/libavfilter/vf_framerate.c
@@ -341,7 +341,7 @@ static int blend_frames8(AVFilterContext *ctx, float 
interpolate,
 return 0;
 }
 
-static int process_work_frame(AVFilterContext *ctx, int stop)
+static int process_work_frame(AVFilterContext *ctx, int in_filter_frame)
 {
 FrameRateContext *s = ctx->priv;
 int64_t work_next_pts;
@@ -360,7 +360,7 @@ static int process_work_frame(AVFilterContext *ctx, int 
stop)
 // the filter cannot do anything
 ff_dlog(ctx, "process_work_frame() no current frame cached: move on to 
next frame, do not output a frame\n");
 next_source(ctx);
-return 0;
+return in_filter_frame ? 0 : ff_request_frame(ctx->inputs[0]);
 }
 
 work_next_pts = s->pts + s->average_dest_pts_delta;
@@ -384,7 +384,7 @@ static int process_work_frame(AVFilterContext *ctx, int 
stop)
 ff_dlog(ctx, "process_work_frame() work crnt pts >= srce next pts: 
SKIP FRAME, move on to next frame, do not output a frame\n");
 next_source(ctx);
 s->pending_srce_frames--;
-return 0;
+return in_filter_frame ? 0 : ff_request_frame(ctx->inputs[0]);
 }
 
 // calculate interpolation
@@ -436,7 +436,7 @@ copy_done:
 }
 ff_dlog(ctx, "process_work_frame() output a frame\n");
 s->dest_frame_num++;
-if (stop)
+if (in_filter_frame)
 s->pending_end_frame = 0;
 s->last_dest_frame_pts = s->work->pts;
 
-- 
2.10.2

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

Re: [FFmpeg-devel] [PATCH 2/2] Add experimental support for Opus in ISO BMFF (MP4)

[Matthew Gregan]

At 2017-04-05T20:43:21-0300, James Almer wrote:
> > +/* OpusSpecificBox size plus magic for Ogg OpusHead header. */
> > +size = atom.size + 16;
> 
> This should be 8. "OpusHead" is not 16 bytes.
> 
> > +avio_read(pb, st->codecpar->extradata + 9, size - 17);
> 
> Same, this should be size - 9.

Oops, silly mistake there.  Thanks for the feedback!  Updated patch attached.
>From 9ff591b9ae6a50ea3326e7374f800f7dd12eeef2 Mon Sep 17 00:00:00 2001
From: Matthew Gregan 
Date: Thu, 16 Mar 2017 14:17:21 +1300
Subject: Re: [PATCH 2/2] Add experimental demuxing support for Opus in ISO BMFF
 (MP4).

Based on the draft spec at http://vfrmaniac.fushizen.eu/contents/opus_in_isobmff.html

Signed-off-by: Matthew Gregan 
---
 libavformat/mov.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index f2296f8917..2995a009a8 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5264,6 +5264,54 @@ static int cenc_filter(MOVContext *c, MOVStreamContext *sc, int64_t index, uint8
 return 0;
 }
 
+static int mov_read_dops(MOVContext *c, AVIOContext *pb, MOVAtom atom)
+{
+const int OPUS_SEEK_PREROLL_MS = 80;
+AVStream *st;
+size_t size;
+int16_t pre_skip;
+
+if (c->fc->nb_streams < 1)
+return 0;
+st = c->fc->streams[c->fc->nb_streams-1];
+
+if ((uint64_t)atom.size > (1<<30) || atom.size < 11)
+return AVERROR_INVALIDDATA;
+
+/* Check OpusSpecificBox version. */
+if (avio_r8(pb) != 0) {
+av_log(c->fc, AV_LOG_ERROR, "unsupported OpusSpecificBox version\n");
+return AVERROR_INVALIDDATA;
+}
+
+/* OpusSpecificBox size plus magic for Ogg OpusHead header. */
+size = atom.size + 8;
+
+if (ff_alloc_extradata(st->codecpar, size))
+return AVERROR(ENOMEM);
+
+AV_WL32(st->codecpar->extradata, MKTAG('O','p','u','s'));
+AV_WL32(st->codecpar->extradata + 4, MKTAG('H','e','a','d'));
+AV_WB8(st->codecpar->extradata + 8, 1); /* OpusHead version */
+avio_read(pb, st->codecpar->extradata + 9, size - 9);
+
+/* OpusSpecificBox is stored in big-endian, but OpusHead is
+   little-endian; aside from the preceeding magic and version they're
+   otherwise currently identical.  Data after output gain at offset 16
+   doesn't need to be bytewapped. */
+pre_skip = AV_RB16(st->codecpar->extradata + 10);
+AV_WL16(st->codecpar->extradata + 10, pre_skip);
+AV_WL32(st->codecpar->extradata + 12, AV_RB32(st->codecpar->extradata + 12));
+AV_WL16(st->codecpar->extradata + 16, AV_RB16(st->codecpar->extradata + 16));
+
+st->codecpar->initial_padding = pre_skip;
+st->codecpar->seek_preroll = av_rescale_q(OPUS_SEEK_PREROLL_MS,
+  (AVRational){1, 1000},
+  (AVRational){1, 48000});
+
+return 0;
+}
+
 static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('A','C','L','R'), mov_read_aclr },
 { MKTAG('A','P','R','G'), mov_read_avid },
@@ -5345,6 +5393,7 @@ static const MOVParseTableEntry mov_default_parse_table[] = {
 { MKTAG('d','f','L','a'), mov_read_dfla },
 { MKTAG('s','t','3','d'), mov_read_st3d }, /* stereoscopic 3D video box */
 { MKTAG('s','v','3','d'), mov_read_sv3d }, /* spherical video box */
+{ MKTAG('d','O','p','s'), mov_read_dops },
 { 0, NULL }
 };
 
-- 
2.12.2

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

[FFmpeg-devel] [PATCH] Observe Set-Cookie headers in HLS streams

[Micah Galizia]

Signed-off-by: Micah Galizia 
---
 libavformat/hls.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index bac53a4..ab81863 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -630,8 +630,14 @@ static int open_url(AVFormatContext *s, AVIOContext **pb, 
const char *url,
 ret = s->io_open(s, pb, url, AVIO_FLAG_READ, );
 if (ret >= 0) {
 // update cookies on http response with setcookies.
-void *u = (s->flags & AVFMT_FLAG_CUSTOM_IO) ? NULL : s->pb;
-update_options(>cookies, "cookies", u);
+char *new_cookies = NULL;
+
+av_opt_get(*pb, "cookies", AV_OPT_SEARCH_CHILDREN, 
(uint8_t**)_cookies);
+if (new_cookies) {
+av_free(c->cookies);
+c->cookies = new_cookies;
+}
+
 av_dict_set(, "cookies", c->cookies, 0);
 }
 
-- 
2.9.3

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

[FFmpeg-devel] [PATCH] Observe Set-Cookie headers in HLS streams

[Micah Galizia]

Hello,

I noticed some Set-Cookie headers are being ignored by the HLS demuxer. Patch 
attached fixes it.

Previously, the cookies wouldn't get updated from the right AVIOContext, or not 
at all if AVFMT_FLAG_CUSTOM_IO is set -- I'm not really sure why that was done 
so please let me know if I overlooked something.

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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/h264: Check weight values to be within the specs limits.

[Ronald S. Bultje]

Hi,

On Thu, Apr 6, 2017 at 8:10 PM, Michael Niedermayer 
wrote:

> On Wed, Mar 22, 2017 at 07:09:23PM +0100, Michael Niedermayer wrote:
> > On Wed, Mar 22, 2017 at 09:06:09AM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Tue, Mar 21, 2017 at 9:59 PM, Michael Niedermayer
>  > > > wrote:
> > >
> > > > @@ -59,6 +59,9 @@ int ff_h264_pred_weight_table(GetBitContext *gb,
> const
> > > > SPS *sps,
> > > >  if (luma_weight_flag) {
> > > >  pwt->luma_weight[i][list][0] = get_se_golomb(gb);
> > > >  pwt->luma_weight[i][list][1] = get_se_golomb(gb);
> > > > +if (   (int8_t)pwt->luma_weight[i][list][0] !=
> > > > pwt->luma_weight[i][list][0]
> > > > +|| (int8_t)pwt->luma_weight[i][list][1] !=
> > > > pwt->luma_weight[i][list][1])
> > > > +goto error;
> > > >
> > >
> > > Can we please put || on the line before? h264* and a very limited
> subset of
> > > other files in our codebase have always been an exception to the large
> > > majority of the codebase and it's about time we fix that [1].
> >
> > i find putting the operators in the first column more readable
> > but if its preferred in the last, iam happy to change it.
> >
> >
> > >
> > > It's interesting (I mean that in a positive way) how you use casting to
> > > check for the range. It's a little obscure, but it's OK.
> >
> > yes, it caused me to pause to but it was the simplest way i saw to do
> > the check
> >
> >
> > >
> > > +error:
> > > > +avpriv_request_sample(logctx, "Out of range weight\n");
> > > > +return AVERROR_INVALIDDATA;
> > >
> > >
> > > Same comment as previously in other, but related, threads: unless
> there is
> > > real, demonstrable evidence that this happens in real-world files,
> this is
> > > fuzz/corruption only and shouldn't be accompanied by an explicit log
> > > message. Just return AVERROR_INVALIDDATA directly and remove the
> goto/error
> > > message.
> >
> > If there is "real, demonstrable evidence that this happens in real-world
> > files" then we would likely have a sample and not ask for one with
> > avpriv_request_sample()
> >
> > I think its very plausible that a encoder would use a weight that is
> > outside the range. Printing something does make sense.
>
> i will apply this with the label chaned to out_range_weight:
> unless there are objections


OK.

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

[FFmpeg-devel] [PATCH] avcodec/wavpack: Fix invalid shift and integer overflow

[Michael Niedermayer]

Fixes: 940/clusterfuzz-testcase-5200378381467648

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/wavpack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
index 2bda3599a8..bc4402f638 100644
--- a/libavcodec/wavpack.c
+++ b/libavcodec/wavpack.c
@@ -157,7 +157,7 @@ static int wv_get_value(WavpackFrameContext *ctx, 
GetBitContext *gb,
 } else {
 t = get_unary_0_33(gb);
 if (t >= 2) {
-if (get_bits_left(gb) < t - 1)
+if (t >= 32 || get_bits_left(gb) < t - 1)
 goto error;
 t = get_bits_long(gb, t - 1) | (1 << (t - 1));
 } else {
-- 
2.11.0

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

Re: [FFmpeg-devel] [PATCH] avcodec/mjpegdec: Fixes runtime error: signed integer overflow: -24543 * 2031616 cannot be represented in type 'int'

[Michael Niedermayer]

On Wed, Mar 29, 2017 at 11:47:31AM +0200, wm4 wrote:
> On Mon, 27 Mar 2017 00:41:05 +0200
> Michael Niedermayer  wrote:
> 
> > On Sun, Mar 26, 2017 at 05:30:05PM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > > 
> > > On Sun, Mar 26, 2017 at 3:31 PM, Michael Niedermayer 
> > >  > > > wrote:  
> > >   
> > > > On Sun, Mar 26, 2017 at 07:41:58PM +0200, wm4 wrote:  
> > > > > On Sun, 26 Mar 2017 19:16:26 +0200
> > > > > Michael Niedermayer  wrote:
> > > > >  
> > > > > > On Sun, Mar 26, 2017 at 06:51:11PM +0200, wm4 wrote:  
> > > > > > > On Sun, 26 Mar 2017 18:11:01 +0200
> > > > > > > Michael Niedermayer  wrote:
> > > > > > >  
> > > > > > > > Fixes: 943/clusterfuzz-testcase-5114865297391616
> > > > > > > >
> > > > > > > > Found-by: continuous fuzzing process  
> > > > https://github.com/google/oss-fuzz/tree/master/targets/ffmpeg  
> > > > > > > > Signed-off-by: Michael Niedermayer 
> > > > > > > > ---
> > > > > > > >  libavcodec/mjpegdec.c | 3 ++-
> > > > > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
> > > > > > > > index f26e8a3f9a..e08b045fe7 100644
> > > > > > > > --- a/libavcodec/mjpegdec.c
> > > > > > > > +++ b/libavcodec/mjpegdec.c
> > > > > > > > @@ -757,7 +757,8 @@ static int 
> > > > > > > > decode_block_progressive(MJpegDecodeContext  
> > > > *s, int16_t *block,  
> > > > > > > >  uint16_t *quant_matrix,
> > > > > > > >  int ss, int se, int Al, 
> > > > > > > > int  
> > > > *EOBRUN)  
> > > > > > > >  {
> > > > > > > > -int code, i, j, level, val, run;
> > > > > > > > +int code, i, j, val, run;
> > > > > > > > +SUINT level;
> > > > > > > >
> > > > > > > >  if (*EOBRUN) {
> > > > > > > >  (*EOBRUN)--;  
> > > > > > >
> > > > > > > Please make the type either signed or unsigned. Making it both
> > > > > > > (depending on the debug level) just to make the fuzzer happy (or
> > > > > > > something more complicated than that?) isn't a good idea. You  
> > > > probably  
> > > > > > > want to make it always unsigned?  
> > > > > >
> > > > > > No, i want to make it SUINT
> > > > > >
> > > > > > If it is always unsigned then its not possible to detect overflows
> > > > > > without explicitly checking for overflows.
> > > > > > If it is SUINT then ubsan can be used to detect overflows, this is
> > > > > > usefull to test files showing artifacts but no decode errors.
> > > > > >  
> > > > >
> > > > > The point of these tools (static analyzers, sanitizers, fuzzers) is to
> > > > > improve the correctness of the code.  
> > > >  
> > > > > SUINT is still defined to "int" if
> > > > > CHECKED is not defined  
> > > >
> > > > no
> > > >
> > > > internal.h:
> > > > #ifdef CHECKED
> > > > #define SUINT   int
> > > > #define SUINT32 int32_t
> > > > #else
> > > > #define SUINT   unsigned
> > > > #define SUINT32 uint32_t
> > > > #endif
> > > >
> > > > I belive the rest of your mail assumes this condition is backward to
> > > > how it is
> > > >
> > > > SUINT is not there to make any tools happy its there to allow finding
> > > > overflows in debug more while having valid c code in normal builds  
> > > 
> > >   
> > 
> > > Why don't we want to detect overflows in debug mode?  
> > 
> > like wm4 you read "#if A" as "#if not A", all your mail and questions
> > seem based on reading the condition for SUINT flipped around
> > 
> > in DEBUG mode CHECKED is enabled, SUINT is int and overflows are
> > undefined behaviour which can be detected easily with ubsan.
> > 
> > This allows us to debug samples producing artifacts but no decode
> > errors due to overflows.
> > 
> > 
> > in normal mode CHECKED is disabled, SUINT is unsigned and overflows
> > are defined behavior. There must be no undefined behavior in releases
> > 
> > maybe you belive everyone is using debug mode and the fuzzers run in
> > debug mode. Maybe this is why everyone belives the condition is
> > backward
> 
> How would we know this? Maybe I've been assuming too much in order to
> try to make sense out of it.
> 
> > I might be wrong but unless you manually pass -DDEBUG you dont use
> > debug mode, adding -DDEBUG is how our debug mode fate client tests that
> > mode
> 

> But why do you want to detect overflows in debug mode, if in release

To debug.
Like i want compiler warnings to "debug"
or coverity warnings to find and correct bugs. == "debug"

If a undamaged file triggers an overflow thats very likely a bug and
a easy to fix one if one knows about the overflow.
If one doesnt know of that overflow it can be very hard to find


> mode they're using unsigned arithmetic, which has defined overflows?
>
> Either the code is correct with unsigned - then it can use unsigned in
> both release and debug mode. Or it isn't - then there's another 

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/h264: Check weight values to be within the specs limits.

[Michael Niedermayer]

On Wed, Mar 22, 2017 at 07:09:23PM +0100, Michael Niedermayer wrote:
> On Wed, Mar 22, 2017 at 09:06:09AM -0400, Ronald S. Bultje wrote:
> > Hi,
> > 
> > On Tue, Mar 21, 2017 at 9:59 PM, Michael Niedermayer  > > wrote:
> > 
> > > @@ -59,6 +59,9 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const
> > > SPS *sps,
> > >  if (luma_weight_flag) {
> > >  pwt->luma_weight[i][list][0] = get_se_golomb(gb);
> > >  pwt->luma_weight[i][list][1] = get_se_golomb(gb);
> > > +if (   (int8_t)pwt->luma_weight[i][list][0] !=
> > > pwt->luma_weight[i][list][0]
> > > +|| (int8_t)pwt->luma_weight[i][list][1] !=
> > > pwt->luma_weight[i][list][1])
> > > +goto error;
> > >
> > 
> > Can we please put || on the line before? h264* and a very limited subset of
> > other files in our codebase have always been an exception to the large
> > majority of the codebase and it's about time we fix that [1].
> 
> i find putting the operators in the first column more readable
> but if its preferred in the last, iam happy to change it.
> 
> 
> > 
> > It's interesting (I mean that in a positive way) how you use casting to
> > check for the range. It's a little obscure, but it's OK.
> 
> yes, it caused me to pause to but it was the simplest way i saw to do
> the check
> 
> 
> > 
> > +error:
> > > +avpriv_request_sample(logctx, "Out of range weight\n");
> > > +return AVERROR_INVALIDDATA;
> > 
> > 
> > Same comment as previously in other, but related, threads: unless there is
> > real, demonstrable evidence that this happens in real-world files, this is
> > fuzz/corruption only and shouldn't be accompanied by an explicit log
> > message. Just return AVERROR_INVALIDDATA directly and remove the goto/error
> > message.
> 
> If there is "real, demonstrable evidence that this happens in real-world
> files" then we would likely have a sample and not ask for one with
> avpriv_request_sample()
> 
> I think its very plausible that a encoder would use a weight that is
> outside the range. Printing something does make sense.

i will apply this with the label chaned to out_range_weight:
unless there are objections

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates


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

[FFmpeg-devel] [PATCH] avformat/utils: fix estimate_timings_from_pts duration calculation for mpegts files which contain a PMT update

[Aman Gupta]

From: Aman Gupta 

For example: https://s3.amazonaws.com/tmm1/pmt-version-change.ts

Before:

Duration: 00:00:10.01, start: 1236.288878, bitrate: 16752 kb/s

After:

Duration: 00:00:19.59, start: 1236.288878, bitrate: 8563 kb/s
---
 libavformat/utils.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index a059046a2c..3028d110aa 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2720,7 +2720,8 @@ static void 
estimate_timings_from_bit_rate(AVFormatContext *ic)
 static void estimate_timings_from_pts(AVFormatContext *ic, int64_t old_offset)
 {
 AVPacket pkt1, *pkt = 
-AVStream *st;
+AVStream *st, *st2;
+AVProgram *prg;
 int num, den, read_size, i, ret;
 int found_duration = 0;
 int is_end;
@@ -2768,6 +2769,24 @@ static void estimate_timings_from_pts(AVFormatContext 
*ic, int64_t old_offset)
 read_size += pkt->size;
 st = ic->streams[pkt->stream_index];
 if (pkt->pts != AV_NOPTS_VALUE &&
+(st->start_time == AV_NOPTS_VALUE &&
+ st->first_dts  == AV_NOPTS_VALUE)) {
+// try copying start time from another track of the same type
+// in the same program.
+prg = av_find_program_from_stream(ic, NULL, pkt->stream_index);
+for (i = 0; prg && i < prg->nb_stream_indexes; i++) {
+st2 = ic->streams[prg->stream_index[i]];
+if (st2 != st &&
+st2->codecpar->codec_type == st->codecpar->codec_type 
&&
+(st2->start_time != AV_NOPTS_VALUE ||
+ st2->first_dts != AV_NOPTS_VALUE)) {
+st->start_time = st2->start_time;
+st->first_dts = st2->first_dts;
+break;
+}
+}
+}
+if (pkt->pts != AV_NOPTS_VALUE &&
 (st->start_time != AV_NOPTS_VALUE ||
  st->first_dts  != AV_NOPTS_VALUE)) {
 if (pkt->duration == 0) {
-- 
2.11.0 (Apple Git-81)

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

Re: [FFmpeg-devel] Fix leaked dictionary in mp3dec

[Thomas Guilbert]

The patch didn't show up as properly formatted on
https://patchwork.ffmpeg.org/patch/3228/.

Re-submitting using no line wrap in the base64 attachment, and copying the
contents of the patch for ease of review:

>From fced5ab0e09f529397adddcb560d1a08f2df4840 Mon Sep 17 00:00:00 2001
From: Thomas Guilbert 
Date: Thu, 30 Mar 2017 18:23:29 -0700
Subject: [PATCH] Fix dictionnary leak in mp3dec

---
 libavformat/mp3dec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 0924a57843..fd8184cc0b 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -349,6 +349,7 @@ static int mp3_read_header(AVFormatContext *s)
 int ret;
 int i;

+av_dict_free(>metadata);
 s->metadata = s->internal->id3v2_meta;
 s->internal->id3v2_meta = NULL;

-- 
2.12.2.564.g063fe858b8-goog


On Fri, Mar 31, 2017 at 12:39 PM, Thomas Guilbert 
wrote:

> Commit '65862f57ad2f7f49d715f334a9d892e0b20d42f1' overwrites s->metada
> with s->internal->id3v2_meta, which leaks an AVDictionary* if
> s->metada was not null.
>
> Please excuse any formatting problems in this email, this is my first
> time uploading a patch :)
>
> Thank you,
> Thomas
>
RnJvbSBmY2VkNWFiMGUwOWY1MjkzOTdhZGRkY2I1NjBkMWEwOGYyZGY0ODQwIE1vbiBTZXAgMTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBUaG9tYXMgR3VpbGJlcnQgPHRndWlsYmVydEBjaHJvbWl1bS5vcmc+CkRhdGU6IFRodSwgMzAgTWFyIDIwMTcgMTg6MjM6MjkgLTA3MDAKU3ViamVjdDogW1BBVENIXSBGaXggZGljdGlvbm5hcnkgbGVhayBpbiBtcDNkZWMKCi0tLQogbGliYXZmb3JtYXQvbXAzZGVjLmMgfCAxICsKIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKQoKZGlmZiAtLWdpdCBhL2xpYmF2Zm9ybWF0L21wM2RlYy5jIGIvbGliYXZmb3JtYXQvbXAzZGVjLmMKaW5kZXggMDkyNGE1Nzg0My4uZmQ4MTg0Y2MwYiAxMDA2NDQKLS0tIGEvbGliYXZmb3JtYXQvbXAzZGVjLmMKKysrIGIvbGliYXZmb3JtYXQvbXAzZGVjLmMKQEAgLTM0OSw2ICszNDksNyBAQCBzdGF0aWMgaW50IG1wM19yZWFkX2hlYWRlcihBVkZvcm1hdENvbnRleHQgKnMpCiAgICAgaW50IHJldDsKICAgICBpbnQgaTsKIAorICAgIGF2X2RpY3RfZnJlZSgmcy0+bWV0YWRhdGEpOwogICAgIHMtPm1ldGFkYXRhID0gcy0+aW50ZXJuYWwtPmlkM3YyX21ldGE7CiAgICAgcy0+aW50ZXJuYWwtPmlkM3YyX21ldGEgPSBOVUxMOwogCi0tIAoyLjEyLjIuNTY0LmcwNjNmZTg1OGI4LWdvb2cKCg==
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 08:56:19PM +0200, Nicolas George wrote:
> Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> > If i fix it i would break your patchset if i apply it before and ill
> > forget to do it later ...
> 
> Do not worry about that, I will rebase and fix the conflicts if
> necessary. On the other hand, I would very much like to avoid wasting
> more time bikeshedding a cosmetic patch.

ok, ill fix the type then

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope


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

Re: [FFmpeg-devel] [PATCH] pthread_frame: don't return stale error codes after flush

[Uoti Urpala]

On Thu, 2017-04-06 at 18:18 +0200, wm4 wrote:
> > >  p->got_frame = 0;
> > >  av_frame_unref(p->frame);
> > > +p->result = 0;

Shouldn't p->result be similarly reset together with p->got_frame also
in ff_thread_decode_frame()? A similar problem seems possible:
- a normal decode call returns an error due to p->result being negative
- drain packet is sent before cycling through all threads
- the loop in ff_thread_decode_frame hits "if (p->result < 0)"
Thus incorrectly returning the same error again from the drain packet.

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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Nicolas George]

Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> If i fix it i would break your patchset if i apply it before and ill
> forget to do it later ...

Do not worry about that, I will rebase and fix the conflicts if
necessary. On the other hand, I would very much like to avoid wasting
more time bikeshedding a cosmetic patch.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 07:35:04PM +0200, Nicolas George wrote:
> Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> > A comment explaining this (if you agree that the issue is real and not
> > some mistake of understanding something on my side)
> > would be a good idea.
> > could be confusing otherwise and lead to misunderstandings
> 
> I am sorry, I have no idea what kind of comment you would like.
> 
> Once again, this patch is just renaming a variable to have a name
> similar to how it is used. It does not change the logic of the current
> code, and if anything it makes it clearer. I do not think adding a
> comment belongs here.

Currently the packet duration is assigned to a variable called
duration
After your patch it is assigned to a variable called duration_dts

Someone reading
duration_dts = pkt->duration;

will have an idea what pkt->duration is, our stuff is often documented
through code.

If a line of code gives a incorrect impression, it should be documented
to avoid it leading to misunderstandings.
I assume here that i didnt miss something all along and this is actually
buggy

Also duration_dts should be documented, what does it exactly mean
the difference between teh current and next dts ?
if so it should probably also be changed to int64_t, thats not a issue
in this patch, but trivial to fix and you already work on this.
If i fix it i would break your patchset if i apply it before and ill
forget to do it later ...

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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

Re: [FFmpeg-devel] [PATCH] pthread_frame: make accesses to debug field be protected by owner lock.

[Kieran Kunhya]

On Thu, 6 Apr 2017 at 18:48 Ronald S. Bultje  wrote:

> The av_log() is done outside the lock, but this way the accesses to the
> field (reads and writes) are always protected by a mutex. The av_log()
> is not run inside the lock context because it may involve user callbacks
> and doing that in performance-sensitive code is probably not a good idea.
>
> This should fix occasional tsan warnings when running fate-h264, like:
>

Looks fine.

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

[FFmpeg-devel] [PATCH] pthread_frame: make accesses to debug field be protected by owner lock.

[Ronald S. Bultje]

The av_log() is done outside the lock, but this way the accesses to the
field (reads and writes) are always protected by a mutex. The av_log()
is not run inside the lock context because it may involve user callbacks
and doing that in performance-sensitive code is probably not a good idea.

This should fix occasional tsan warnings when running fate-h264, like:

WARNING: ThreadSanitizer: data race (pid=10916)
  Write of size 4 at 0x7d64000174fc by main thread (mutexes: write M2313):
#0 update_context_from_user src/libavcodec/pthread_frame.c:335 
(ffmpeg+0x00df7b06)
[..]
  Previous read of size 4 at 0x7d64000174fc by thread T1 (mutexes: write M2311):
#0 ff_thread_await_progress src/libavcodec/pthread_frame.c:592 
(ffmpeg+0x00df8b3e)
---
 libavcodec/pthread_frame.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index c246c2f..8857bfc 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -559,6 +559,7 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int 
field)
 {
 PerThreadContext *p;
 atomic_int *progress = f->progress ? (atomic_int*)f->progress->data : NULL;
+int debug_mv;
 
 if (!progress ||
 atomic_load_explicit([field], memory_order_relaxed) >= n)
@@ -566,22 +567,24 @@ void ff_thread_report_progress(ThreadFrame *f, int n, int 
field)
 
 p = f->owner[field]->internal->thread_ctx;
 
-if (f->owner[field]->debug_DEBUG_THREADS)
-av_log(f->owner[field], AV_LOG_DEBUG,
-   "%p finished %d field %d\n", progress, n, field);
-
 pthread_mutex_lock(>progress_mutex);
+debug_mv = f->owner[field]->debug_DEBUG_THREADS;
 
 atomic_store_explicit([field], n, memory_order_release);
 
 pthread_cond_broadcast(>progress_cond);
 pthread_mutex_unlock(>progress_mutex);
+
+if (debug_mv)
+av_log(f->owner[field], AV_LOG_DEBUG,
+   "%p finished %d field %d\n", progress, n, field);
 }
 
 void ff_thread_await_progress(ThreadFrame *f, int n, int field)
 {
 PerThreadContext *p;
 atomic_int *progress = f->progress ? (atomic_int*)f->progress->data : NULL;
+int debug_mv;
 
 if (!progress ||
 atomic_load_explicit([field], memory_order_acquire) >= n)
@@ -589,14 +592,15 @@ void ff_thread_await_progress(ThreadFrame *f, int n, int 
field)
 
 p = f->owner[field]->internal->thread_ctx;
 
-if (f->owner[field]->debug_DEBUG_THREADS)
-av_log(f->owner[field], AV_LOG_DEBUG,
-   "thread awaiting %d field %d from %p\n", n, field, progress);
-
 pthread_mutex_lock(>progress_mutex);
+debug_mv = f->owner[field]->debug_DEBUG_THREADS;
 while (atomic_load_explicit([field], memory_order_relaxed) < n)
 pthread_cond_wait(>progress_cond, >progress_mutex);
 pthread_mutex_unlock(>progress_mutex);
+
+if (debug_mv)
+av_log(f->owner[field], AV_LOG_DEBUG,
+   "thread awaited %d field %d from %p\n", n, field, progress);
 }
 
 void ff_thread_finish_setup(AVCodecContext *avctx) {
-- 
2.8.1

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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Nicolas George]

Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> A comment explaining this (if you agree that the issue is real and not
> some mistake of understanding something on my side)
> would be a good idea.
> could be confusing otherwise and lead to misunderstandings

I am sorry, I have no idea what kind of comment you would like.

Once again, this patch is just renaming a variable to have a name
similar to how it is used. It does not change the logic of the current
code, and if anything it makes it clearer. I do not think adding a
comment belongs here.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 07:03:25PM +0200, Nicolas George wrote:
> Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> > Its a while ago i worked with this and the code changed since then
> > so maybe iam wrong but
> > 
> > pkt->duration is the duration of the frame encoded in that packet
> > that is not neccesarily the time between its dts and the next dts
> > (well it is for a lot of cases of course where its guranteed but ...)
> > 
> > example (mpeg1/mpeg2/mpeg4)
> > I   P   B   P   B
> > PTS 1  10   2   20  19
> > DTS 0   1   2   10  19
> > DUR 1   9   8   x   1
> > 
> > Its quite possible i made a mistake of course ...
> 
> Well, I do not know about that, I always thought DTS was stupid anyway.
> 
> My patch is not changing the DTS logic, only the name of a variable to
> reflect its usage in the current code and avoid conflict with the next
> patch. The next patch is the important one.
> 
> If the DTS logic needs fixing and you (or anybody else) end up doing it,
> then the name of the variable can be fixed at the same time. And having
> "dts" in it will help finding it.

A comment explaining this (if you agree that the issue is real and not
some mistake of understanding something on my side)
would be a good idea.
could be confusing otherwise and lead to misunderstandings

[...]

-- 
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 4/4] ffmpeg: send EOF pts to filters.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 10:44:51AM +0200, Nicolas George wrote:
> Signed-off-by: Nicolas George 
> ---
>  ffmpeg.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> 
> With this change, filters have a timestamp when EOF is reached, available in
> link->current_pts. For example, vf_fps could make use of it to fix the
> duration of the last frame; the logic in vf_fps seems unnecessarily
> complicated, I have not yet implemented this, but a quick fix could be
> possible.

should be ok

thx

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

The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."


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: option for dropping negative CTS frames Initial frames with negative pts can produce video/audio desynchronization when a decoder is not prepared to handle negative

[wm4]

On Thu, 6 Apr 2017 14:18:20 -0300
Felipe Astroza  wrote:

> 2017-04-06 2:00 GMT-03:00 wm4 :
> 
> > On Wed, 5 Apr 2017 17:15:26 -0300
> > Felipe Astroza  wrote:
> >  
> > > 2017-04-05 15:35 GMT-03:00 wm4 :
> > >  
> > > > On Wed,  5 Apr 2017 14:29:30 -0300
> > > > fel...@astroza.cl wrote:
> > > >  
> > > > > From: Felipe Astroza 
> > > > >
> > > > > Signed-off-by: Felipe Astroza 
> > > > > ---
> > > > >  libavformat/flvdec.c | 14 +++---
> > > > >  1 file changed, 11 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > > > > index 3959a36..1556fe0 100644
> > > > > --- a/libavformat/flvdec.c
> > > > > +++ b/libavformat/flvdec.c
> > > > > @@ -44,6 +44,7 @@
> > > > >  typedef struct FLVContext {
> > > > >  const AVClass *class; ///< Class for private options.
> > > > >  int trust_metadata;   ///< configure streams according  
> > onMetaData  
> > > > > +int drop_negative_cts;///< drop frames if cts is negative
> > > > >  int wrong_dts;///< wrong dts due to negative cts
> > > > >  uint8_t *new_extradata[FLV_STREAM_TYPE_NB];
> > > > >  int new_extradata_size[FLV_STREAM_TYPE_NB];
> > > > > @@ -1139,10 +1140,16 @@ retry_duration:
> > > > >  int32_t cts = (avio_rb24(s->pb) + 0xff80) ^  
> > 0xff80;  
> > > > >  pts = dts + cts;
> > > > >  if (cts < 0) { // dts might be wrong
> > > > > -if (!flv->wrong_dts)
> > > > > +if (flv->drop_negative_cts) {
> > > > >  av_log(s, AV_LOG_WARNING,
> > > > > -"Negative cts, previous timestamps might be  
> > > > wrong.\n");  
> > > > > -flv->wrong_dts = 1;
> > > > > +"Negative cts, frames will be  
> > dropped.\n");  
> > > > > +dts = pts = AV_NOPTS_VALUE;
> > > > > +} else {
> > > > > +if (!flv->wrong_dts)
> > > > > +av_log(s, AV_LOG_WARNING,
> > > > > +"Negative cts, previous timestamps  
> > might be  
> > > > wrong.\n");  
> > > > > +flv->wrong_dts = 1;
> > > > > +}
> > > > >  } else if (FFABS(dts - pts) > 1000*60*15) {
> > > > >  av_log(s, AV_LOG_WARNING,
> > > > > "invalid timestamps %"PRId64" %"PRId64"\n",  
> > dts,  
> > > > pts);  
> > > > > @@ -1253,6 +1260,7 @@ static int flv_read_seek(AVFormatContext *s,  
> > int  
> > > > stream_index,  
> > > > >  #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> > > > >  static const AVOption options[] = {
> > > > >  { "flv_metadata", "Allocate streams according to the onMetaData  
> > > > array", OFFSET(trust_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,  
> > VD },  
> > > > > +{ "flv_drop_negative_cts", "Drop frames with negative  
> > composition  
> > > > timestamp", OFFSET(drop_negative_cts), AV_OPT_TYPE_BOOL, { .i64 = 0 },  
> > 0,  
> > > > 1, VD },  
> > > > >  { "missing_streams", "", OFFSET(missing_streams),  
> > AV_OPT_TYPE_INT,  
> > > > { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY  
> > },  
> > > > >  { NULL }
> > > > >  };  
> > > >
> > > > This seems all kind of wrong. You don't add a hack to a single demuxer
> > > > just because a single decoder can't handle unusual things in "some"
> > > > files. You don't add it as option either. (If this is a "fix my problem
> > > > the easiest way" hack, you should probably keep it in your own ffmpeg
> > > > branch.)
> > > >
> > > > It was the way I found to avoid the initial frames without a preceding  
> > > keyframe (marked with pts < 0) that RTMP wowza server sends in live
> > > streams, just cover flv format case :/. And yes yes, you're right, this  
> > is  
> > > a hack because of I was not able to patch QSV decoder.
> > >
> > > h264_qsv decoder -> h264_qsv encoder produces a video delayed output
> > > h264_qsv decoder -> libx264 encoder produces a video delayed output
> > > libx264 decoder -> libx264 encoder produces a right output  
> >
> > There's no libx264 decoder - I assume you mean ffmpeg's native decoder.
> >  
> > > h264_qsv is the source of my issues. I was passing -itsoffset  
> > CONSTANT(0.5  
> > > in my case) as workaround but it works 90% of the time and I just want a
> > > definitive solution.  
> >
> > Did you check whether there's some obvious cause, like due to how qsv
> > represents timestamps? Also there is no reason to use the qsv
> > _decoder_. The native ffmpeg decoder + hwaccel will do getter. Anyway,
> > still legitimate to want to fix qsv, of course.
> >  
> 
> I'm not sure of that. Reading input at native frame rate:
> 
> * h264 native decoder -> h264_qsv encoder (needs hwcontext)
> command: ffmpeg -re -loglevel 

Re: [FFmpeg-devel] [PATCH] flvdec: option for dropping negative CTS frames Initial frames with negative pts can produce video/audio desynchronization when a decoder is not prepared to handle negative

[Felipe Astroza]

2017-04-06 2:00 GMT-03:00 wm4 :

> On Wed, 5 Apr 2017 17:15:26 -0300
> Felipe Astroza  wrote:
>
> > 2017-04-05 15:35 GMT-03:00 wm4 :
> >
> > > On Wed,  5 Apr 2017 14:29:30 -0300
> > > fel...@astroza.cl wrote:
> > >
> > > > From: Felipe Astroza 
> > > >
> > > > Signed-off-by: Felipe Astroza 
> > > > ---
> > > >  libavformat/flvdec.c | 14 +++---
> > > >  1 file changed, 11 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/libavformat/flvdec.c b/libavformat/flvdec.c
> > > > index 3959a36..1556fe0 100644
> > > > --- a/libavformat/flvdec.c
> > > > +++ b/libavformat/flvdec.c
> > > > @@ -44,6 +44,7 @@
> > > >  typedef struct FLVContext {
> > > >  const AVClass *class; ///< Class for private options.
> > > >  int trust_metadata;   ///< configure streams according
> onMetaData
> > > > +int drop_negative_cts;///< drop frames if cts is negative
> > > >  int wrong_dts;///< wrong dts due to negative cts
> > > >  uint8_t *new_extradata[FLV_STREAM_TYPE_NB];
> > > >  int new_extradata_size[FLV_STREAM_TYPE_NB];
> > > > @@ -1139,10 +1140,16 @@ retry_duration:
> > > >  int32_t cts = (avio_rb24(s->pb) + 0xff80) ^
> 0xff80;
> > > >  pts = dts + cts;
> > > >  if (cts < 0) { // dts might be wrong
> > > > -if (!flv->wrong_dts)
> > > > +if (flv->drop_negative_cts) {
> > > >  av_log(s, AV_LOG_WARNING,
> > > > -"Negative cts, previous timestamps might be
> > > wrong.\n");
> > > > -flv->wrong_dts = 1;
> > > > +"Negative cts, frames will be
> dropped.\n");
> > > > +dts = pts = AV_NOPTS_VALUE;
> > > > +} else {
> > > > +if (!flv->wrong_dts)
> > > > +av_log(s, AV_LOG_WARNING,
> > > > +"Negative cts, previous timestamps
> might be
> > > wrong.\n");
> > > > +flv->wrong_dts = 1;
> > > > +}
> > > >  } else if (FFABS(dts - pts) > 1000*60*15) {
> > > >  av_log(s, AV_LOG_WARNING,
> > > > "invalid timestamps %"PRId64" %"PRId64"\n",
> dts,
> > > pts);
> > > > @@ -1253,6 +1260,7 @@ static int flv_read_seek(AVFormatContext *s,
> int
> > > stream_index,
> > > >  #define VD AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_DECODING_PARAM
> > > >  static const AVOption options[] = {
> > > >  { "flv_metadata", "Allocate streams according to the onMetaData
> > > array", OFFSET(trust_metadata), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1,
> VD },
> > > > +{ "flv_drop_negative_cts", "Drop frames with negative
> composition
> > > timestamp", OFFSET(drop_negative_cts), AV_OPT_TYPE_BOOL, { .i64 = 0 },
> 0,
> > > 1, VD },
> > > >  { "missing_streams", "", OFFSET(missing_streams),
> AV_OPT_TYPE_INT,
> > > { .i64 = 0 }, 0, 0xFF, VD | AV_OPT_FLAG_EXPORT | AV_OPT_FLAG_READONLY
> },
> > > >  { NULL }
> > > >  };
> > >
> > > This seems all kind of wrong. You don't add a hack to a single demuxer
> > > just because a single decoder can't handle unusual things in "some"
> > > files. You don't add it as option either. (If this is a "fix my problem
> > > the easiest way" hack, you should probably keep it in your own ffmpeg
> > > branch.)
> > >
> > > It was the way I found to avoid the initial frames without a preceding
> > keyframe (marked with pts < 0) that RTMP wowza server sends in live
> > streams, just cover flv format case :/. And yes yes, you're right, this
> is
> > a hack because of I was not able to patch QSV decoder.
> >
> > h264_qsv decoder -> h264_qsv encoder produces a video delayed output
> > h264_qsv decoder -> libx264 encoder produces a video delayed output
> > libx264 decoder -> libx264 encoder produces a right output
>
> There's no libx264 decoder - I assume you mean ffmpeg's native decoder.
>
> > h264_qsv is the source of my issues. I was passing -itsoffset
> CONSTANT(0.5
> > in my case) as workaround but it works 90% of the time and I just want a
> > definitive solution.
>
> Did you check whether there's some obvious cause, like due to how qsv
> represents timestamps? Also there is no reason to use the qsv
> _decoder_. The native ffmpeg decoder + hwaccel will do getter. Anyway,
> still legitimate to want to fix qsv, of course.
>

I'm not sure of that. Reading input at native frame rate:

* h264 native decoder -> h264_qsv encoder (needs hwcontext)
command: ffmpeg -re -loglevel verbose -hwaccel qsv -qsv_device
/dev/dri/renderD129 -i INPUT -c:v h264_qsv -look_ahead 0 -profile:v high
-preset:v veryfast -bufsize 1000k -r 30 -b:v 3440800 -maxrate 3440800 -c:a
aac test.mp4

Stream mapping:
  Stream #0:1 -> #0:0 (h264 (native) -> h264 (h264_qsv))
  Stream #0:2 -> #0:1 (aac (native) -> aac (native))
[h264_qsv @ 0x25052a0] Warning in encoder initialization: 

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Nicolas George]

Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> Its a while ago i worked with this and the code changed since then
> so maybe iam wrong but
> 
> pkt->duration is the duration of the frame encoded in that packet
> that is not neccesarily the time between its dts and the next dts
> (well it is for a lot of cases of course where its guranteed but ...)
> 
> example (mpeg1/mpeg2/mpeg4)
> I   P   B   P   B
> PTS 1  10   2   20  19
> DTS 0   1   2   10  19
> DUR 1   9   8   x   1
> 
> Its quite possible i made a mistake of course ...

Well, I do not know about that, I always thought DTS was stupid anyway.

My patch is not changing the DTS logic, only the name of a variable to
reflect its usage in the current code and avoid conflict with the next
patch. The next patch is the important one.

If the DTS logic needs fixing and you (or anybody else) end up doing it,
then the name of the variable can be fixed at the same time. And having
"dts" in it will help finding it.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 06:15:56PM +0200, Nicolas George wrote:
> Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> > what is duration_dts ?
> 
> It is the duration that is needed to update the DTS variables. The next
> patch, as you can see, introduces duration_pts for the same task for the
> PTS variables.
> 
> The current code incorrectly used the same duration for both, which is
> incorrect when a packet is not decoded into a frame immediately,
> possibly due to B-frames and reordering or to threading.

Its a while ago i worked with this and the code changed since then
so maybe iam wrong but

pkt->duration is the duration of the frame encoded in that packet
that is not neccesarily the time between its dts and the next dts
(well it is for a lot of cases of course where its guranteed but ...)

example (mpeg1/mpeg2/mpeg4)
I   P   B   P   B
PTS 1  10   2   20  19
DTS 0   1   2   10  19
DUR 1   9   8   x   1

Its quite possible i made a mistake of course ...


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides


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

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/h264: add avx 8-bit h264_idct_dc_add

[James Almer]

On 4/6/2017 1:01 PM, James Darnley wrote:
> On 2017-04-05 06:05, James Almer wrote:
>> On 4/4/2017 10:53 PM, James Darnley wrote:
>>> Haswell:
>>>  - 1.02x faster (405±0.7 vs. 397±0.8 decicycles) compared with mmxext
>>>
>>> Skylake-U:
>>>  - 1.06x faster (498±1.8 vs. 470±1.3 decicycles) compared with mmxext
>>> ---
>>>  libavcodec/x86/h264_idct.asm  | 20 
>>>  libavcodec/x86/h264dsp_init.c |  2 ++
>>>  2 files changed, 22 insertions(+)
>>>
>>> diff --git a/libavcodec/x86/h264_idct.asm b/libavcodec/x86/h264_idct.asm
>>> index 24fb4d2..7fd57d3 100644
>>> --- a/libavcodec/x86/h264_idct.asm
>>> +++ b/libavcodec/x86/h264_idct.asm
>>> @@ -1158,7 +1158,27 @@ INIT_XMM avx
>>>  movd  [%7+%8], %4
>>>  %endmacro
>>>  
>>> +%macro DC_ADD_INIT 1
>>> +add  %1d, 32
>>> +sar  %1d, 6
>>> +movd m0, %1d
>>> +SPLATW   m0, m0, 0
>>
>> Considering DC_ADD_MMXEXT_OP works with dwords, a single pshuflw should be
>> enough. This macro calls two instructions to fill the entire XMM register,
>> and there's no need for that.
> 
> Noted, I made that change butit doesn't seemto change much in terms of
> performance.
> 
>> You could for that matter try to optimize DC_ADD_MMXEXT_OP a bit, combining
>> said dwords with punpk* into fewer registers to reduce the amount of padd*
>> and psub* needed afterwards. See ADD_RES_MMX_4_8 in hevc_add_res.asm
> 
> Noted.  Maybe in the future.
> 
>> And again, SSE2 first, AVX only if measurably faster. But since you're not
>> making use of the wider XMM regs here at all, the only chips that will see
>> any real speed up are those slow in mmx (like Skylake seems to be).
> 
> Yorkfield gets no benefit from sse2 (575±0.4 vs. 574±0.3 decicycles).
> Haswell gets most of its benefit from sse2 (404±0.6 vs. 390±0.3 vs.
> 388±0.3).
> Skylake-U gets all of its speedup from sse2 (533±3.0 vs 488±2.0 vs 497±1.4).
> 
> Nehalem and 64-bit also gets no benefit from sse2.
> 
> Again: SSE2 yay or nay?  Maybe I should just drop this; I'm not sure 5
> cycles is worth it.

Yes, add SSE2 as it's the source of pretty much any speedup.
AVX seems to not make a difference on top of SSE2 if i'm reading
your numbers right (as i asked in the previous reply, please post
actual numbers for each version), so you can just skip it.

> 
> (I will now go and modify my script to divide the recorded decicycle
> count by 10.)
> 
>>> +cglobal h264_idct_dc_add_8, 3, 4, 0, dst_, block_, stride_
>   ^
> Fixed this bug.
> 
> ___
> 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] pthread_frame: don't return stale error codes after flush

[wm4]

On Thu, 6 Apr 2017 11:38:55 -0400
"Ronald S. Bultje"  wrote:

> Hi,
> 
> On Thu, Apr 6, 2017 at 11:36 AM, wm4  wrote:
> 
> > Consider the following sequence of events:
> >
> > - open a codec without AV_CODEC_CAP_DELAY
> > - decode call fails with an error
> > - ff_thread_flush() is called
> > - drain packet is sent
> >
> > Then the last step would make ff_thread_decode_frame() return an error,
> > because p->result can still be set to an error value. This is because
> > submit_packet returns immediately if AV_CODEC_CAP_DELAY is not set, and
> > no worker thread gets the chance to reset p->result, yet its value is
> > trusted by ff_thread_decode_frame().
> >
> > Fix this by clearing the error fields on flush.
> > ---
> >  libavcodec/pthread_frame.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> > index 9a6b83ac45..7586f00bec 100644
> > --- a/libavcodec/pthread_frame.c
> > +++ b/libavcodec/pthread_frame.c
> > @@ -859,6 +859,7 @@ void ff_thread_flush(AVCodecContext *avctx)
> >  // Make sure decode flush calls with size=0 won't return old
> > frames
> >  p->got_frame = 0;
> >  av_frame_unref(p->frame);
> > +p->result = 0;
> >
> >  release_delayed_buffers(p);
> >
> > --
> > 2.11.0  
> 
> 
> Nice catch - I think that looks good.
> 
> Ronald

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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Nicolas George]

Le septidi 17 germinal, an CCXXV, Michael Niedermayer a écrit :
> what is duration_dts ?

It is the duration that is needed to update the DTS variables. The next
patch, as you can see, introduces duration_pts for the same task for the
PTS variables.

The current code incorrectly used the same duration for both, which is
incorrect when a packet is not decoded into a frame immediately,
possibly due to B-frames and reordering or to threading.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 10:44:48AM +0200, Nicolas George wrote:
> Makes the reason of the "FIXME" comment more obvious.
> Avoid name conflicts for the next commit.
> 
> Signed-off-by: Nicolas George 
> ---
>  ffmpeg.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> 
> Unchanged.
> 
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index ea03179c21..4db8ea82ac 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2617,7 +2617,7 @@ static int process_input_packet(InputStream *ist, const 
> AVPacket *pkt, int no_eo
>  
>  // while we have more to decode or while the decoder did output 
> something on EOF
>  while (ist->decoding_needed) {
> -int duration = 0;
> +int duration_dts = 0;

what is duration_dts ?


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

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


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

Re: [FFmpeg-devel] [PATCH 4/5] avcodec/h264: add avx 8-bit h264_idct_add

[James Almer]

On 4/6/2017 12:34 PM, James Darnley wrote:
> On 2017-04-05 05:44, James Almer wrote:
>> On 4/4/2017 10:53 PM, James Darnley wrote:
>>> Haswell:
>>>  - 1.11x faster (522±0.4 vs. 469±1.8 decicycles) compared with mmxext
>>>
>>> Skylake-U:
>>>  - 1.21x faster (671±5.5 vs. 555±1.4 decicycles) compared with mmxext
>>
>> Again, you should add an SSE2 version first, then an AVX one if it's
>> measurably faster than the SSE2 one.
> 
> On a Yorkfield sse2 is barely faster: 1.02x faster (728±2.1 vs. 710±3.9
> decicycles).  So 1 or 2 cycles
> 
> On a Skylake-U sse2 is most of the speedup: 1.15x faster (661±2.2 vs
> 573±1.9).  Then avx gains a mere 3 cycles: 547±0.5
> 
> On a Haswell sse2 provides only half the speedup:
>  - sse2: 1.06x faster (525±2.5 vs 497±1.0 decicycles)
>  - avx:  1.06x faster (497±1.0 vs 468±1.2 decicycles)
> 
> (All on 64-bit Linux)
> 
> On Nehalem and 64-bit Windows sse2 is slower:  0.92x faster (597±3.0 vs.
> 650±9.3 decicycles)

Slower than what? MMX or AVX?

Your numbers are really confusing. Could you post the actual numbers for
each function instead of doing comparisons?

ff_h264_idct_add_8_mmx  = ??? cycles
ff_h264_idct_add_8_sse2 = ??? cycles
ff_h264_idct_add_8_avx  = ??? cycles

Does checkasm support these functions? Maybe you could just run that and
paste the results you get, which would be easier and faster.

> 
> And on that note I should probably recheck the deblock patches I pushed
> a little while ago.
> 
> So...  SSE2 for this function, yay or nay?

By default, always add SSE2 if it's measurably faster than MMX, especially
when you take advantage of the wider XMM regs, like you're doing here.
What you need to check is if adding AVX is worth it on top of SSE2 when
you're not taking advantage of the wider YMM regs, like it happens here.

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

Re: [FFmpeg-devel] [PATCH 5/5] avcodec/h264: add avx 8-bit h264_idct_dc_add

[James Darnley]

On 2017-04-05 06:05, James Almer wrote:
> On 4/4/2017 10:53 PM, James Darnley wrote:
>> Haswell:
>>  - 1.02x faster (405±0.7 vs. 397±0.8 decicycles) compared with mmxext
>>
>> Skylake-U:
>>  - 1.06x faster (498±1.8 vs. 470±1.3 decicycles) compared with mmxext
>> ---
>>  libavcodec/x86/h264_idct.asm  | 20 
>>  libavcodec/x86/h264dsp_init.c |  2 ++
>>  2 files changed, 22 insertions(+)
>>
>> diff --git a/libavcodec/x86/h264_idct.asm b/libavcodec/x86/h264_idct.asm
>> index 24fb4d2..7fd57d3 100644
>> --- a/libavcodec/x86/h264_idct.asm
>> +++ b/libavcodec/x86/h264_idct.asm
>> @@ -1158,7 +1158,27 @@ INIT_XMM avx
>>  movd  [%7+%8], %4
>>  %endmacro
>>  
>> +%macro DC_ADD_INIT 1
>> +add  %1d, 32
>> +sar  %1d, 6
>> +movd m0, %1d
>> +SPLATW   m0, m0, 0
> 
> Considering DC_ADD_MMXEXT_OP works with dwords, a single pshuflw should be
> enough. This macro calls two instructions to fill the entire XMM register,
> and there's no need for that.

Noted, I made that change butit doesn't seemto change much in terms of
performance.

> You could for that matter try to optimize DC_ADD_MMXEXT_OP a bit, combining
> said dwords with punpk* into fewer registers to reduce the amount of padd*
> and psub* needed afterwards. See ADD_RES_MMX_4_8 in hevc_add_res.asm

Noted.  Maybe in the future.

> And again, SSE2 first, AVX only if measurably faster. But since you're not
> making use of the wider XMM regs here at all, the only chips that will see
> any real speed up are those slow in mmx (like Skylake seems to be).

Yorkfield gets no benefit from sse2 (575±0.4 vs. 574±0.3 decicycles).
Haswell gets most of its benefit from sse2 (404±0.6 vs. 390±0.3 vs.
388±0.3).
Skylake-U gets all of its speedup from sse2 (533±3.0 vs 488±2.0 vs 497±1.4).

Nehalem and 64-bit also gets no benefit from sse2.

Again: SSE2 yay or nay?  Maybe I should just drop this; I'm not sure 5
cycles is worth it.

(I will now go and modify my script to divide the recorded decicycle
count by 10.)

>> +cglobal h264_idct_dc_add_8, 3, 4, 0, dst_, block_, stride_
  ^
Fixed this bug.

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

Re: [FFmpeg-devel] [PATCH] vf_framestep: add blend parameter for motion blur effect

[Michael Niedermayer]

On Thu, Apr 06, 2017 at 02:09:46PM +0100, Matthias Troffaes wrote:
> Many thanks for your feedback, Michael and Nicolas.
> 
> On Wed, Apr 5, 2017 at 7:02 PM, Michael Niedermayer
>  wrote:
> > fails on qemu mips
> 
> Thank you for flagging this failure. I'm currently trying to reproduce
> the problem. May I ask what toolchain you typically use to compile to
> mips? (I'm trying debian malta on qemu-system-mips, but it seems that
> the configure script can't correctly set up config.h in that
> environment.)

its a really old cross compiler from the no longer existing emdebian

i also have a native loongson system but that is less convenient to
test with

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

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.


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

[FFmpeg-devel] [PATCH] h264: don't re-call ff_h264_direct_ref_list_init() w/ frame-mt.

[Ronald S. Bultje]

I'm hoping that this will address the remaining tsan fate-h264 issues:

WARNING: ThreadSanitizer: data race (pid=24478)
  Read of size 8 at 0x7dbc0001c828 by main thread (mutexes: write M3243):
#0 ff_h264_ref_picture src/libavcodec/h264_picture.c:107 
(ffmpeg+0x013b78d8)
[..]
  Previous write of size 1 at 0x7dbc0001c82e by thread T2 (mutexes: write 
M3245):
#0 ff_h264_direct_ref_list_init src/libavcodec/h264_direct.c:137 
(ffmpeg+0x01382c93)

But I'm not sure because I haven't been able to reproduce locally.
---
 libavcodec/h264_slice.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index d4d31cc..8d7b6b8 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1888,7 +1888,8 @@ static int h264_slice_init(H264Context *h, 
H264SliceContext *sl,
 
 if (sl->slice_type_nos == AV_PICTURE_TYPE_B && !sl->direct_spatial_mv_pred)
 ff_h264_direct_dist_scale_factor(h, sl);
-ff_h264_direct_ref_list_init(h, sl);
+if (!h->setup_finished)
+ff_h264_direct_ref_list_init(h, sl);
 
 if (h->avctx->skip_loop_filter >= AVDISCARD_ALL ||
 (h->avctx->skip_loop_filter >= AVDISCARD_NONKEY &&
-- 
2.8.1

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

Re: [FFmpeg-devel] [PATCH 3/3] avformat/dump : Display Content Light Level metadata

[Steve Lhomme]

Thanks !

On Thu, Apr 6, 2017 at 4:45 PM, James Almer  wrote:
> On 4/2/2017 5:08 AM, Steve Lhomme wrote:
>> --
>> update the previous patch:
>> - use MaxCLL/MaxFALL which are not rational numbers anymore
>> ---
>>  libavformat/dump.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/libavformat/dump.c b/libavformat/dump.c
>> index ef4a6b093b..8fd58a0dba 100644
>> --- a/libavformat/dump.c
>> +++ b/libavformat/dump.c
>> @@ -343,6 +343,14 @@ static void dump_mastering_display_metadata(void *ctx, 
>> AVPacketSideData* sd) {
>> av_q2d(metadata->min_luminance), 
>> av_q2d(metadata->max_luminance));
>>  }
>>
>> +static void dump_content_light_metadata(void *ctx, AVPacketSideData* sd)
>> +{
>> +AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
>> +av_log(ctx, AV_LOG_INFO, "Content Light Level Metadata, "
>> +   "MaxCLL=%d, MaxFALL=%d",
>> +   metadata->MaxCLL, metadata->MaxFALL);
>> +}
>> +
>>  static void dump_spherical(void *ctx, AVCodecParameters *par, 
>> AVPacketSideData *sd)
>>  {
>>  AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data;
>> @@ -426,6 +434,9 @@ static void dump_sidedata(void *ctx, AVStream *st, const 
>> char *indent)
>>  av_log(ctx, AV_LOG_INFO, "spherical: ");
>>  dump_spherical(ctx, st->codecpar, );
>>  break;
>> +case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:
>> +dump_content_light_metadata(ctx, );
>> +break;
>>  default:
>>  av_log(ctx, AV_LOG_INFO,
>> "unknown side data type %d (%d bytes)", sd.type, 
>> sd.size);
>>
>
> Pushed, thanks.
>
> ___
> 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] pthread_frame: don't return stale error codes after flush

[Ronald S. Bultje]

Hi,

On Thu, Apr 6, 2017 at 11:36 AM, wm4  wrote:

> Consider the following sequence of events:
>
> - open a codec without AV_CODEC_CAP_DELAY
> - decode call fails with an error
> - ff_thread_flush() is called
> - drain packet is sent
>
> Then the last step would make ff_thread_decode_frame() return an error,
> because p->result can still be set to an error value. This is because
> submit_packet returns immediately if AV_CODEC_CAP_DELAY is not set, and
> no worker thread gets the chance to reset p->result, yet its value is
> trusted by ff_thread_decode_frame().
>
> Fix this by clearing the error fields on flush.
> ---
>  libavcodec/pthread_frame.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
> index 9a6b83ac45..7586f00bec 100644
> --- a/libavcodec/pthread_frame.c
> +++ b/libavcodec/pthread_frame.c
> @@ -859,6 +859,7 @@ void ff_thread_flush(AVCodecContext *avctx)
>  // Make sure decode flush calls with size=0 won't return old
> frames
>  p->got_frame = 0;
>  av_frame_unref(p->frame);
> +p->result = 0;
>
>  release_delayed_buffers(p);
>
> --
> 2.11.0


Nice catch - I think that looks good.

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

[FFmpeg-devel] [PATCH] pthread_frame: don't return stale error codes after flush

[wm4]

Consider the following sequence of events:

- open a codec without AV_CODEC_CAP_DELAY
- decode call fails with an error
- ff_thread_flush() is called
- drain packet is sent

Then the last step would make ff_thread_decode_frame() return an error,
because p->result can still be set to an error value. This is because
submit_packet returns immediately if AV_CODEC_CAP_DELAY is not set, and
no worker thread gets the chance to reset p->result, yet its value is
trusted by ff_thread_decode_frame().

Fix this by clearing the error fields on flush.
---
 libavcodec/pthread_frame.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 9a6b83ac45..7586f00bec 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -859,6 +859,7 @@ void ff_thread_flush(AVCodecContext *avctx)
 // Make sure decode flush calls with size=0 won't return old frames
 p->got_frame = 0;
 av_frame_unref(p->frame);
+p->result = 0;
 
 release_delayed_buffers(p);
 
-- 
2.11.0

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

Re: [FFmpeg-devel] [PATCH 4/5] avcodec/h264: add avx 8-bit h264_idct_add

[James Darnley]

On 2017-04-05 05:44, James Almer wrote:
> On 4/4/2017 10:53 PM, James Darnley wrote:
>> Haswell:
>>  - 1.11x faster (522±0.4 vs. 469±1.8 decicycles) compared with mmxext
>>
>> Skylake-U:
>>  - 1.21x faster (671±5.5 vs. 555±1.4 decicycles) compared with mmxext
> 
> Again, you should add an SSE2 version first, then an AVX one if it's
> measurably faster than the SSE2 one.

On a Yorkfield sse2 is barely faster: 1.02x faster (728±2.1 vs. 710±3.9
decicycles).  So 1 or 2 cycles

On a Skylake-U sse2 is most of the speedup: 1.15x faster (661±2.2 vs
573±1.9).  Then avx gains a mere 3 cycles: 547±0.5

On a Haswell sse2 provides only half the speedup:
 - sse2: 1.06x faster (525±2.5 vs 497±1.0 decicycles)
 - avx:  1.06x faster (497±1.0 vs 468±1.2 decicycles)

(All on 64-bit Linux)

On Nehalem and 64-bit Windows sse2 is slower:  0.92x faster (597±3.0 vs.
650±9.3 decicycles)

And on that note I should probably recheck the deblock patches I pushed
a little while ago.

So...  SSE2 for this function, yay or nay?

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

Re: [FFmpeg-devel] [PATCH 2/5] avcodec/h264: change some labels to be macro-local

[Ronald S. Bultje]

Hi,

On Thu, Apr 6, 2017 at 10:22 AM, James Darnley  wrote:

> On 2017-04-05 13:41, Ronald S. Bultje wrote:
> > Hi,
> >
> > On Tue, Apr 4, 2017 at 9:53 PM, James Darnley  wrote:
> >
> >> The labels get stripped leading to (slightly) nicer disassembly from
> >> objdump.
> >>
> > [..]
> >
> >> -jz .cycle%1end
> >> +jz %%skip
> >
> >
> > Can you preserve the leading dot? I don't mind the %%skip, but please
> make
> > it .%%skip.
>
> That makes the patch pointless because those symbols don't get stripped.
>  If you want the leading dot then I will drop this patch.


Oh I see, right...

So, yes, that has disadvantages as well as advantages. I don't really have
an opinion on it. So if nobody else does either, and you like it, I guess
you could commit it?

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

[FFmpeg-devel] [PATCH v2] avformat/hlsenc: add hls encrypt options

[Steven Liu]

refer to:
https://git.libav.org/?p=libav.git;a=commitdiff;h=0a4b9d0ccd10b3c39105f99bd320f696f69a75a2
add hls encrypt options looks like libav's libavformat/hlsenc.c

Signed-off-by: Steven Liu 
---
 doc/muxers.texi  |  16 +++
 libavformat/hlsenc.c | 122 ---
 2 files changed, 133 insertions(+), 5 deletions(-)

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 166c929369..db1c3db999 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -597,6 +597,22 @@ ffmpeg -f lavfi -re -i testsrc -c:v h264 -hls_flags 
delete_segments \
   -hls_key_info_file file.keyinfo out.m3u8
 @end example
 
+@item -hls_enc @var{enc}
+Enable (1) or disable (0) the AES128 encryption.
+When enabled every segment generated is encrypted and the encryption key
+is saved as @var{playlist name}.key.
+
+@item -hls_enc_key @var{key}
+Hex-coded 16byte key to encrypt the segments, by default it
+is randomly generated.
+
+@item -hls_enc_key_url @var{keyurl}
+If set, @var{keyurl} is prepended instead of @var{baseurl} to the key filename
+in the playlist.
+
+@item -hls_enc_iv @var{iv}
+Hex-coded 16byte initialization vector for every segment instead
+of the autogenerated ones.
 
 @item hls_flags @var{flags}
 Possible values:
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index e6c378df2e..f359bb9036 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -26,10 +26,18 @@
 #include 
 #endif
 
+#if CONFIG_GCRYPT
+#include 
+#elif CONFIG_OPENSSL
+#include 
+#endif
+
 #include "libavutil/avassert.h"
 #include "libavutil/mathematics.h"
 #include "libavutil/parseutils.h"
 #include "libavutil/avstring.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/random_seed.h"
 #include "libavutil/opt.h"
 #include "libavutil/log.h"
 #include "libavutil/time_internal.h"
@@ -139,6 +147,12 @@ typedef struct HLSContext {
 char *subtitle_filename;
 AVDictionary *format_options;
 
+int encrypt;
+char *key;
+char *key_url;
+char *iv;
+char *key_basename;
+
 char *key_info_file;
 char key_file[LINE_BUFFER_SIZE + 1];
 char key_uri[LINE_BUFFER_SIZE + 1];
@@ -349,6 +363,89 @@ fail:
 return ret;
 }
 
+static int randomize(uint8_t *buf, int len)
+{
+#if CONFIG_GCRYPT
+gcry_randomize(buf, len, GCRY_VERY_STRONG_RANDOM);
+return 0;
+#elif CONFIG_OPENSSL
+if (RAND_bytes(buf, len))
+return 0;
+#else
+return AVERROR(ENOSYS);
+#endif
+return AVERROR(EINVAL);
+}
+
+static int do_encrypt(AVFormatContext *s)
+{
+HLSContext *hls = s->priv_data;
+int ret;
+int len;
+AVIOContext *pb;
+uint8_t key[KEYSIZE];
+
+len = strlen(hls->basename) + 4 + 1;
+hls->key_basename = av_mallocz(len);
+if (!hls->key_basename)
+return AVERROR(ENOMEM);
+
+av_strlcpy(hls->key_basename, s->filename, len);
+av_strlcat(hls->key_basename, ".key", len);
+
+if (hls->key_url) {
+strncpy(hls->key_file, hls->key_url, sizeof(hls->key_file));
+strncpy(hls->key_uri, hls->key_url, sizeof(hls->key_uri));
+} else {
+strncpy(hls->key_file, hls->key_basename, sizeof(hls->key_file));
+strncpy(hls->key_uri, hls->key_basename, sizeof(hls->key_uri));
+}
+
+if (!*hls->iv_string) {
+uint8_t iv[16] = { 0 };
+char buf[33];
+
+if (!hls->iv) {
+AV_WB64(iv + 8, hls->sequence);
+} else {
+memcpy(iv, hls->iv, sizeof(iv));
+}
+ff_data_to_hex(buf, iv, sizeof(iv), 0);
+buf[32] = '\0';
+memcpy(hls->iv_string, buf, sizeof(hls->iv_string));
+}
+
+if (!*hls->key_uri) {
+av_log(hls, AV_LOG_ERROR, "no key URI specified in key info file\n");
+return AVERROR(EINVAL);
+}
+
+if (!*hls->key_file) {
+av_log(hls, AV_LOG_ERROR, "no key file specified in key info file\n");
+return AVERROR(EINVAL);
+}
+
+if (!*hls->key_string) {
+if (!hls->key) {
+if ((ret = randomize(key, sizeof(key))) < 0) {
+av_log(s, AV_LOG_ERROR, "Cannot generate a strong random 
key\n");
+return ret;
+}
+} else {
+memcpy(key, hls->key, sizeof(key));
+}
+
+ff_data_to_hex(hls->key_string, key, sizeof(key), 0);
+if ((ret = s->io_open(s, , hls->key_file, AVIO_FLAG_WRITE, NULL)) < 
0)
+return ret;
+avio_seek(pb, 0, SEEK_CUR);
+avio_write(pb, key, KEYSIZE);
+avio_close(pb);
+}
+return 0;
+}
+
+
 static int hls_encryption_start(AVFormatContext *s)
 {
 HLSContext *hls = s->priv_data;
@@ -662,7 +759,7 @@ static int hls_append_segment(struct AVFormatContext *s, 
HLSContext *hls, double
 hls->discontinuity = 0;
 }
 
-if (hls->key_info_file) {
+if (hls->key_info_file || hls->encrypt) {
 av_strlcpy(en->key_uri, hls->key_uri, sizeof(en->key_uri));
 av_strlcpy(en->iv_string, hls->iv_string, 

Re: [FFmpeg-devel] [PATCH 3/3] avformat/dump : Display Content Light Level metadata

[James Almer]

On 4/2/2017 5:08 AM, Steve Lhomme wrote:
> --
> update the previous patch:
> - use MaxCLL/MaxFALL which are not rational numbers anymore
> ---
>  libavformat/dump.c | 11 +++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/libavformat/dump.c b/libavformat/dump.c
> index ef4a6b093b..8fd58a0dba 100644
> --- a/libavformat/dump.c
> +++ b/libavformat/dump.c
> @@ -343,6 +343,14 @@ static void dump_mastering_display_metadata(void *ctx, 
> AVPacketSideData* sd) {
> av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
>  }
>  
> +static void dump_content_light_metadata(void *ctx, AVPacketSideData* sd)
> +{
> +AVContentLightMetadata* metadata = (AVContentLightMetadata*)sd->data;
> +av_log(ctx, AV_LOG_INFO, "Content Light Level Metadata, "
> +   "MaxCLL=%d, MaxFALL=%d",
> +   metadata->MaxCLL, metadata->MaxFALL);
> +}
> +
>  static void dump_spherical(void *ctx, AVCodecParameters *par, 
> AVPacketSideData *sd)
>  {
>  AVSphericalMapping *spherical = (AVSphericalMapping *)sd->data;
> @@ -426,6 +434,9 @@ static void dump_sidedata(void *ctx, AVStream *st, const 
> char *indent)
>  av_log(ctx, AV_LOG_INFO, "spherical: ");
>  dump_spherical(ctx, st->codecpar, );
>  break;
> +case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:
> +dump_content_light_metadata(ctx, );
> +break;
>  default:
>  av_log(ctx, AV_LOG_INFO,
> "unknown side data type %d (%d bytes)", sd.type, sd.size);
> 

Pushed, thanks.

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

Re: [FFmpeg-devel] [PATCH 2/3] lavc: Add Content Light Level side metadata found in HEVC

[James Almer]

On 4/3/2017 4:29 AM, Steve Lhomme wrote:
> These data are necessary when transmitting HDR over HDMI.
> 
> --
> update the previous patch:
> - rename CEA 861.3 to CTA-861.3
> - update with MaxCLL/MaxFALL changes in avutil
> update the previous patch:
> - rebased and updated version bump
> ---
>  libavcodec/avcodec.h  |  7 +++
>  libavcodec/avpacket.c |  1 +
>  libavcodec/hevc_sei.c | 15 +++
>  libavcodec/hevcdec.c  | 18 ++
>  libavcodec/hevcdec.h  |  5 +
>  libavcodec/utils.c|  1 +
>  libavcodec/version.h  |  2 +-
>  7 files changed, 48 insertions(+), 1 deletion(-)

Updated version since it was already outdated, added an APIChanges
line and pushed.

Thanks.

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

Re: [FFmpeg-devel] [libav-devel] ff_thread_get_format and metadata

[Steve Lhomme]

On Thu, Apr 6, 2017 at 1:08 PM, Hendrik Leppkes  wrote:
> On Thu, Apr 6, 2017 at 11:35 AM, Steve Lhomme  wrote:
>> On Thu, Apr 6, 2017 at 11:08 AM, Hendrik Leppkes  wrote:
>>> On Thu, Apr 6, 2017 at 11:01 AM, Steve Lhomme  wrote:
 Hi,

 As I am progressing in proper HDR10 support in VLC, I am facing an
 issue. The mastering (and lighting pending patch) metadata are set on
 the AVFrame but not available to the ff_get_format() receiver.

 That means when we configure our vout and decoder we don't know if
 it's going to have HDR metadata or not.

 Could we move the mastering (and lighting) metadata on the
 AVCodecContext ? Or provide the metadata on the ff_get_format() call ?

 I CC'ed libav as they have opinions on possible API changes, even
 though there is no mastering metadata support in there yet.
>>>
>>>
>>> I'm against polluting AVCodecContext with more random fields, and
>>> get_format is fixed API/ABI, we can't change that very easily.
>>
>> I understand, that's why I prefer asking before trying things out.
>>
>>> In general, this is per-frame metadata, which can appear or change
>>> with any given frame, without get_format being invoked again. You
>>> should probably just be able to have your output react to that once it
>>> receives it.
>>
>> This is already the case. I copy the relevant metadata of the AVFrame
>> in the picture we are going to display. But the underlying stream
>> format has no idea that it has HDR metadata. We can't tell the user
>> what kind of metadata the stream has (think of ffprobe-like info).
>>
>
> So you just want this for probing, not even for actual playback?
> The users will get over it then, I'm sure. Otherwise you could just
> decode one frame and retrieve the metadata - thats the best way to
> retrieve full accurate information anyway.

In this particular case, for now, yes. But I am asking a more general
question on how metadata, that are necessary for playback may be known
when configuring the playback pipeline, can be provided early.

I guess for now I can find a solution for my special use case.

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

Re: [FFmpeg-devel] [PATCH 1/3] lavu: add support for Content Light Level side metadata

[James Almer]

On 4/3/2017 4:29 AM, Steve Lhomme wrote:
> As found in HEVC.
> 
> I put the code in mastering_display_metadata as they usually go together in
> Blu-Ray UHD sources.
> 
> --
> update the previous patch:
> - rename CEA 861.3 to CTA-861.3
> - use MaxCLL and MaxFALL names that are more commonly found
> - use unsigned integer rather than rational numbers as (supposedly) in the 
> specs
> - provide the structure size on output of av_content_light_metadata_alloc()
> update the previous patch:
> - rebased and updated version bump
> update the previous patch:
> - rebased and updated version bump
> - fix contains typo
> ---
>  libavutil/frame.c  |  1 +
>  libavutil/frame.h  |  6 ++
>  libavutil/mastering_display_metadata.c | 23 
>  libavutil/mastering_display_metadata.h | 39 
> ++
>  libavutil/version.h|  2 +-
>  5 files changed, 70 insertions(+), 1 deletion(-)

Added an APIChanges line and pushed.

Thanks.

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

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: add hls encrypt options

[Steven Liu]

2017-04-06 22:07 GMT+08:00 Moritz Barsnick :

> On Thu, Apr 06, 2017 at 15:47:58 +0200, Moritz Barsnick wrote:
> > On Wed, Apr 05, 2017 at 22:04:01 +0800, Steven Liu wrote:
> > > +{"hls_enc",   "AES128 encryption support",
>  OFFSET(encrypt), AV_OPT_TYPE_INT,{.i64 = 0}, 0, 1, E},
>
> Furthermore, this looks like a boolean option, -> AV_OPT_TYPE_BOOL,
>  {"hls_enc",   "enable AES128 encryption support"
> or something like this.
>
> > > +{"hls_enc_key",   "use the specified hex-coded 16byte key to
> encrypt the segments",  OFFSET(key), AV_OPT_TYPE_STRING, .flags = E},
> > > +{"hls_enc_key_url", "url to access the key to decrypt the
> segments",OFFSET(key_url), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0, E},
> > > +{"hls_enc_iv", "use the specified hex-coded 16byte
> initialization vector",  OFFSET(iv), AV_OPT_TYPE_STRING, .flags = E},
> >
> > "use the specified" makes it sound like a boolean option. I suggest:
> >
> >  {"hls_enc_key", "hex-coded 16 byte key to encrypt the segments" [...]
> >  {"hls_enc_iv", "hex-coded 16 byte initialization vector" [...]
> >
> > And these should be documented of course.
> >
> > BTW, your use of spaces in the option declaration is quite ... weird.
> > ;-)
> >
> > Moritz
>

Just move copy the option from
https://git.libav.org/?p=libav.git;a=commitdiff;h=0a4b9d0ccd10b3c39105f99bd320f696f69a75a2
and i will add option document copy from
https://git.libav.org/?p=libav.git;a=commitdiff;h=0a4b9d0ccd10b3c39105f99bd320f696f69a75a2


> ___
> 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] avformat/hlsenc: add hls encrypt options

[Steven Liu]

2017-04-06 22:07 GMT+08:00 Moritz Barsnick :

> On Thu, Apr 06, 2017 at 15:47:58 +0200, Moritz Barsnick wrote:
> > On Wed, Apr 05, 2017 at 22:04:01 +0800, Steven Liu wrote:
> > > +{"hls_enc",   "AES128 encryption support",
>  OFFSET(encrypt), AV_OPT_TYPE_INT,{.i64 = 0}, 0, 1, E},
>
> Furthermore, this looks like a boolean option, -> AV_OPT_TYPE_BOOL,
>  {"hls_enc",   "enable AES128 encryption support"
> or something like this.
>
> > > +{"hls_enc_key",   "use the specified hex-coded 16byte key to
> encrypt the segments",  OFFSET(key), AV_OPT_TYPE_STRING, .flags = E},
> > > +{"hls_enc_key_url", "url to access the key to decrypt the
> segments",OFFSET(key_url), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0, E},
> > > +{"hls_enc_iv", "use the specified hex-coded 16byte
> initialization vector",  OFFSET(iv), AV_OPT_TYPE_STRING, .flags = E},
> >
> > "use the specified" makes it sound like a boolean option. I suggest:
> >
> >  {"hls_enc_key", "hex-coded 16 byte key to encrypt the segments" [...]
> >  {"hls_enc_iv", "hex-coded 16 byte initialization vector" [...]
> >
> > And these should be documented of course.
> >
> > BTW, your use of spaces in the option declaration is quite ... weird.
> > ;-)
> >
> > Moritz
>

But  i think Moritz suggestion is useful, will apply the suggestions to new
patch  :)

> ___
> 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/5] avcodec/h264: change some labels to be macro-local

[James Darnley]

On 2017-04-05 13:41, Ronald S. Bultje wrote:
> Hi,
> 
> On Tue, Apr 4, 2017 at 9:53 PM, James Darnley  wrote:
> 
>> The labels get stripped leading to (slightly) nicer disassembly from
>> objdump.
>>
> [..]
> 
>> -jz .cycle%1end
>> +jz %%skip
> 
> 
> Can you preserve the leading dot? I don't mind the %%skip, but please make
> it .%%skip.

That makes the patch pointless because those symbols don't get stripped.
 If you want the leading dot then I will drop this patch.

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

Re: [FFmpeg-devel] [PATCH] Build fix for MIPS

[Ronald S. Bultje]

Hi,

On Wed, Apr 5, 2017 at 4:53 PM, Ronald S. Bultje  wrote:

> Hi,
>
> On Wed, Apr 5, 2017 at 8:38 AM, Shivraj Patil 
> wrote:
>
>> Hi,
>>
>>
>>
>> Updated the patch as per comments.
>>
>> As hevcpred_init_mips.c needs definition of HAVE_MSA and av_cold, now I
>> have included config.h & libavutil/attributes.h in this file.
>>
>
> Patch LGTM. I'll leave it up for a day for others to comment before I push.
>

Pushed.

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

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: add hls encrypt options

[Moritz Barsnick]

On Thu, Apr 06, 2017 at 15:47:58 +0200, Moritz Barsnick wrote:
> On Wed, Apr 05, 2017 at 22:04:01 +0800, Steven Liu wrote:
> > +{"hls_enc",   "AES128 encryption support",   
> > OFFSET(encrypt), AV_OPT_TYPE_INT,{.i64 = 0}, 0, 1, E},

Furthermore, this looks like a boolean option, -> AV_OPT_TYPE_BOOL,
 {"hls_enc",   "enable AES128 encryption support"
or something like this.

> > +{"hls_enc_key",   "use the specified hex-coded 16byte key to encrypt 
> > the segments",  OFFSET(key), AV_OPT_TYPE_STRING, .flags = E},
> > +{"hls_enc_key_url", "url to access the key to decrypt the segments",   
> >  OFFSET(key_url), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0, E},
> > +{"hls_enc_iv", "use the specified hex-coded 16byte initialization 
> > vector",  OFFSET(iv), AV_OPT_TYPE_STRING, .flags = E},
> 
> "use the specified" makes it sound like a boolean option. I suggest:
> 
>  {"hls_enc_key", "hex-coded 16 byte key to encrypt the segments" [...]
>  {"hls_enc_iv", "hex-coded 16 byte initialization vector" [...]
> 
> And these should be documented of course.
> 
> BTW, your use of spaces in the option declaration is quite ... weird.
> ;-)
> 
> Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/hlsenc: add hls encrypt options

[Moritz Barsnick]

On Wed, Apr 05, 2017 at 22:04:01 +0800, Steven Liu wrote:
> +{"hls_enc",   "AES128 encryption support",   
> OFFSET(encrypt), AV_OPT_TYPE_INT,{.i64 = 0}, 0, 1, E},
> +{"hls_enc_key",   "use the specified hex-coded 16byte key to encrypt the 
> segments",  OFFSET(key), AV_OPT_TYPE_STRING, .flags = E},
> +{"hls_enc_key_url", "url to access the key to decrypt the segments",
> OFFSET(key_url), AV_OPT_TYPE_STRING, {.str = NULL},  0, 0, E},
> +{"hls_enc_iv", "use the specified hex-coded 16byte initialization 
> vector",  OFFSET(iv), AV_OPT_TYPE_STRING, .flags = E},

"use the specified" makes it sound like a boolean option. I suggest:

 {"hls_enc_key", "hex-coded 16 byte key to encrypt the segments" [...]
 {"hls_enc_iv", "hex-coded 16 byte initialization vector" [...]

And these should be documented of course.

BTW, your use of spaces in the option declaration is quite ... weird.
;-)

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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_framerate: always request input if no output is provided in request_frame

[Moritz Barsnick]

On Wed, Apr 05, 2017 at 02:03:00 +0200, Marton Balint wrote:
> -static int process_work_frame(AVFilterContext *ctx, int stop)
> +static int process_work_frame(AVFilterContext *ctx, int filter_frame)

Just from a level of confusion, I don't find it smart to name a
variable (albeit a local one) the same as an existing function.

> +return filter_frame ? 0 : ff_request_frame(ctx->inputs[0]);

The unknowing code reader (as myself) could interpret this as the check
for a function pointer. Only because I know functions of the name
filter_frame() are often used in filters. Yeah, silly me.

Just nitpicking a bit, perhaps quite irrelevant,
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] vf_framestep: add blend parameter for motion blur effect

[Matthias Troffaes]

Many thanks for your feedback, Michael and Nicolas.

On Wed, Apr 5, 2017 at 7:02 PM, Michael Niedermayer
 wrote:
> fails on qemu mips

Thank you for flagging this failure. I'm currently trying to reproduce
the problem. May I ask what toolchain you typically use to compile to
mips? (I'm trying debian malta on qemu-system-mips, but it seems that
the configure script can't correctly set up config.h in that
environment.)

On Thu, Apr 6, 2017 at 8:42 AM, Nicolas George  wrote:
> There is already blending code in vf_framerate and vf_overlay,

... indeed, and also in vf_blend.

> I think
> we should not add new similar code without sharing it as much as
> possible.

Thanks for pointing out the existence of similar code. I agree.

There is however one problem: all blend codes mentioned work on 2
frames only. Whilst it is possible to express an n-frame blend in
terms of n-1 consecutive 2-frame blends with appropriate weights, the
issues with doing so are that it is much slower for large n (n-1
additions + 1 divide per pixel for n-frame blend, versus n-1 additions
+ n-1 multiplications + n-1 divides per pixel for n-1 consecutive
2-frame blends), and that the precision might be compromised as the
current 2-frame blend code round everything to the integer at each
step. Here's some python code to demonstrate the latter problem:

rnd = round

def blend_exact(xs):
return rnd(sum(xs) / float(len(xs)))

def blend_recurse(xs):
head = xs[0]
if len(xs) == 1:
return head
else:
tail = xs[1:]
return rnd((head + len(tail) * blend_recurse(tail)) / float(len(xs)))

def test(xs):
print(blend_exact(xs))
print(blend_recurse(xs))

test([1,1,3])
test([1,1,1,3])
...
test([1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,1,3])
...

The exact version gives 1 for all of these test cases, whilst the
recursive version gives 2 - I'd say that's quite bad: you can add as
many 1's to the front as you want, the result for the recursive
version will always return 2, even if the exact result is arbitrarily
close to 1.

One way of fixing this is by increasing the precision of the 2-frame
blend. However, the current blending codes for 2 frames, as they are
designed now in ffmpeg, do not have increased precision in the target
frame. They are therefore unsuitable for consecutively blending a
large number of frames: they are both slower and less inaccurate for
that purpose when compared to the exact version as in the patch.

This all said, I could absolutely see good use in refactoring the code
to provide a common basis for the various frame blending codes that
are already out there. Given that there is no suitable n-frame blend
code currently elsewhere (unless someone can point me to it, but I
didn't find anything), do you think it is fair of me to argue that
such refactoring is beyond the scope of this specific patch?

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

Re: [FFmpeg-devel] [libav-devel] ff_thread_get_format and metadata

[Hendrik Leppkes]

On Thu, Apr 6, 2017 at 11:35 AM, Steve Lhomme  wrote:
> On Thu, Apr 6, 2017 at 11:08 AM, Hendrik Leppkes  wrote:
>> On Thu, Apr 6, 2017 at 11:01 AM, Steve Lhomme  wrote:
>>> Hi,
>>>
>>> As I am progressing in proper HDR10 support in VLC, I am facing an
>>> issue. The mastering (and lighting pending patch) metadata are set on
>>> the AVFrame but not available to the ff_get_format() receiver.
>>>
>>> That means when we configure our vout and decoder we don't know if
>>> it's going to have HDR metadata or not.
>>>
>>> Could we move the mastering (and lighting) metadata on the
>>> AVCodecContext ? Or provide the metadata on the ff_get_format() call ?
>>>
>>> I CC'ed libav as they have opinions on possible API changes, even
>>> though there is no mastering metadata support in there yet.
>>
>>
>> I'm against polluting AVCodecContext with more random fields, and
>> get_format is fixed API/ABI, we can't change that very easily.
>
> I understand, that's why I prefer asking before trying things out.
>
>> In general, this is per-frame metadata, which can appear or change
>> with any given frame, without get_format being invoked again. You
>> should probably just be able to have your output react to that once it
>> receives it.
>
> This is already the case. I copy the relevant metadata of the AVFrame
> in the picture we are going to display. But the underlying stream
> format has no idea that it has HDR metadata. We can't tell the user
> what kind of metadata the stream has (think of ffprobe-like info).
>

So you just want this for probing, not even for actual playback?
The users will get over it then, I'm sure. Otherwise you could just
decode one frame and retrieve the metadata - thats the best way to
retrieve full accurate information anyway.

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

Re: [FFmpeg-devel] [PATCH] ffmpeg_opt: remove the stats metadata added by mkvmerge.

[wm4]

On Thu,  6 Apr 2017 12:27:43 +0200
Nicolas George  wrote:

> They are probably not valid for the resulting file.
> They look like this:
>   BPS : 40665
>   BPS-eng : 40665
>   DURATION: 00:00:20.0
>   DURATION-eng: 00:00:20.0
>   NUMBER_OF_FRAMES: 10
>   NUMBER_OF_FRAMES-eng: 10
>   NUMBER_OF_BYTES : 101664
>   NUMBER_OF_BYTES-eng: 101664
>   _STATISTICS_WRITING_APP: mkvmerge v9.8.0 ('Kuglblids') 64bit
>   _STATISTICS_WRITING_APP-eng: mkvmerge v9.8.0 ('Kuglblids') 64bit
>   _STATISTICS_WRITING_DATE_UTC: 2017-04-06 08:22:08
>   _STATISTICS_WRITING_DATE_UTC-eng: 2017-04-06 08:22:08
>   _STATISTICS_TAGS: BPS DURATION NUMBER_OF_FRAMES NUMBER_OF_BYTES
>   _STATISTICS_TAGS-eng: BPS DURATION NUMBER_OF_FRAMES NUMBER_OF_BYTES
> 
> Signed-off-by: Nicolas George 
> ---
>  ffmpeg_opt.c | 49 -
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> 
> Note: as written in a comment, this code ignores the return value of
> av_dict_set(), because the surrounding code ignores the return value of
> av_dict_copy(). A lot of unchecked errors happen in this function, but I am
> not reworking a 2700-lines function today.
> 
> 
> diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> index d1fe8742ff..f72f38796a 100644
> --- a/ffmpeg_opt.c
> +++ b/ffmpeg_opt.c
> @@ -497,6 +497,53 @@ static void parse_meta_type(char *arg, char *type, int 
> *index, const char **stre
>  *type = 'g';
>  }
>  
> +/**
> + * Copy a dictionary except the stats metadata added by mkvmerge.
> + * They are probably not valid for the resulting file.
> + *
> + * FIXME Return values are ignored by the surrounding code and here.
> + */
> +static void dict_copy_nostats(AVDictionary **dst, const AVDictionary *src)
> +{
> +AVDictionaryEntry *t;
> +const char *tags = NULL, *p, *q;
> +size_t len;
> +
> +t = av_dict_get(src, "_STATISTICS_TAGS", NULL, AV_DICT_MATCH_CASE);
> +if (t)
> +tags = t->value;
> +t = NULL;
> +while ((t = av_dict_get(src, "", t, AV_DICT_IGNORE_SUFFIX))) {
> +if (av_strstart(t->key, "_STATISTICS_", NULL))
> +continue;
> +if (tags) {
> +/* tags is a space-separated list of stats tags */
> +p = tags;
> +while (*p) {
> +q = strchr(p, ' ');
> +len = q ? q - p : strlen(p);
> +if (!q)
> +q = p + strlen(p);
> +#define ff_islower(c) ((unsigned)((c) - 'a') < 26)
> +if (!memcmp(t->key, p, len) &&
> +(!t->key[len] ||
> + /* also remove TAG-lng */
> + (t->key[len] == '-' &&
> +  ff_islower(t->key[len + 1]) &&
> +  ff_islower(t->key[len + 2]) &&
> +  ff_islower(t->key[len + 3]) &&
> +  !t->key[len + 4])))
> +break;
> +#undef ff_islower
> +for (p += len; *p == ' '; p++);
> +}
> +if (*p) /* match found => do not copy */
> +continue;
> +}
> +av_dict_set(dst, t->key, t->value, AV_DICT_DONT_OVERWRITE);
> +}
> +}
> +
>  static int copy_metadata(char *outspec, char *inspec, AVFormatContext *oc, 
> AVFormatContext *ic, OptionsContext *o)
>  {
>  AVDictionary **meta_in = NULL;
> @@ -2546,7 +2593,7 @@ loop_end:
>  if (output_streams[i]->source_index < 0) /* this is true 
> e.g. for attached files */
>  continue;
>  ist = input_streams[output_streams[i]->source_index];
> -av_dict_copy(_streams[i]->st->metadata, 
> ist->st->metadata, AV_DICT_DONT_OVERWRITE);
> +dict_copy_nostats(_streams[i]->st->metadata, 
> ist->st->metadata);
>  if (!output_streams[i]->stream_copy) {
>  av_dict_set(_streams[i]->st->metadata, "encoder", 
> NULL, 0);
>  }

I sure love format-specific hacks in ffmpeg CLI!

I don't think this patch is acceptable.

I went on trying to find out how Matroska readers are supposed to
handle this:

 mosu: what's the correct way to discard _STATISTICS tags if you want only 
"actual" tags the user added?
 matching by name doesn't work, because there are many names to be 
considered and they don't all start with _ or _STATISTICS
 You mean in the context of programs other than mkvmerge?
 yes
 mkvmerge adds one tag called _STATISTICS_TAGS which includes the a 
space-separated list of names of "normal" tags it has added. Additionally it 
adds _STATISTICS_WRITING_APP and _STATISTICS_WRITING_DATE.
 that's... painful
 There's nothing in the specs about this, though; it's a convention 
created by/invented for mkvmerge

So that's what you have to implement in the demuxer.
___
ffmpeg-devel mailing list

[FFmpeg-devel] [PATCH] ffmpeg_opt: remove the stats metadata added by mkvmerge.

[Nicolas George]

They are probably not valid for the resulting file.
They look like this:
  BPS : 40665
  BPS-eng : 40665
  DURATION: 00:00:20.0
  DURATION-eng: 00:00:20.0
  NUMBER_OF_FRAMES: 10
  NUMBER_OF_FRAMES-eng: 10
  NUMBER_OF_BYTES : 101664
  NUMBER_OF_BYTES-eng: 101664
  _STATISTICS_WRITING_APP: mkvmerge v9.8.0 ('Kuglblids') 64bit
  _STATISTICS_WRITING_APP-eng: mkvmerge v9.8.0 ('Kuglblids') 64bit
  _STATISTICS_WRITING_DATE_UTC: 2017-04-06 08:22:08
  _STATISTICS_WRITING_DATE_UTC-eng: 2017-04-06 08:22:08
  _STATISTICS_TAGS: BPS DURATION NUMBER_OF_FRAMES NUMBER_OF_BYTES
  _STATISTICS_TAGS-eng: BPS DURATION NUMBER_OF_FRAMES NUMBER_OF_BYTES

Signed-off-by: Nicolas George 
---
 ffmpeg_opt.c | 49 -
 1 file changed, 48 insertions(+), 1 deletion(-)


Note: as written in a comment, this code ignores the return value of
av_dict_set(), because the surrounding code ignores the return value of
av_dict_copy(). A lot of unchecked errors happen in this function, but I am
not reworking a 2700-lines function today.


diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
index d1fe8742ff..f72f38796a 100644
--- a/ffmpeg_opt.c
+++ b/ffmpeg_opt.c
@@ -497,6 +497,53 @@ static void parse_meta_type(char *arg, char *type, int 
*index, const char **stre
 *type = 'g';
 }
 
+/**
+ * Copy a dictionary except the stats metadata added by mkvmerge.
+ * They are probably not valid for the resulting file.
+ *
+ * FIXME Return values are ignored by the surrounding code and here.
+ */
+static void dict_copy_nostats(AVDictionary **dst, const AVDictionary *src)
+{
+AVDictionaryEntry *t;
+const char *tags = NULL, *p, *q;
+size_t len;
+
+t = av_dict_get(src, "_STATISTICS_TAGS", NULL, AV_DICT_MATCH_CASE);
+if (t)
+tags = t->value;
+t = NULL;
+while ((t = av_dict_get(src, "", t, AV_DICT_IGNORE_SUFFIX))) {
+if (av_strstart(t->key, "_STATISTICS_", NULL))
+continue;
+if (tags) {
+/* tags is a space-separated list of stats tags */
+p = tags;
+while (*p) {
+q = strchr(p, ' ');
+len = q ? q - p : strlen(p);
+if (!q)
+q = p + strlen(p);
+#define ff_islower(c) ((unsigned)((c) - 'a') < 26)
+if (!memcmp(t->key, p, len) &&
+(!t->key[len] ||
+ /* also remove TAG-lng */
+ (t->key[len] == '-' &&
+  ff_islower(t->key[len + 1]) &&
+  ff_islower(t->key[len + 2]) &&
+  ff_islower(t->key[len + 3]) &&
+  !t->key[len + 4])))
+break;
+#undef ff_islower
+for (p += len; *p == ' '; p++);
+}
+if (*p) /* match found => do not copy */
+continue;
+}
+av_dict_set(dst, t->key, t->value, AV_DICT_DONT_OVERWRITE);
+}
+}
+
 static int copy_metadata(char *outspec, char *inspec, AVFormatContext *oc, 
AVFormatContext *ic, OptionsContext *o)
 {
 AVDictionary **meta_in = NULL;
@@ -2546,7 +2593,7 @@ loop_end:
 if (output_streams[i]->source_index < 0) /* this is true 
e.g. for attached files */
 continue;
 ist = input_streams[output_streams[i]->source_index];
-av_dict_copy(_streams[i]->st->metadata, ist->st->metadata, 
AV_DICT_DONT_OVERWRITE);
+dict_copy_nostats(_streams[i]->st->metadata, 
ist->st->metadata);
 if (!output_streams[i]->stream_copy) {
 av_dict_set(_streams[i]->st->metadata, "encoder", NULL, 
0);
 }
-- 
2.11.0

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

Re: [FFmpeg-devel] [libav-devel] ff_thread_get_format and metadata

[Steve Lhomme]

On Thu, Apr 6, 2017 at 11:38 AM, wm4  wrote:
> On Thu, 6 Apr 2017 11:08:24 +0200
> Hendrik Leppkes  wrote:
>
>> On Thu, Apr 6, 2017 at 11:01 AM, Steve Lhomme  wrote:
>> > Hi,
>> >
>> > As I am progressing in proper HDR10 support in VLC, I am facing an
>> > issue. The mastering (and lighting pending patch) metadata are set on
>> > the AVFrame but not available to the ff_get_format() receiver.
>> >
>> > That means when we configure our vout and decoder we don't know if
>> > it's going to have HDR metadata or not.
>> >
>> > Could we move the mastering (and lighting) metadata on the
>> > AVCodecContext ? Or provide the metadata on the ff_get_format() call ?
>> >
>> > I CC'ed libav as they have opinions on possible API changes, even
>> > though there is no mastering metadata support in there yet.
>>
>>
>> I'm against polluting AVCodecContext with more random fields, and
>> get_format is fixed API/ABI, we can't change that very easily.
>>
>> In general, this is per-frame metadata, which can appear or change
>> with any given frame, without get_format being invoked again. You
>> should probably just be able to have your output react to that once it
>> receives it.
>
> +1
>
> Also, get_format is not for such things - it's to allocate frames
> _only_, and thus needs only size and pixfmt.

Yes, but for example if you're going to have stereo pictures you may
need to setup your allocated textures a certain way.

Also AVColorPrimaries are found in the mastering metadata are found in
AVCodecContext, should the metadata write there instead ?

> You should configure your vout based on the _decoded_ frame, not
> get_format.
> ___
> libav-devel mailing list
> libav-de...@libav.org
> https://lists.libav.org/mailman/listinfo/libav-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [libav-devel] ff_thread_get_format and metadata

[wm4]

On Thu, 6 Apr 2017 11:08:24 +0200
Hendrik Leppkes  wrote:

> On Thu, Apr 6, 2017 at 11:01 AM, Steve Lhomme  wrote:
> > Hi,
> >
> > As I am progressing in proper HDR10 support in VLC, I am facing an
> > issue. The mastering (and lighting pending patch) metadata are set on
> > the AVFrame but not available to the ff_get_format() receiver.
> >
> > That means when we configure our vout and decoder we don't know if
> > it's going to have HDR metadata or not.
> >
> > Could we move the mastering (and lighting) metadata on the
> > AVCodecContext ? Or provide the metadata on the ff_get_format() call ?
> >
> > I CC'ed libav as they have opinions on possible API changes, even
> > though there is no mastering metadata support in there yet.  
> 
> 
> I'm against polluting AVCodecContext with more random fields, and
> get_format is fixed API/ABI, we can't change that very easily.
> 
> In general, this is per-frame metadata, which can appear or change
> with any given frame, without get_format being invoked again. You
> should probably just be able to have your output react to that once it
> receives it.

+1

Also, get_format is not for such things - it's to allocate frames
_only_, and thus needs only size and pixfmt.

You should configure your vout based on the _decoded_ frame, not
get_format.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [libav-devel] ff_thread_get_format and metadata

[Steve Lhomme]

On Thu, Apr 6, 2017 at 11:08 AM, Hendrik Leppkes  wrote:
> On Thu, Apr 6, 2017 at 11:01 AM, Steve Lhomme  wrote:
>> Hi,
>>
>> As I am progressing in proper HDR10 support in VLC, I am facing an
>> issue. The mastering (and lighting pending patch) metadata are set on
>> the AVFrame but not available to the ff_get_format() receiver.
>>
>> That means when we configure our vout and decoder we don't know if
>> it's going to have HDR metadata or not.
>>
>> Could we move the mastering (and lighting) metadata on the
>> AVCodecContext ? Or provide the metadata on the ff_get_format() call ?
>>
>> I CC'ed libav as they have opinions on possible API changes, even
>> though there is no mastering metadata support in there yet.
>
>
> I'm against polluting AVCodecContext with more random fields, and
> get_format is fixed API/ABI, we can't change that very easily.

I understand, that's why I prefer asking before trying things out.

> In general, this is per-frame metadata, which can appear or change
> with any given frame, without get_format being invoked again. You
> should probably just be able to have your output react to that once it
> receives it.

This is already the case. I copy the relevant metadata of the AVFrame
in the picture we are going to display. But the underlying stream
format has no idea that it has HDR metadata. We can't tell the user
what kind of metadata the stream has (think of ffprobe-like info).

Also as a more general case there can even be an issue with
selecting/configuring the display when ff_get_format() is called. For
example you don't know the frame packing format (3D, side by side,
etc). It's available in SEI_TYPE_FRAME_PACKING for H264 and HEVC.

Couldn't there be a way to query AVCodecContext for certain metadata ?
In the case of HEVC/H264 it would look in the calling associated
HEVCContext/H264Context. Which is the (upper) context that is calling
the ff_get_format().

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

Re: [FFmpeg-devel] [libav-devel] ff_thread_get_format and metadata

[Hendrik Leppkes]

On Thu, Apr 6, 2017 at 11:01 AM, Steve Lhomme  wrote:
> Hi,
>
> As I am progressing in proper HDR10 support in VLC, I am facing an
> issue. The mastering (and lighting pending patch) metadata are set on
> the AVFrame but not available to the ff_get_format() receiver.
>
> That means when we configure our vout and decoder we don't know if
> it's going to have HDR metadata or not.
>
> Could we move the mastering (and lighting) metadata on the
> AVCodecContext ? Or provide the metadata on the ff_get_format() call ?
>
> I CC'ed libav as they have opinions on possible API changes, even
> though there is no mastering metadata support in there yet.


I'm against polluting AVCodecContext with more random fields, and
get_format is fixed API/ABI, we can't change that very easily.

In general, this is per-frame metadata, which can appear or change
with any given frame, without get_format being invoked again. You
should probably just be able to have your output react to that once it
receives it.

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

[FFmpeg-devel] ff_thread_get_format and metadata

[Steve Lhomme]

Hi,

As I am progressing in proper HDR10 support in VLC, I am facing an
issue. The mastering (and lighting pending patch) metadata are set on
the AVFrame but not available to the ff_get_format() receiver.

That means when we configure our vout and decoder we don't know if
it's going to have HDR metadata or not.

Could we move the mastering (and lighting) metadata on the
AVCodecContext ? Or provide the metadata on the ff_get_format() call ?

I CC'ed libav as they have opinions on possible API changes, even
though there is no mastering metadata support in there yet.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/4] ffmpeg: use reordered duration for stream PTS.

[wm4]

On Thu, 6 Apr 2017 11:04:48 +0200
Nicolas George  wrote:

> Le septidi 17 germinal, an CCXXV, Hendrik Leppkes a écrit :
> > pkt_duration is often missing or incorrect.  
> 
> I have FATE-tested the resulting code and it exhibits no regression.

FATE doesn't test most of the potentially problematic decoders. Also,
I'm not sure how FATE is supposed to inform you about broken
pkt_durations if it passed before.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/4] ffmpeg: use reordered duration for stream PTS.

[Nicolas George]

Le septidi 17 germinal, an CCXXV, Hendrik Leppkes a écrit :
> pkt_duration is often missing or incorrect.

I have FATE-tested the resulting code and it exhibits no regression.

> At the very last falling back to the old logic if its unset seems
> sensible.

I do not think so. The old logic was utterly flawed, mixing PTS and DTS.
If the new logic did exhibit regressions, I would have tried to find a
proper fix before submitting. Since it does not, I say we wait for an
actual problematic case to implement workaround logic.

Regards,

-- 
  Nicolas George


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/4] lavfi/buffersrc: add av_buffersrc_close().

[wm4]

On Thu,  6 Apr 2017 10:44:50 +0200
Nicolas George  wrote:

> TODO APIChanges and minor bump.
> 
> Signed-off-by: Nicolas George 
> ---
>  libavfilter/buffersrc.c | 22 --
>  libavfilter/buffersrc.h |  8 
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
> index 3f80d5f413..250b06dfbc 100644
> --- a/libavfilter/buffersrc.c
> +++ b/libavfilter/buffersrc.c
> @@ -196,16 +196,9 @@ static int 
> av_buffersrc_add_frame_internal(AVFilterContext *ctx,
>  
>  s->nb_failed_requests = 0;
>  
> -if (!frame) {
> -s->eof = 1;
> -ff_avfilter_link_set_in_status(ctx->outputs[0], AVERROR_EOF, 
> AV_NOPTS_VALUE);
> -if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
> -ret = push_frame(ctx->graph);
> -if (ret < 0)
> -return ret;
> -}
> -return 0;
> -} else if (s->eof)
> +if (!frame)
> +return av_buffersrc_close(ctx, AV_NOPTS_VALUE, flags);
> +if (s->eof)
>  return AVERROR(EINVAL);
>  
>  refcounted = !!frame->buf[0];
> @@ -267,6 +260,15 @@ static int 
> av_buffersrc_add_frame_internal(AVFilterContext *ctx,
>  return 0;
>  }
>  
> +int av_buffersrc_close(AVFilterContext *ctx, int64_t pts, unsigned flags)
> +{
> +BufferSourceContext *s = ctx->priv;
> +
> +s->eof = 1;
> +ff_avfilter_link_set_in_status(ctx->outputs[0], AVERROR_EOF, pts);
> +return (flags & AV_BUFFERSRC_FLAG_PUSH) ? push_frame(ctx->graph) : 0;
> +}
> +
>  static av_cold int init_video(AVFilterContext *ctx)
>  {
>  BufferSourceContext *c = ctx->priv;
> diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
> index e42c78196b..0652113f2b 100644
> --- a/libavfilter/buffersrc.h
> +++ b/libavfilter/buffersrc.h
> @@ -193,6 +193,14 @@ av_warn_unused_result
>  int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
>   AVFrame *frame, int flags);
>  
> +/**
> + * Close the buffer source after EOF.
> + *
> + * This is similar to passing NULL to av_buffersrc_add_frame_flags()
> + * except it takes the timestamp of the EOF, i.e. the timestamp of the end
> + * of the last frame.
> + */
> +int av_buffersrc_close(AVFilterContext *ctx, int64_t pts, unsigned flags);
>  
>  /**
>   * @}

Not sure why this special function, instead of adding a frame duration
field to AVFrame. (Which would not necessarily be the often unreliable
or broken pkt_duration.)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/4] ffmpeg: use reordered duration for stream PTS.

[Hendrik Leppkes]

On Thu, Apr 6, 2017 at 10:44 AM, Nicolas George  wrote:
> Signed-off-by: Nicolas George 
> ---
>  ffmpeg.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
>
> Unchanged. Necessary for the next patches.
>
>
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 4db8ea82ac..41f1f076ca 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -2357,7 +2357,7 @@ static int decode_audio(InputStream *ist, AVPacket 
> *pkt, int *got_output,
>  return err < 0 ? err : ret;
>  }
>
> -static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, 
> int eof,
> +static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, 
> int *duration_pts, int eof,
>  int *decode_failed)
>  {
>  AVFrame *decoded_frame;
> @@ -2448,6 +2448,7 @@ static int decode_video(InputStream *ist, AVPacket 
> *pkt, int *got_output, int eo
>  ist->hwaccel_retrieved_pix_fmt = decoded_frame->format;
>
>  best_effort_timestamp= av_frame_get_best_effort_timestamp(decoded_frame);
> +*duration_pts = decoded_frame->pkt_duration;
>
>  if (ist->framerate.num)
>  best_effort_timestamp = ist->cfr_next_pts++;
> @@ -2618,6 +2619,7 @@ static int process_input_packet(InputStream *ist, const 
> AVPacket *pkt, int no_eo
>  // while we have more to decode or while the decoder did output 
> something on EOF
>  while (ist->decoding_needed) {
>  int duration_dts = 0;
> +int duration_pts = 0;
>  int got_output = 0;
>  int decode_failed = 0;
>
> @@ -2630,7 +2632,7 @@ static int process_input_packet(InputStream *ist, const 
> AVPacket *pkt, int no_eo
> _failed);
>  break;
>  case AVMEDIA_TYPE_VIDEO:
> -ret = decode_video(ist, repeating ? NULL : , 
> _output, !pkt,
> +ret = decode_video(ist, repeating ? NULL : , 
> _output, _pts, !pkt,
> _failed);
>  if (!repeating || !pkt || got_output) {
>  if (pkt && pkt->duration) {
> @@ -2649,7 +2651,7 @@ static int process_input_packet(InputStream *ist, const 
> AVPacket *pkt, int no_eo
>  }
>
>  if (got_output)
> -ist->next_pts += duration_dts; //FIXME the duration is not 
> correct in some cases
> +ist->next_pts += av_rescale_q(duration_pts, 
> ist->st->time_base, AV_TIME_BASE_Q);
>  break;
>  case AVMEDIA_TYPE_SUBTITLE:
>  if (repeating)
> --
> 2.11.0
>

As was pointed out before, pkt_duration is often missing or incorrect.
At the very last falling back to the old logic if its unset seems
sensible.

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

[FFmpeg-devel] Patch: filter af_amix inputs

[John Warburton]

Dear All,

While needing automatically to mix several hundred audio files, I
noticed that the libavfilter module af_amix.c (audio filter 'amix') is
hard-coded to limit inputs to 32. Of course, I believe this to be
sensible as no doubt you do, too.

However, because the code appears to be written robustly, it was
trivial to increase the limit to 1024 and, on a fast machine with SATA
drives, I have used FFmpeg to mix simultaneously over 400 16-bit
stereo wav files (48kHz sample rate) at approximately 2 x realtime. It
starts very slowly, but speed soon builds up.

Naturally, memory use increased. But the job was done. It's generating
files for a sound installation in an art exhibition in Brighton, UK,
where many hundreds of migrant birds' songs must be heard together
("SWAY" at the ONCA Gallery).

Below is the very trivial patch in case the result of this experiment
is of interest.

Thank you for all the work you do to make FFmpeg so incredibly useful.
J

--- libavfilter/af_amix.c.orig 2017-04-05 22:26:26.326379600 +0100
+++ libavfilter/af_amix.c 2017-04-05 18:00:59.291196000 +0100
@@ -177,7 +177,7 @@
 #define F AV_OPT_FLAG_FILTERING_PARAM
 static const AVOption amix_options[] = {
 { "inputs", "Number of inputs.",
-OFFSET(nb_inputs), AV_OPT_TYPE_INT, { .i64 = 2 }, 1, 32, A|F },
+OFFSET(nb_inputs), AV_OPT_TYPE_INT, { .i64 = 2 }, 1, 1024, A|F },
 { "duration", "How to determine the end-of-stream.",
 OFFSET(duration_mode), AV_OPT_TYPE_INT, { .i64 =
DURATION_LONGEST }, 0,  2, A|F, "duration" },
 { "longest",  "Duration of longest input.",  0,
AV_OPT_TYPE_CONST, { .i64 = DURATION_LONGEST  }, INT_MIN, INT_MAX,
A|F, "duration" },





--
John Warburton
Tonmeister, Director,
Associate Lecturer, University of Surrey Department of Music and Media
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 4/4] ffmpeg: send EOF pts to filters.

[Nicolas George]

Signed-off-by: Nicolas George 
---
 ffmpeg.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)


With this change, filters have a timestamp when EOF is reached, available in
link->current_pts. For example, vf_fps could make use of it to fix the
duration of the last frame; the logic in vf_fps seems unnecessarily
complicated, I have not yet implemented this, but a quick fix could be
possible.


diff --git a/ffmpeg.c b/ffmpeg.c
index 41f1f076ca..45d477dcf4 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2212,14 +2212,14 @@ static int ifilter_send_frame(InputFilter *ifilter, 
AVFrame *frame)
 return 0;
 }
 
-static int ifilter_send_eof(InputFilter *ifilter)
+static int ifilter_send_eof(InputFilter *ifilter, int64_t pts)
 {
 int i, j, ret;
 
 ifilter->eof = 1;
 
 if (ifilter->filter) {
-ret = av_buffersrc_add_frame_flags(ifilter->filter, NULL, 
AV_BUFFERSRC_FLAG_PUSH);
+ret = av_buffersrc_close(ifilter->filter, pts, AV_BUFFERSRC_FLAG_PUSH);
 if (ret < 0)
 return ret;
 } else {
@@ -2570,8 +2570,12 @@ out:
 static int send_filter_eof(InputStream *ist)
 {
 int i, ret;
+/* TODO keep pts also in stream time base to avoid converting back */
+int64_t pts = av_rescale_q_rnd(ist->pts, AV_TIME_BASE_Q, 
ist->st->time_base,
+   AV_ROUND_NEAR_INF | AV_ROUND_PASS_MINMAX);
+
 for (i = 0; i < ist->nb_filters; i++) {
-ret = ifilter_send_eof(ist->filters[i]);
+ret = ifilter_send_eof(ist->filters[i], pts);
 if (ret < 0)
 return ret;
 }
-- 
2.11.0

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

[FFmpeg-devel] [PATCH 3/4] lavfi/buffersrc: add av_buffersrc_close().

[Nicolas George]

TODO APIChanges and minor bump.

Signed-off-by: Nicolas George 
---
 libavfilter/buffersrc.c | 22 --
 libavfilter/buffersrc.h |  8 
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/libavfilter/buffersrc.c b/libavfilter/buffersrc.c
index 3f80d5f413..250b06dfbc 100644
--- a/libavfilter/buffersrc.c
+++ b/libavfilter/buffersrc.c
@@ -196,16 +196,9 @@ static int av_buffersrc_add_frame_internal(AVFilterContext 
*ctx,
 
 s->nb_failed_requests = 0;
 
-if (!frame) {
-s->eof = 1;
-ff_avfilter_link_set_in_status(ctx->outputs[0], AVERROR_EOF, 
AV_NOPTS_VALUE);
-if ((flags & AV_BUFFERSRC_FLAG_PUSH)) {
-ret = push_frame(ctx->graph);
-if (ret < 0)
-return ret;
-}
-return 0;
-} else if (s->eof)
+if (!frame)
+return av_buffersrc_close(ctx, AV_NOPTS_VALUE, flags);
+if (s->eof)
 return AVERROR(EINVAL);
 
 refcounted = !!frame->buf[0];
@@ -267,6 +260,15 @@ static int av_buffersrc_add_frame_internal(AVFilterContext 
*ctx,
 return 0;
 }
 
+int av_buffersrc_close(AVFilterContext *ctx, int64_t pts, unsigned flags)
+{
+BufferSourceContext *s = ctx->priv;
+
+s->eof = 1;
+ff_avfilter_link_set_in_status(ctx->outputs[0], AVERROR_EOF, pts);
+return (flags & AV_BUFFERSRC_FLAG_PUSH) ? push_frame(ctx->graph) : 0;
+}
+
 static av_cold int init_video(AVFilterContext *ctx)
 {
 BufferSourceContext *c = ctx->priv;
diff --git a/libavfilter/buffersrc.h b/libavfilter/buffersrc.h
index e42c78196b..0652113f2b 100644
--- a/libavfilter/buffersrc.h
+++ b/libavfilter/buffersrc.h
@@ -193,6 +193,14 @@ av_warn_unused_result
 int av_buffersrc_add_frame_flags(AVFilterContext *buffer_src,
  AVFrame *frame, int flags);
 
+/**
+ * Close the buffer source after EOF.
+ *
+ * This is similar to passing NULL to av_buffersrc_add_frame_flags()
+ * except it takes the timestamp of the EOF, i.e. the timestamp of the end
+ * of the last frame.
+ */
+int av_buffersrc_close(AVFilterContext *ctx, int64_t pts, unsigned flags);
 
 /**
  * @}
-- 
2.11.0

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

[FFmpeg-devel] [PATCH 2/4] ffmpeg: use reordered duration for stream PTS.

[Nicolas George]

Signed-off-by: Nicolas George 
---
 ffmpeg.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)


Unchanged. Necessary for the next patches.


diff --git a/ffmpeg.c b/ffmpeg.c
index 4db8ea82ac..41f1f076ca 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2357,7 +2357,7 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, 
int *got_output,
 return err < 0 ? err : ret;
 }
 
-static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int 
eof,
+static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int 
*duration_pts, int eof,
 int *decode_failed)
 {
 AVFrame *decoded_frame;
@@ -2448,6 +2448,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, 
int *got_output, int eo
 ist->hwaccel_retrieved_pix_fmt = decoded_frame->format;
 
 best_effort_timestamp= av_frame_get_best_effort_timestamp(decoded_frame);
+*duration_pts = decoded_frame->pkt_duration;
 
 if (ist->framerate.num)
 best_effort_timestamp = ist->cfr_next_pts++;
@@ -2618,6 +2619,7 @@ static int process_input_packet(InputStream *ist, const 
AVPacket *pkt, int no_eo
 // while we have more to decode or while the decoder did output something 
on EOF
 while (ist->decoding_needed) {
 int duration_dts = 0;
+int duration_pts = 0;
 int got_output = 0;
 int decode_failed = 0;
 
@@ -2630,7 +2632,7 @@ static int process_input_packet(InputStream *ist, const 
AVPacket *pkt, int no_eo
_failed);
 break;
 case AVMEDIA_TYPE_VIDEO:
-ret = decode_video(ist, repeating ? NULL : , 
_output, !pkt,
+ret = decode_video(ist, repeating ? NULL : , 
_output, _pts, !pkt,
_failed);
 if (!repeating || !pkt || got_output) {
 if (pkt && pkt->duration) {
@@ -2649,7 +2651,7 @@ static int process_input_packet(InputStream *ist, const 
AVPacket *pkt, int no_eo
 }
 
 if (got_output)
-ist->next_pts += duration_dts; //FIXME the duration is not 
correct in some cases
+ist->next_pts += av_rescale_q(duration_pts, 
ist->st->time_base, AV_TIME_BASE_Q);
 break;
 case AVMEDIA_TYPE_SUBTITLE:
 if (repeating)
-- 
2.11.0

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

[FFmpeg-devel] [PATCH 1/4] ffmpeg: rename a variable.

[Nicolas George]

Makes the reason of the "FIXME" comment more obvious.
Avoid name conflicts for the next commit.

Signed-off-by: Nicolas George 
---
 ffmpeg.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)


Unchanged.


diff --git a/ffmpeg.c b/ffmpeg.c
index ea03179c21..4db8ea82ac 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2617,7 +2617,7 @@ static int process_input_packet(InputStream *ist, const 
AVPacket *pkt, int no_eo
 
 // while we have more to decode or while the decoder did output something 
on EOF
 while (ist->decoding_needed) {
-int duration = 0;
+int duration_dts = 0;
 int got_output = 0;
 int decode_failed = 0;
 
@@ -2634,22 +2634,22 @@ static int process_input_packet(InputStream *ist, const 
AVPacket *pkt, int no_eo
_failed);
 if (!repeating || !pkt || got_output) {
 if (pkt && pkt->duration) {
-duration = av_rescale_q(pkt->duration, ist->st->time_base, 
AV_TIME_BASE_Q);
+duration_dts = av_rescale_q(pkt->duration, 
ist->st->time_base, AV_TIME_BASE_Q);
 } else if(ist->dec_ctx->framerate.num != 0 && 
ist->dec_ctx->framerate.den != 0) {
 int ticks= av_stream_get_parser(ist->st) ? 
av_stream_get_parser(ist->st)->repeat_pict+1 : ist->dec_ctx->ticks_per_frame;
-duration = ((int64_t)AV_TIME_BASE *
+duration_dts = ((int64_t)AV_TIME_BASE *
 ist->dec_ctx->framerate.den * ticks) /
 ist->dec_ctx->framerate.num / 
ist->dec_ctx->ticks_per_frame;
 }
 
-if(ist->dts != AV_NOPTS_VALUE && duration) {
-ist->next_dts += duration;
+if(ist->dts != AV_NOPTS_VALUE && duration_dts) {
+ist->next_dts += duration_dts;
 }else
 ist->next_dts = AV_NOPTS_VALUE;
 }
 
 if (got_output)
-ist->next_pts += duration; //FIXME the duration is not correct 
in some cases
+ist->next_pts += duration_dts; //FIXME the duration is not 
correct in some cases
 break;
 case AVMEDIA_TYPE_SUBTITLE:
 if (repeating)
-- 
2.11.0

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

Re: [FFmpeg-devel] [PATCH] vf_framestep: add blend parameter for motion blur effect

[Nicolas George]

Le sextidi 16 germinal, an CCXXV, Matthias C. M. Troffaes a écrit :
> +#define DEFINE_BLEND(NAME, TYPE, DECL, EXPR) 
>   \
> +static void blend_##NAME##_##TYPE(AVFilterContext *ctx, AVFrame *in, int 
> plane)\
> +{
>   \
> +FrameStepContext *s = ctx->priv; 
>   \
> +DECL 
>   \
> +const int height = s->planeheight[plane];
>   \
> +const int width  = s->planewidth[plane]; 
>   \
> +const int stride = in->linesize[plane] / sizeof(TYPE);   
>   \
> +TYPE *src = (TYPE *)in->data[plane]; 
>   \
> +uint32_t *dst = s->data[plane];  
>   \
> +int y, x;
>   \
> + 
>   \
> +for (y = 0; y < height; y++) {   
>   \
> +for (x = 0; x < width; x++) {
>   \
> +EXPR;
>   \
> +}
>   \
> +src += stride;   
>   \
> +}
>   \
> +}
> +
> +#define SET_DECL
> +#define SET_EXPR *dst++ = src[x]
> +#define ADD_DECL
> +#define ADD_EXPR *dst++ += src[x]
> +#define DIV_DECL const int frame_blend = s->frame_blend;
> +#define DIV_EXPR src[x] = *dst++ / frame_blend
> +
> +DEFINE_BLEND(set, uint8_t,  SET_DECL, SET_EXPR)
> +DEFINE_BLEND(set, uint16_t, SET_DECL, SET_EXPR)
> +DEFINE_BLEND(add, uint8_t,  ADD_DECL, ADD_EXPR)
> +DEFINE_BLEND(add, uint16_t, ADD_DECL, ADD_EXPR)
> +DEFINE_BLEND(div, uint8_t,  DIV_DECL, DIV_EXPR)
> +DEFINE_BLEND(div, uint16_t, DIV_DECL, DIV_EXPR)

There is already blending code in vf_framerate and vf_overlay, I think
we should not add new similar code without sharing it as much as
possible.

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH] avfilter/vf_framerate: always request input if no output is provided in request_frame

[Nicolas George]

Le sextidi 16 germinal, an CCXXV, Marton Balint a écrit :
> Fixes ticket #6285.
> 
> Signed-off-by: Marton Balint 
> ---
>  libavfilter/vf_framerate.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

It respects the scheduling requirements, and the logic seems sound from
what I understand of framerate logic, so I guess it is ok.

Thanks for taking care of it.

Regards,

-- 
  Nicolas George


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