Re: [FFmpeg-devel] [PATCH, v3] lavc/hevc_parser: cope with more leading_zero_8bits and update FATE

2018-12-25 Thread Fu, Linjie
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Hendrik Leppkes
> Sent: Wednesday, December 26, 2018 09:15
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, v3] lavc/hevc_parser: cope with more
> leading_zero_8bits and update FATE
> 
> On Wed, Dec 26, 2018 at 12:42 AM Michael Niedermayer
>  wrote:
> >
> > On Mon, Dec 10, 2018 at 06:23:54PM +0800, Linjie Fu wrote:
> > > Given that leading_zero_8bits can be included many times at the
> beginning
> > > of a NAL unit, blindly taking the startcode as 3 bytes will leave 0x00
> > > in last frame and may lead to some warnings in parse_nal_units when s-
> >flags
> > > is set to PARSER_FLAG_COMPLETE_FRAMES.
> > >
> > > Modify to put all of the zeroes between two frames into the second
> frame.
> > > Modify the code to print the buf_size like in H264 and reduce the
> duplication.
> > >
> > > Update the FATE checksum for fate-hevc-bsf-mp4toannexb:
> > > The ref output has redundant 0x00 at the end of the SUFFIX_SEI:
> > > { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41
> > > c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1
> > > 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 }
> > >
> > > To verify the output of FATE:
> > > ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -
> flags \
> > > +bitexact hevc-mp4.mov
> > > ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out
> > >
> > > Signed-off-by: Linjie Fu 
> > > ---
> > > [v2]: Update FATE checksum.
> > > [v3]: Cope with more leading_zero_8bits cases.
> > >
> > >  libavcodec/hevc_parser.c | 14 +-
> > >  tests/fate/hevc.mak  |  2 +-
> > >  2 files changed, 10 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> > > index 369d1338d0..62100ac654 100644
> > > --- a/libavcodec/hevc_parser.c
> > > +++ b/libavcodec/hevc_parser.c
> > > @@ -239,7 +239,7 @@ static int parse_nal_units(AVCodecParserContext
> *s, const uint8_t *buf,
> > >  }
> > >  }
> > >  /* didn't find a picture! */
> > > -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n");
> > > +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with
> size %d\n", buf_size);
> > >  return -1;
> >
> > this should ne in a seperate patch
> >
> >
> > >  }
> > >
> > > @@ -267,8 +267,7 @@ static int
> hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf,
> > >  if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut
> == HEVC_NAL_SEI_PREFIX ||
> > >  (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) {
> > >  if (pc->frame_start_found) {
> > > -pc->frame_start_found = 0;
> > > -return i - 5;
> > > +goto found;
> > >  }
> > >  } else if (nut <= HEVC_NAL_RASL_R ||
> > > (nut >= HEVC_NAL_BLA_W_LP && nut <=
> HEVC_NAL_CRA_NUT)) {
> > > @@ -277,14 +276,19 @@ static int
> hevc_find_frame_end(AVCodecParserContext *s, const uint8_t *buf,
> > >  if (!pc->frame_start_found) {
> > >  pc->frame_start_found = 1;
> > >  } else { // First slice of next frame found
> > > -pc->frame_start_found = 0;
> > > -return i - 5;
> > > +goto found;
> > >  }
> > >  }
> > >  }
> > >  }
> > >
> > >  return END_NOT_FOUND;
> > > +
> > > +found:
> > > +pc->frame_start_found = 0;
> > > +while (i - 6 >= 0 && !buf[i - 6])
> > > +i--;
> > > +return i - 5;
> >
> > I think this is incorrect
> > a parser should produce the same output independant on how the input is
> cut
> > into bytsstream parts. But this would behave different depending on if the
> > last 0 bytes are still in the current buffer
> >
> 
> Isnt it the parsers job to do just this cutting, or correct badly cut packets?
> 
> - Hendrik

Paser will cache the bit stream in pc->buffer if END_NOT_FOUND in current buf.
I think Michael's concern was some 0 bytes were stored in pc->buffer, and this 
patch could 
only cope with the 0 bytes in current buf but leave those 0 bytes stored in 
pc->buffer.

If so,  is it  make sense to find all the leading 0 bytes in pc->buffer and 
change the index to cut 
correctly after finding the frame end?

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


Re: [FFmpeg-devel] [PATCH, v3] lavc/hevc_parser: cope with more leading_zero_8bits and update FATE

2018-12-25 Thread Hendrik Leppkes
On Wed, Dec 26, 2018 at 12:42 AM Michael Niedermayer
 wrote:
>
> On Mon, Dec 10, 2018 at 06:23:54PM +0800, Linjie Fu wrote:
> > Given that leading_zero_8bits can be included many times at the beginning
> > of a NAL unit, blindly taking the startcode as 3 bytes will leave 0x00
> > in last frame and may lead to some warnings in parse_nal_units when s->flags
> > is set to PARSER_FLAG_COMPLETE_FRAMES.
> >
> > Modify to put all of the zeroes between two frames into the second frame.
> > Modify the code to print the buf_size like in H264 and reduce the 
> > duplication.
> >
> > Update the FATE checksum for fate-hevc-bsf-mp4toannexb:
> > The ref output has redundant 0x00 at the end of the SUFFIX_SEI:
> > { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41
> > c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1
> > 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 }
> >
> > To verify the output of FATE:
> > ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags \
> > +bitexact hevc-mp4.mov
> > ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out
> >
> > Signed-off-by: Linjie Fu 
> > ---
> > [v2]: Update FATE checksum.
> > [v3]: Cope with more leading_zero_8bits cases.
> >
> >  libavcodec/hevc_parser.c | 14 +-
> >  tests/fate/hevc.mak  |  2 +-
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> > index 369d1338d0..62100ac654 100644
> > --- a/libavcodec/hevc_parser.c
> > +++ b/libavcodec/hevc_parser.c
> > @@ -239,7 +239,7 @@ static int parse_nal_units(AVCodecParserContext *s, 
> > const uint8_t *buf,
> >  }
> >  }
> >  /* didn't find a picture! */
> > -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n");
> > +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with size 
> > %d\n", buf_size);
> >  return -1;
>
> this should ne in a seperate patch
>
>
> >  }
> >
> > @@ -267,8 +267,7 @@ static int hevc_find_frame_end(AVCodecParserContext *s, 
> > const uint8_t *buf,
> >  if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == 
> > HEVC_NAL_SEI_PREFIX ||
> >  (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) {
> >  if (pc->frame_start_found) {
> > -pc->frame_start_found = 0;
> > -return i - 5;
> > +goto found;
> >  }
> >  } else if (nut <= HEVC_NAL_RASL_R ||
> > (nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) {
> > @@ -277,14 +276,19 @@ static int hevc_find_frame_end(AVCodecParserContext 
> > *s, const uint8_t *buf,
> >  if (!pc->frame_start_found) {
> >  pc->frame_start_found = 1;
> >  } else { // First slice of next frame found
> > -pc->frame_start_found = 0;
> > -return i - 5;
> > +goto found;
> >  }
> >  }
> >  }
> >  }
> >
> >  return END_NOT_FOUND;
> > +
> > +found:
> > +pc->frame_start_found = 0;
> > +while (i - 6 >= 0 && !buf[i - 6])
> > +i--;
> > +return i - 5;
>
> I think this is incorrect
> a parser should produce the same output independant on how the input is cut
> into bytsstream parts. But this would behave different depending on if the
> last 0 bytes are still in the current buffer
>

Isnt it the parsers job to do just this cutting, or correct badly cut packets?

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


Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

2018-12-25 Thread James Almer
On 12/25/2018 7:15 PM, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 
> 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> Before: Executed 
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>  in 11294 ms
> After : Executed 
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
>  in 4249 ms
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavutil/imgutils.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
> index 4938a7ef67..cc38f1e878 100644
> --- a/libavutil/imgutils.c
> +++ b/libavutil/imgutils.c
> @@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t dst_size, 
> uint8_t *clear,
>  }
>  } else if (clear_size == 4) {
>  uint32_t val = AV_RN32(clear);
> +uint64_t val8 = val * 0x10001ULL;
> +for (; dst_size >= 32; dst_size -= 32) {
> +AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
> +AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
> +dst += 32;
> +}

This should be wrapped with a HAVE_FAST_64BIT preprocessor check.

Also, is it much slower if you also write one per loop like everywhere
else in the function? I'd prefer if things are consistent.
Similarly, you could add four and eight bytes loops to the clear_size ==
2 case above.

>  for (; dst_size >= 4; dst_size -= 4) {
>  AV_WN32(dst, val);
>  dst += 4;
> 

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


Re: [FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

2018-12-25 Thread Kieran Kunhya
On Tue, 25 Dec 2018 at 22:17 Michael Niedermayer 
wrote:

> Fixes: Timeout
> Fixes:
> 11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> Before: Executed
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> in 11294 ms
> After : Executed
> clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
> in 4249 ms
>

So basically you go from one arbitrary timeout to another. I assume your
fuzzer timeout is 5 or 10 seconds and so you manage to make it pass?

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


Re: [FFmpeg-devel] [PATCH, v3] lavc/hevc_parser: cope with more leading_zero_8bits and update FATE

2018-12-25 Thread Michael Niedermayer
On Mon, Dec 10, 2018 at 06:23:54PM +0800, Linjie Fu wrote:
> Given that leading_zero_8bits can be included many times at the beginning
> of a NAL unit, blindly taking the startcode as 3 bytes will leave 0x00
> in last frame and may lead to some warnings in parse_nal_units when s->flags
> is set to PARSER_FLAG_COMPLETE_FRAMES.
> 
> Modify to put all of the zeroes between two frames into the second frame.
> Modify the code to print the buf_size like in H264 and reduce the duplication.
> 
> Update the FATE checksum for fate-hevc-bsf-mp4toannexb:
> The ref output has redundant 0x00 at the end of the SUFFIX_SEI:
> { 50 01 84 31 00 19 39 77 d0 7b 3f bd 1e 09 38 9a 79 41
> c0 16 11 da 18 aa 0a db 2b 19 fa 47 3f 0f 67 4a 91 9c a1
> 12 72 67 d6 f0 8f 66 ee 95 f9 e2 b9 ba 23 9a e8 80 00 }
> 
> To verify the output of FATE:
> ffmpeg -i hevc-conformance/WPP_A_ericsson_MAIN10_2.bit -c copy -flags \
> +bitexact hevc-mp4.mov
> ffmpeg -i hevc-mp4.mov -c:v copy -fflags +bitexact -f hevc hevc.out
> 
> Signed-off-by: Linjie Fu 
> ---
> [v2]: Update FATE checksum.
> [v3]: Cope with more leading_zero_8bits cases.
> 
>  libavcodec/hevc_parser.c | 14 +-
>  tests/fate/hevc.mak  |  2 +-
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/hevc_parser.c b/libavcodec/hevc_parser.c
> index 369d1338d0..62100ac654 100644
> --- a/libavcodec/hevc_parser.c
> +++ b/libavcodec/hevc_parser.c
> @@ -239,7 +239,7 @@ static int parse_nal_units(AVCodecParserContext *s, const 
> uint8_t *buf,
>  }
>  }
>  /* didn't find a picture! */
> -av_log(avctx, AV_LOG_ERROR, "missing picture in access unit\n");
> +av_log(avctx, AV_LOG_ERROR, "missing picture in access unit with size 
> %d\n", buf_size);
>  return -1;

this should ne in a seperate patch


>  }
>  
> @@ -267,8 +267,7 @@ static int hevc_find_frame_end(AVCodecParserContext *s, 
> const uint8_t *buf,
>  if ((nut >= HEVC_NAL_VPS && nut <= HEVC_NAL_EOB_NUT) || nut == 
> HEVC_NAL_SEI_PREFIX ||
>  (nut >= 41 && nut <= 44) || (nut >= 48 && nut <= 55)) {
>  if (pc->frame_start_found) {
> -pc->frame_start_found = 0;
> -return i - 5;
> +goto found;
>  }
>  } else if (nut <= HEVC_NAL_RASL_R ||
> (nut >= HEVC_NAL_BLA_W_LP && nut <= HEVC_NAL_CRA_NUT)) {
> @@ -277,14 +276,19 @@ static int hevc_find_frame_end(AVCodecParserContext *s, 
> const uint8_t *buf,
>  if (!pc->frame_start_found) {
>  pc->frame_start_found = 1;
>  } else { // First slice of next frame found
> -pc->frame_start_found = 0;
> -return i - 5;
> +goto found;
>  }
>  }
>  }
>  }
>  
>  return END_NOT_FOUND;
> +
> +found:
> +pc->frame_start_found = 0;
> +while (i - 6 >= 0 && !buf[i - 6])
> +i--;
> +return i - 5;

I think this is incorrect
a parser should produce the same output independant on how the input is cut
into bytsstream parts. But this would behave different depending on if the 
last 0 bytes are still in the current buffer

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

"Nothing to hide" only works if the folks in power share the values of
you and everyone you know entirely and always will -- Tom Scott



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


[FFmpeg-devel] [PATCH 1/3] avutil/imgutils: Optimize writing 4 bytes in memset_bytes()

2018-12-25 Thread Michael Niedermayer
Fixes: Timeout
Fixes: 
11502/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920
Before: Executed 
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920 
in 11294 ms
After : Executed 
clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WCMV_fuzzer-5664893810769920 
in 4249 ms

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavutil/imgutils.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavutil/imgutils.c b/libavutil/imgutils.c
index 4938a7ef67..cc38f1e878 100644
--- a/libavutil/imgutils.c
+++ b/libavutil/imgutils.c
@@ -529,6 +529,12 @@ static void memset_bytes(uint8_t *dst, size_t dst_size, 
uint8_t *clear,
 }
 } else if (clear_size == 4) {
 uint32_t val = AV_RN32(clear);
+uint64_t val8 = val * 0x10001ULL;
+for (; dst_size >= 32; dst_size -= 32) {
+AV_WN64(dst   , val8); AV_WN64(dst+ 8, val8);
+AV_WN64(dst+16, val8); AV_WN64(dst+24, val8);
+dst += 32;
+}
 for (; dst_size >= 4; dst_size -= 4) {
 AV_WN32(dst, val);
 dst += 4;
-- 
2.20.1

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


[FFmpeg-devel] [PATCH 3/3] avcodec/exr: set layer_match in all branches

2018-12-25 Thread Michael Niedermayer
Otherwise it is left to the value from the previous iteration

Signed-off-by: Michael Niedermayer 
---
 libavcodec/exr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 13755e1e6e..0f8b0fda9f 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1389,6 +1389,7 @@ static int decode_header(EXRContext *s, AVFrame *frame)
 if (*ch_gb.buffer == '.')
 ch_gb.buffer++; /* skip dot if not given */
 } else {
+layer_match = 0;
 av_log(s->avctx, AV_LOG_INFO,
"Channel doesn't match layer : %s.\n", 
ch_gb.buffer);
 }
-- 
2.20.1

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


[FFmpeg-devel] [PATCH 2/3] avcodec/exr: Check for duplicate channel index

2018-12-25 Thread Michael Niedermayer
Fixes: Out of memory
Fixes: 
11582/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_EXR_fuzzer-5730204559867904

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/exr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index 5253cc3f13..13755e1e6e 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -1463,6 +1463,11 @@ static int decode_header(EXRContext *s, AVFrame *frame)
 }
 s->pixel_type = current_pixel_type;
 s->channel_offsets[channel_index] = 
s->current_channel_offset;
+} else if (channel_index >= 0) {
+av_log(s->avctx, AV_LOG_ERROR,
+"Multiple channels with index %d.\n", 
channel_index);
+ret = AVERROR_INVALIDDATA;
+goto fail;
 }
 
 s->channels = av_realloc(s->channels,
-- 
2.20.1

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


[FFmpeg-devel] [PATCH] avfilter/af_asetnsamples: fix last frame props

2018-12-25 Thread Marton Balint
Frame properties were not copied, so e.g. PTS was not set for the last frame.

Regression since ef3babb2c70f564dc1634b3f29c6e35a2b2dc239.

Signed-off-by: Marton Balint 
---
 libavfilter/af_asetnsamples.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavfilter/af_asetnsamples.c b/libavfilter/af_asetnsamples.c
index e8daec8d8f..ea20d66ac5 100644
--- a/libavfilter/af_asetnsamples.c
+++ b/libavfilter/af_asetnsamples.c
@@ -76,6 +76,12 @@ static int activate(AVFilterContext *ctx)
 return AVERROR(ENOMEM);
 }
 
+ret = av_frame_copy_props(pad_frame, frame);
+if (ret < 0) {
+av_frame_free();
+return ret;
+}
+
 av_samples_copy(pad_frame->extended_data, frame->extended_data,
 0, 0, frame->nb_samples, frame->channels, 
frame->format);
 av_samples_set_silence(pad_frame->extended_data, frame->nb_samples,
-- 
2.16.4

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


[FFmpeg-devel] [PATCH 2/2] avcodec/realtime_bsf: add option to scale speed

2018-12-25 Thread Moritz Barsnick
Signed-off-by: Moritz Barsnick 
---
 doc/bitstream_filters.texi | 4 
 libavcodec/realtime_bsf.c  | 7 +--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index 8c71100460..860c555c3f 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -610,6 +610,10 @@ It accepts the following options:
 @item limit
 Time limit for the pauses. Any pause longer than that will be considered
 a timestamp discontinuity and reset the timer. Default is 2 seconds.
+@item speed
+Speed factor for processing. The value must be a float larger than zero.
+Values larger than 1.0 will speed up processing, smaller will slow it down.
+The @var{limit} is automatically adapted accordingly. Default is 1.0.
 @end table
 
 @section remove_extra
diff --git a/libavcodec/realtime_bsf.c b/libavcodec/realtime_bsf.c
index eb4b0b69ed..31a648f96c 100644
--- a/libavcodec/realtime_bsf.c
+++ b/libavcodec/realtime_bsf.c
@@ -24,11 +24,13 @@
 #include "libavutil/opt.h"
 #include "avcodec.h"
 #include "bsf.h"
+#include 
 
 typedef struct RealtimeContext {
 const AVClass *class;
 int64_t delta;
 int64_t limit;
+double speed;
 unsigned inited;
 } RealtimeContext;
 
@@ -42,7 +44,7 @@ static int realtime_filter(AVBSFContext *bsf, AVPacket *pkt)
 return ret;
 
 if (pkt->pts != AV_NOPTS_VALUE) {
-int64_t pts = av_rescale_q(pkt->pts, bsf->time_base_in, 
AV_TIME_BASE_Q);
+int64_t pts = av_rescale_q(pkt->pts, bsf->time_base_in, 
AV_TIME_BASE_Q) / ctx->speed;
 int64_t now = av_gettime_relative();
 int64_t sleep = pts - now + ctx->delta;
 if (!ctx->inited) {
@@ -50,7 +52,7 @@ static int realtime_filter(AVBSFContext *bsf, AVPacket *pkt)
 sleep = 0;
 ctx->delta = now - pts;
 }
-if (sleep > ctx->limit || sleep < -ctx->limit) {
+if (sleep > ctx->limit / ctx->speed || sleep < -ctx->limit / 
ctx->speed) {
 av_log(ctx, AV_LOG_WARNING,
"time discontinuity detected: %"PRIi64" us, resetting\n",
sleep);
@@ -72,6 +74,7 @@ static int realtime_filter(AVBSFContext *bsf, AVPacket *pkt)
 #define FLAGS (AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_BSF_PARAM)
 static const AVOption options[] = {
 { "limit", "sleep time limit", OFFSET(limit), AV_OPT_TYPE_DURATION, { .i64 
= 200 }, 0, INT64_MAX, FLAGS },
+{ "speed", "speed factor", OFFSET(speed), AV_OPT_TYPE_DOUBLE, { .dbl = 1.0 
}, DBL_MIN, DBL_MAX, FLAGS },
 { NULL },
 };
 
-- 
2.14.5

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


[FFmpeg-devel] [PATCH 1/2] avcodec: add realtime bitstream filter

2018-12-25 Thread Moritz Barsnick
Works for video and audio streams.

Similar to the -re option in ffmpeg that only works for input files,
and is only implemented for the command line tool, not available in
the libraries.

Implementation mostly taken from libavfilter/f_realtime.c.

Signed-off-by: Moritz Barsnick 
---
 doc/bitstream_filters.texi | 16 
 libavcodec/Makefile|  1 +
 libavcodec/bitstream_filters.c |  1 +
 libavcodec/realtime_bsf.c  | 90 ++
 4 files changed, 108 insertions(+)
 create mode 100644 libavcodec/realtime_bsf.c

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index b779265f58..8c71100460 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -596,6 +596,22 @@ Set Rec709 colorspace for each frame of the file
 ffmpeg -i INPUT -c copy -bsf:v 
prores_metadata=color_primaries=bt709:color_trc=bt709:colorspace=bt709 
output.mov
 @end example
 
+@section realtime
+
+Slow down output processing to match real time approximately.
+
+This filter will pause the filtering for a variable amount of time to
+match the output rate with the input timestamps.
+It is similar to the @option{re} option to @code{ffmpeg}.
+
+It accepts the following options:
+
+@table @option
+@item limit
+Time limit for the pauses. Any pause longer than that will be considered
+a timestamp discontinuity and reset the timer. Default is 2 seconds.
+@end table
+
 @section remove_extra
 
 Remove extradata from packets.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 08f89ae0b2..a85a5e1fa2 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1085,6 +1085,7 @@ OBJS-$(CONFIG_MPEG2_METADATA_BSF) += 
mpeg2_metadata_bsf.o
 OBJS-$(CONFIG_NOISE_BSF)  += noise_bsf.o
 OBJS-$(CONFIG_NULL_BSF)   += null_bsf.o
 OBJS-$(CONFIG_PRORES_METADATA_BSF)+= prores_metadata_bsf.o
+OBJS-$(CONFIG_REALTIME_BSF)   += realtime_bsf.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)   += remove_extradata_bsf.o
 OBJS-$(CONFIG_TEXT2MOVSUB_BSF)+= movsub_bsf.o
 OBJS-$(CONFIG_TRACE_HEADERS_BSF)  += trace_headers_bsf.o
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 2c999d3c1d..ba7d3224c4 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -48,6 +48,7 @@ extern const AVBitStreamFilter ff_mov2textsub_bsf;
 extern const AVBitStreamFilter ff_noise_bsf;
 extern const AVBitStreamFilter ff_null_bsf;
 extern const AVBitStreamFilter ff_prores_metadata_bsf;
+extern const AVBitStreamFilter ff_realtime_bsf;
 extern const AVBitStreamFilter ff_remove_extradata_bsf;
 extern const AVBitStreamFilter ff_text2movsub_bsf;
 extern const AVBitStreamFilter ff_trace_headers_bsf;
diff --git a/libavcodec/realtime_bsf.c b/libavcodec/realtime_bsf.c
new file mode 100644
index 00..eb4b0b69ed
--- /dev/null
+++ b/libavcodec/realtime_bsf.c
@@ -0,0 +1,90 @@
+/*
+ * Realtime filter
+ * Copyright (c) 2015 Nicolas George
+ * Copyright (c) 2018 Moritz Barsnick
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/time.h"
+#include "libavutil/opt.h"
+#include "avcodec.h"
+#include "bsf.h"
+
+typedef struct RealtimeContext {
+const AVClass *class;
+int64_t delta;
+int64_t limit;
+unsigned inited;
+} RealtimeContext;
+
+static int realtime_filter(AVBSFContext *bsf, AVPacket *pkt)
+{
+int ret;
+RealtimeContext *ctx = bsf->priv_data;
+
+ret = ff_bsf_get_packet_ref(bsf, pkt);
+if (ret < 0)
+return ret;
+
+if (pkt->pts != AV_NOPTS_VALUE) {
+int64_t pts = av_rescale_q(pkt->pts, bsf->time_base_in, 
AV_TIME_BASE_Q);
+int64_t now = av_gettime_relative();
+int64_t sleep = pts - now + ctx->delta;
+if (!ctx->inited) {
+ctx->inited = 1;
+sleep = 0;
+ctx->delta = now - pts;
+}
+if (sleep > ctx->limit || sleep < -ctx->limit) {
+av_log(ctx, AV_LOG_WARNING,
+   "time discontinuity detected: %"PRIi64" us, resetting\n",
+   sleep);
+sleep = 0;
+ctx->delta = now - pts;
+}
+if (sleep > 0) {
+av_log(ctx, AV_LOG_DEBUG, "sleeping 

[FFmpeg-devel] [PATCH 0/2] avcodec: add realtime bitstream filter

2018-12-25 Thread Moritz Barsnick
The realtime bitstream filter is modeled exactly along the
realtime/arealtime filters, but works on bitstream level, allowing to use
them with the copy codec.

It can be considered a proof of concept, as ffmpeg offers the "-re" option
to achieve this, but:

- the "-re" option is not available in the libraries, and has to be
  remodeled by other applications wishing to achieve that effect;
- the second patch in this series allows to scale the speed, which the "-re"
  option does not offer.

Since the core implementation is a blatant copy of f_realtime, its author
Nicolas is credited here.

Moritz Barsnick (2):
  avcodec: add realtime bitstream filter
  avcodec/realtime_bsf: add option to scale speed

 doc/bitstream_filters.texi | 20 +
 libavcodec/Makefile|  1 +
 libavcodec/bitstream_filters.c |  1 +
 libavcodec/realtime_bsf.c  | 93 ++
 4 files changed, 115 insertions(+)
 create mode 100644 libavcodec/realtime_bsf.c

-- 
2.14.5

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


[FFmpeg-devel] [PATCH] avfilter/f_realtime: add option to scale speed

2018-12-25 Thread Moritz Barsnick
Signed-off-by: Moritz Barsnick 
---
This adds two double precision divisions per frame, which seems acceptable.

I'm not sure scaling the limit by the factor is the correct idea.  It feels
right regarding the discontinuities, but not according to the limit option's
description.


 doc/filters.texi | 4 
 libavfilter/f_realtime.c | 7 +--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 65ce25bc18..60ebf2aa99 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -20829,6 +20829,10 @@ They accept the following options:
 @item limit
 Time limit for the pauses. Any pause longer than that will be considered
 a timestamp discontinuity and reset the timer. Default is 2 seconds.
+@item speed
+Speed factor for processing. The value must be a float larger than zero.
+Values larger than 1.0 will speed up processing, smaller will slow it down.
+The @var{limit} is automatically adapted accordingly. Default is 1.0.
 @end table
 
 @anchor{select}
diff --git a/libavfilter/f_realtime.c b/libavfilter/f_realtime.c
index 171c16..8d4fbf642b 100644
--- a/libavfilter/f_realtime.c
+++ b/libavfilter/f_realtime.c
@@ -22,11 +22,13 @@
 #include "libavutil/time.h"
 #include "avfilter.h"
 #include "internal.h"
+#include 
 
 typedef struct RealtimeContext {
 const AVClass *class;
 int64_t delta;
 int64_t limit;
+double speed;
 unsigned inited;
 } RealtimeContext;
 
@@ -36,7 +38,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 RealtimeContext *s = ctx->priv;
 
 if (frame->pts != AV_NOPTS_VALUE) {
-int64_t pts = av_rescale_q(frame->pts, inlink->time_base, 
AV_TIME_BASE_Q);
+int64_t pts = av_rescale_q(frame->pts, inlink->time_base, 
AV_TIME_BASE_Q) / s->speed;
 int64_t now = av_gettime_relative();
 int64_t sleep = pts - now + s->delta;
 if (!s->inited) {
@@ -44,7 +46,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 sleep = 0;
 s->delta = now - pts;
 }
-if (sleep > s->limit || sleep < -s->limit) {
+if (sleep > s->limit / s->speed || sleep < -s->limit / s->speed) {
 av_log(ctx, AV_LOG_WARNING,
"time discontinuity detected: %"PRIi64" us, resetting\n",
sleep);
@@ -65,6 +67,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame)
 #define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_AUDIO_PARAM | 
AV_OPT_FLAG_FILTERING_PARAM
 static const AVOption options[] = {
 { "limit", "sleep time limit", OFFSET(limit), AV_OPT_TYPE_DURATION, { .i64 
= 200 }, 0, INT64_MAX, FLAGS },
+{ "speed", "speed factor", OFFSET(speed), AV_OPT_TYPE_DOUBLE, { .dbl = 1.0 
}, DBL_MIN, DBL_MAX, FLAGS },
 { NULL }
 };
 
-- 
2.14.5

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


Re: [FFmpeg-devel] [PATCH 2/3] avformat/mxfdec: support subsecond precision of decoded timestamps

2018-12-25 Thread Marton Balint



On Tue, 25 Dec 2018, Tomas Härdin wrote:


sön 2018-12-23 klockan 01:12 +0100 skrev Marton Balint:

> Signed-off-by: Marton Balint 
---
 libavformat/mxfdec.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index d78f8ad2e4..0553adcb06 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -2556,12 +2556,14 @@ fail_and_free:
 static int64_t mxf_timestamp_to_int64(uint64_t timestamp)
 {
 struct tm time = { 0 };
+int msecs;
 time.tm_year = (timestamp >> 48) - 1900;
 time.tm_mon  = (timestamp >> 40 & 0xFF) - 1;
 time.tm_mday = (timestamp >> 32 & 0xFF);
 time.tm_hour = (timestamp >> 24 & 0xFF);
 time.tm_min  = (timestamp >> 16 & 0xFF);
 time.tm_sec  = (timestamp >> 8  & 0xFF);
+msecs= (timestamp & 0xFF) * 4;
 
 /* Clip values for legacy reasons. Maybe we should return error instead? */
 time.tm_mon  = av_clip(time.tm_mon,  0, 11);
@@ -2569,8 +2571,9 @@ static int64_t mxf_timestamp_to_int64(uint64_t timestamp)
 time.tm_hour = av_clip(time.tm_hour, 0, 23);
 time.tm_min  = av_clip(time.tm_min,  0, 59);
 time.tm_sec  = av_clip(time.tm_sec,  0, 59);
+msecs= av_clip(msecs, 0, 999);
 
-return (int64_t)av_timegm() * 100;
+return (int64_t)av_timegm() * 100 + msecs * 1000;


Looks OK


Thanks, pushed this and the other patch.



I kinda wonder how this and the muxer code could have ignored the
milliseconds. Did the old creation time metadata use seconds only?


Yes, it used to be in this format: "%Y-%m-%d %H:%M:%S".

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


Re: [FFmpeg-devel] [PATCH 1/3] avformat/mxfdec: replace obsolete comment

2018-12-25 Thread Marton Balint



On Sun, 23 Dec 2018, Paul B Mahol wrote:


On 12/23/18, Marton Balint  wrote:

We no longer use strftime directly but use av_timegm to get an int64_t
timestamp.

Signed-off-by: Marton Balint 
---
 libavformat/mxfdec.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)



LGTM


Thanks, pushed.

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


Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

2018-12-25 Thread Michael Niedermayer
On Tue, Dec 25, 2018 at 02:11:45PM +0100, Paul B Mahol wrote:
> On 12/25/18, Michael Niedermayer  wrote:
> > On Mon, Dec 24, 2018 at 06:34:30PM +0100, Paul B Mahol wrote:
> > [...]
> >> -static const char *get_channel_name(int channel_id)
> >> +const char *av_channel_name(enum AVChannel channel_id)
> >>  {
> >
> >>  if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
> >>  return NULL;
> >> +if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names))
> >> +return "?";
> >
> > this looks like a untended duplicate check
> >
> > [...]
> >
> >> +/**
> >> + * Check whether two channel layouts are semantically the same, i.e. the
> >> same
> >> + * channels are present on the same positions in both.
> >> + *
> >> + * If one of the channel layouts is AV_CHANNEL_ORDER_UNSPEC, while the
> >> other is
> >> + * not, they are considered to be unequal. If both are
> >> AV_CHANNEL_ORDER_UNSPEC,
> >> + * they are considered equal iff the channel counts are the same in
> >> both.
> >> + *
> >> + * @param chl input channel layout
> >> + * @param chl1 input channel layout
> >> + * @return 0 if chl and chl1 are equal, 1 if they are not equal. A
> >> negative
> >> + * AVERROR code if one or both are invalid.
> >> + */
> >> +int av_channel_layout_compare(const AVChannelLayout *chl, const
> >> AVChannelLayout *chl1);
> >
> > It could be usefull if this is a full compare function that allows
> > sorting/ordering
> 
> Which kind of sorting? 

it doesnt matter which sorting.
Having some allows some operations to be performed more efficient
than having none.
An example is merging 2 lists removing duplicates.
2 sorted lists can be merged in O(n) but if theres no way to sort
and just a equal / non equal check it requires O(n^2)


> That could be only added later when needed.

adding it later is fine but i think its better if the API
allows it.
This would not work with above as it requires "1 if they are not equal",
it could allow 1 or -1 (or just non zero) then it can be extended

thx

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

Whats the most studid thing your enemy could do ? Blow himself up
Whats the most studid thing you could do ? Give up your rights and
freedom because your enemy blew himself up.



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


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/lagarith: Optimize case with singleton probability distribution

2018-12-25 Thread Paul B Mahol
On 12/25/18, Paul B Mahol  wrote:
> On 12/25/18, Michael Niedermayer  wrote:
>> On Mon, Dec 24, 2018 at 11:54:31PM +, Kieran Kunhya wrote:
>>> >
>>> > commit 0ca7a8deeffd33e05ae15a447259b32b6678c727 (HEAD -> master)
>>> > Author: Michael Niedermayer 
>>> > Date:   Mon Dec 24 01:14:50 2018 +0100
>>> >
>>> > avcodec/lagarith: Optimize case with singleton probability
>>> > distribution
>>> >
>>> > Fixes: Timeout
>>> > Fixes:
>>> > 10554/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LAGARITH_fuzzer-5739938067251200
>>> >
>>> > In case of a Denial of Service attack, the attacker wants to
>>> > maximize
>>> > the load on the target
>>> > per byte transmitted from the attacker.
>>> > For such a DoS attack it is best for the attacker to setup the
>>> > probabilities so that the
>>> > arithmetic decoder does not advance in the bytestream that way the
>>> > attacker only needs to
>>> > transmit the initial bytes and header for an arbitrary large
>>> > frame.
>>> > This patch here optimizes this codepath and avoids executing the
>>> > arithmetic decoder more than
>>> > once. It thus reduces the load causes by this codepath on the
>>> > target.
>>> > We also could completely disallow this codepath but it appears
>>> > such
>>> > odd probability
>>> > distributions are not invalid.
>>> >
>>> > Found-by: continuous fuzzing process
>>> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> > Signed-off-by: Michael Niedermayer 
>>> >
>>>
>>> This is a nonsense argument, a user could send a frame that was
>>> x in dimensions, would have the same effect.
>>
>> This suggested attack would not work, a user wanting to minimize these
>> DoS issues would have set AVCodecContext.max_pixels which would block
>> this.
>>
>>
>>> The calling application should manage timeouts themselves in a sandbox
>>> or
>>> container or similar.
>>
>> Its always possible and also a very good idea to have a 2nd line of
>> defense
>> like a sandbox / VM, ... as you suggest here, I did and do agree here.
>> And also a 3rd line of defense, ...
>>
>> But this doesnt mean we should not attempt to fix or mitigate
>> security (or other) issues directly in the code.
>>
>> I think the point you are raising has been raised previously, so let me
>> argue a little broader here and not specific to just what you suggest.
>>
>> If you compare a native fix in the code with a fix by a timeout, a
>> fix by a timeout causes:
>> * The whole process to be killed, so any application using libavcodec
>>   would basically "crash" and would not neccessarily save its state,
>>   flush out buffers, write any trailers or do proper protocol shutdowns
>>   or save any unsafed data. This is a outcome that should be minimized
>> * Using a timeout as the main way to block DoS is difficult as there is
>>   often no good timeout value. Its not unexpected that a system may need
>> to
>>   support processing large videos taking several hours, thats a long
>>   time for a file of a hundread bytes in a DoS attack
>>   100bytes from an attacker could cause the same load as 10 bytes
>> from
>>   a user.
>> * Worst maybe is that a tight timeout likely makes the system more
>> vulnerable
>>   not less. because during an attack all the processes would likely slow
>>   down and real users would be pushed into the timeout even if the system
>>   without the user processes timeouting would still function correctly
>>
>> Iam sure there are more arguments but above are the ones that came to my
>> mind
>> ATM.
>>
>>>
>>> Merry Xmas.
>>
>> Merry Xmas as well!
>>
>> [...]
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Those who would give up essential Liberty, to purchase a little
>> temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
>>
>
> This is still missing numbers/statistics.
>

And comments explaining added code.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/lagarith: Optimize case with singleton probability distribution

2018-12-25 Thread Paul B Mahol
On 12/25/18, Michael Niedermayer  wrote:
> On Mon, Dec 24, 2018 at 11:54:31PM +, Kieran Kunhya wrote:
>> >
>> > commit 0ca7a8deeffd33e05ae15a447259b32b6678c727 (HEAD -> master)
>> > Author: Michael Niedermayer 
>> > Date:   Mon Dec 24 01:14:50 2018 +0100
>> >
>> > avcodec/lagarith: Optimize case with singleton probability
>> > distribution
>> >
>> > Fixes: Timeout
>> > Fixes:
>> > 10554/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LAGARITH_fuzzer-5739938067251200
>> >
>> > In case of a Denial of Service attack, the attacker wants to
>> > maximize
>> > the load on the target
>> > per byte transmitted from the attacker.
>> > For such a DoS attack it is best for the attacker to setup the
>> > probabilities so that the
>> > arithmetic decoder does not advance in the bytestream that way the
>> > attacker only needs to
>> > transmit the initial bytes and header for an arbitrary large frame.
>> > This patch here optimizes this codepath and avoids executing the
>> > arithmetic decoder more than
>> > once. It thus reduces the load causes by this codepath on the
>> > target.
>> > We also could completely disallow this codepath but it appears such
>> > odd probability
>> > distributions are not invalid.
>> >
>> > Found-by: continuous fuzzing process
>> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> > Signed-off-by: Michael Niedermayer 
>> >
>>
>> This is a nonsense argument, a user could send a frame that was
>> x in dimensions, would have the same effect.
>
> This suggested attack would not work, a user wanting to minimize these
> DoS issues would have set AVCodecContext.max_pixels which would block this.
>
>
>> The calling application should manage timeouts themselves in a sandbox or
>> container or similar.
>
> Its always possible and also a very good idea to have a 2nd line of defense
> like a sandbox / VM, ... as you suggest here, I did and do agree here.
> And also a 3rd line of defense, ...
>
> But this doesnt mean we should not attempt to fix or mitigate
> security (or other) issues directly in the code.
>
> I think the point you are raising has been raised previously, so let me
> argue a little broader here and not specific to just what you suggest.
>
> If you compare a native fix in the code with a fix by a timeout, a
> fix by a timeout causes:
> * The whole process to be killed, so any application using libavcodec
>   would basically "crash" and would not neccessarily save its state,
>   flush out buffers, write any trailers or do proper protocol shutdowns
>   or save any unsafed data. This is a outcome that should be minimized
> * Using a timeout as the main way to block DoS is difficult as there is
>   often no good timeout value. Its not unexpected that a system may need to
>   support processing large videos taking several hours, thats a long
>   time for a file of a hundread bytes in a DoS attack
>   100bytes from an attacker could cause the same load as 10 bytes
> from
>   a user.
> * Worst maybe is that a tight timeout likely makes the system more
> vulnerable
>   not less. because during an attack all the processes would likely slow
>   down and real users would be pushed into the timeout even if the system
>   without the user processes timeouting would still function correctly
>
> Iam sure there are more arguments but above are the ones that came to my
> mind
> ATM.
>
>>
>> Merry Xmas.
>
> Merry Xmas as well!
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Those who would give up essential Liberty, to purchase a little
> temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin
>

This is still missing numbers/statistics.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] Add a new channel layout API

2018-12-25 Thread Paul B Mahol
On 12/25/18, Michael Niedermayer  wrote:
> On Mon, Dec 24, 2018 at 06:34:30PM +0100, Paul B Mahol wrote:
> [...]
>> -static const char *get_channel_name(int channel_id)
>> +const char *av_channel_name(enum AVChannel channel_id)
>>  {
>
>>  if (channel_id < 0 || channel_id >= FF_ARRAY_ELEMS(channel_names))
>>  return NULL;
>> +if ((unsigned) channel_id >= FF_ARRAY_ELEMS(channel_names))
>> +return "?";
>
> this looks like a untended duplicate check
>
> [...]
>
>> +/**
>> + * Check whether two channel layouts are semantically the same, i.e. the
>> same
>> + * channels are present on the same positions in both.
>> + *
>> + * If one of the channel layouts is AV_CHANNEL_ORDER_UNSPEC, while the
>> other is
>> + * not, they are considered to be unequal. If both are
>> AV_CHANNEL_ORDER_UNSPEC,
>> + * they are considered equal iff the channel counts are the same in
>> both.
>> + *
>> + * @param chl input channel layout
>> + * @param chl1 input channel layout
>> + * @return 0 if chl and chl1 are equal, 1 if they are not equal. A
>> negative
>> + * AVERROR code if one or both are invalid.
>> + */
>> +int av_channel_layout_compare(const AVChannelLayout *chl, const
>> AVChannelLayout *chl1);
>
> It could be usefull if this is a full compare function that allows
> sorting/ordering

Which kind of sorting? That could be only added later when needed.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH V2 2/2] add an example to show how to fill the ROI info

2018-12-25 Thread Guo, Yejun
This patchset contains two patches.
- the first patch finished the code and ask for upstreaming.
- the second patch (this patch) is just a quick example and
  not ask for upstreaming.

to verify it, the command line looks like:

./ffmpeg -i .../path_to_1920x1080_video_file -vf scale=1920:1080 -c:v libx264 
-b:v 2000k -y tmp.264

Signed-off-by: Guo, Yejun 
---
 libavfilter/vf_scale.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index f741419..561391c 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -437,6 +437,20 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 return ret;
 }
 
+// just to show how a filter fills the ROI info
+size_t nb_rois = 1;
+AVFrameSideData *sd = av_frame_new_side_data(in, AV_FRAME_DATA_ROIS, 
nb_rois * sizeof(AVROI));
+if (!sd) {
+av_frame_free();
+return AVERROR(ENOMEM);
+}
+AVROI* rois = (AVROI*)sd->data;
+rois[0].top = 0;
+rois[0].left = 0;
+rois[0].bottom = in->height;
+rois[0].right = in->width/2;
+rois[0].qoffset = -15;
+
 if (!scale->sws)
 return ff_filter_frame(outlink, in);
 
-- 
2.7.4

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


[FFmpeg-devel] [PATCH V2 1/2] add support for ROI-based encoding

2018-12-25 Thread Guo, Yejun
This patchset contains two patches.
- the first patch (this patch) finished the code and ask for upstream.
- the second patch is just a quick example on how to generate ROI info.

The encoders such as libx264 support different QPs offset for different MBs,
it makes possible for ROI-based encoding. It makes sense to add support
within ffmpeg to generate/accept ROI infos and pass into encoders.

Typical usage: After AVFrame is decoded, a ffmpeg filter or user's code
generates ROI info for that frame, and the encoder finally does the
ROI-based encoding.

This patch just enabled the path from ffmpeg to libx264, the more encoders
can be added later.

Signed-off-by: Guo, Yejun 
---
 libavcodec/libx264.c | 40 
 libavutil/frame.c|  1 +
 libavutil/frame.h| 19 +++
 3 files changed, 60 insertions(+)

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index a68d0a7..a4f8677 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -40,6 +40,10 @@
 #include 
 #include 
 
+// from x264.h, for quant_offsets, Macroblocks are 16x16
+// blocks of pixels (with respect to the luma plane)
+#define MB_SIZE 16
+
 typedef struct X264Context {
 AVClass*class;
 x264_param_tparams;
@@ -345,6 +349,42 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
const AVFrame *frame,
 }
 }
 }
+
+AVFrameSideData *sd = av_frame_get_side_data(frame, 
AV_FRAME_DATA_ROIS);
+if (sd != NULL) {
+if (x4->params.rc.i_aq_mode == X264_AQ_NONE) {
+av_log(ctx, AV_LOG_WARNING, "Adaptive quantization must be 
enabled to use ROI encoding, skipping ROI.\n");
+} else {
+if (frame->interlaced_frame == 0) {
+size_t mbx = (frame->width + MB_SIZE - 1) / MB_SIZE;
+size_t mby = (frame->height + MB_SIZE - 1) / MB_SIZE;
+float* qoffsets;
+qoffsets = (float*)av_malloc(sizeof(*qoffsets) * mbx * 
mby);
+if (qoffsets == NULL)
+return AVERROR(ENOMEM);
+memset(qoffsets, 0, sizeof(*qoffsets) * mbx * mby);
+
+size_t nb_rois = sd->size / sizeof(AVROI);
+AVROI* rois = (AVROI*)sd->data;
+for (size_t roi = 0; roi < nb_rois; roi++) {
+int starty = FFMIN(mby, rois[roi].top / MB_SIZE);
+int endy = FFMIN(mby, (rois[roi].bottom + MB_SIZE - 
1)/ MB_SIZE);
+int startx = FFMIN(mbx, rois[roi].left / MB_SIZE);
+int endx = FFMIN(mbx, (rois[roi].right + MB_SIZE - 1)/ 
MB_SIZE);
+for (int y = starty; y < endy; y++) {
+for (int x = startx; x < endx; x++) {
+qoffsets[x + y*mbx] = rois[roi].qoffset;
+}
+}
+}
+
+x4->pic.prop.quant_offsets = qoffsets;
+x4->pic.prop.quant_offsets_free = av_free;
+} else {
+av_log(ctx, AV_LOG_WARNING, "interlaced_frame not 
supported for ROI encoding yet, skipping ROI.\n");
+}
+}
+}
 }
 
 do {
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 34a6210..bebc50e 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum 
AVFrameSideDataType type)
 case AV_FRAME_DATA_QP_TABLE_DATA:   return "QP table data";
 #endif
 case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata 
SMPTE2094-40 (HDR10+)";
+case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
 }
 return NULL;
 }
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 582ac47..d18d235 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -173,6 +173,12 @@ enum AVFrameSideDataType {
  * volume transform - application 4 of SMPTE 2094-40:2016 standard.
  */
 AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
+
+/**
+ * Regions Of Interest, the number of ROI area is implied
+ * in the size of buf.
+ */
+AV_FRAME_DATA_ROIS,
 };
 
 enum AVActiveFormatDescription {
@@ -200,6 +206,19 @@ typedef struct AVFrameSideData {
 AVBufferRef *buf;
 } AVFrameSideData;
 
+typedef struct AVROI {
+/* coordinates at frame pixel level.
+ * It will be extended internally if the codec requires an alignment.
+ * If the regions overlap, the last value in the list will be used.
+ */
+size_t top;
+size_t bottom;
+size_t left;
+size_t right;
+// quant offset is encoder dependent
+int qoffset;
+} AVROI;
+
 /**
  * This structure describes decoded (raw) audio or video data.
  *
-- 
2.7.4

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

Re: [FFmpeg-devel] [PATCH 1/2] lavfi/vf_hwmap: make hwunmap from software frame work.

2018-12-25 Thread Song, Ruiling


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Mark Thompson
> Sent: Friday, December 21, 2018 7:39 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavfi/vf_hwmap: make hwunmap from
> software frame work.
> 
> On 18/12/2018 01:28, Song, Ruiling wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of
> >> Mark Thompson
> >> Sent: Tuesday, December 18, 2018 6:33 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH 1/2] lavfi/vf_hwmap: make hwunmap
> from
> >> software frame work.
> >>
> >>  13/12/2018 01:50, Ruiling Song wrote:
> >>> This patch was used to fix the second hwmap filter issue:
> >>> [vaapi_frame] hwmap [software filters] hwmap [vaapi_frame]
> >>> For such case, we also need to allocate the hardware frame
> >>> and map it back to software.
> >>>
> >>> Signed-off-by: Ruiling Song 
> >>> ---
> >>>  libavfilter/vf_hwmap.c | 125 +-
> ---
> >> 
> >>>  1 file changed, 75 insertions(+), 50 deletions(-)
> >>>
> >>> diff --git a/libavfilter/vf_hwmap.c b/libavfilter/vf_hwmap.c
> >>> index 290559a..03cb325 100644
> >>> --- a/libavfilter/vf_hwmap.c
> >>> +++ b/libavfilter/vf_hwmap.c
> >>> @@ -50,6 +50,36 @@ static int hwmap_query_formats(AVFilterContext
> >> *avctx)
> >>>  return 0;
> >>>  }
> >>>
> >>> +static int create_hwframe_context(HWMapContext *ctx, AVFilterContext
> >> *avctx,
> >>> +  AVBufferRef *device, int format,
> >>> +  int sw_format, int width, int height)
> >>> +{
> >>> +int err;
> >>> +AVHWFramesContext *frames;
> >>> +
> >>> +ctx->hwframes_ref = av_hwframe_ctx_alloc(device);
> >>> +if (!ctx->hwframes_ref) {
> >>> +return AVERROR(ENOMEM);
> >>> +}
> >>> +frames = (AVHWFramesContext*)ctx->hwframes_ref->data;
> >>> +
> >>> +frames->format= format;
> >>> +frames->sw_format = sw_format;
> >>> +frames->width = width;
> >>> +frames->height= height;
> >>> +
> >>> +if (avctx->extra_hw_frames >= 0)
> >>> +frames->initial_pool_size = 2 + avctx->extra_hw_frames;
> >>> +
> >>> +err = av_hwframe_ctx_init(ctx->hwframes_ref);
> >>> +if (err < 0) {
> >>> +av_log(avctx, AV_LOG_ERROR, "Failed to initialise "
> >>> +   "target frames context: %d.\n", err);
> >>> +return err;
> >>> +}
> >>> +return 0;
> >>> +}
> >>> +
> >>>  static int hwmap_config_output(AVFilterLink *outlink)
> >>>  {
> >>>  AVFilterContext *avctx = outlink->src;
> >>> @@ -130,29 +160,11 @@ static int hwmap_config_output(AVFilterLink
> >> *outlink)
> >>>  // overwrite the input hwframe context with a derived context
> >>>  // mapped from that back to the source type.
> >>>  AVBufferRef *source;
> >>> -AVHWFramesContext *frames;
> >>> -
> >>> -ctx->hwframes_ref = av_hwframe_ctx_alloc(device);
> >>> -if (!ctx->hwframes_ref) {
> >>> -err = AVERROR(ENOMEM);
> >>> +err = create_hwframe_context(ctx, avctx, device, 
> >>> outlink->format,
> >>> + hwfc->sw_format, hwfc->width,
> >>> + hwfc->height);
> >>> +if (err < 0)
> >>>  goto fail;
> >>> -}
> >>> -frames = (AVHWFramesContext*)ctx->hwframes_ref->data;
> >>> -
> >>> -frames->format= outlink->format;
> >>> -frames->sw_format = hwfc->sw_format;
> >>> -frames->width = hwfc->width;
> >>> -frames->height= hwfc->height;
> >>> -
> >>> -if (avctx->extra_hw_frames >= 0)
> >>> -frames->initial_pool_size = 2 + avctx->extra_hw_frames;
> >>> -
> >>> -err = av_hwframe_ctx_init(ctx->hwframes_ref);
> >>> -if (err < 0) {
> >>> -av_log(avctx, AV_LOG_ERROR, "Failed to initialise "
> >>> -   "target frames context: %d.\n", err);
> >>> -goto fail;
> >>> -}
> >>>
> >>>  err = av_hwframe_ctx_create_derived(,
> >>>  inlink->format,
> >>> @@ -175,10 +187,20 @@ static int hwmap_config_output(AVFilterLink
> >> *outlink)
> >>>  inlink->hw_frames_ctx = source;
> >>>
> >>>  } else if ((outlink->format == hwfc->format &&
> >>> -inlink->format  == hwfc->sw_format) ||
> >>> -   inlink->format == hwfc->format) {
> >>> -// Map from a hardware format to a software format, or
> >>> -// undo an existing such mapping.
> >>> +inlink->format  == hwfc->sw_format)) {
> >>> +// unmap a software frame back to hardware
> >>> +ctx->reverse = 1;
> >>> +   

Re: [FFmpeg-devel] [PATCH 2/3] avcodec/lagarith: Optimize case with singleton probability distribution

2018-12-25 Thread Michael Niedermayer
On Mon, Dec 24, 2018 at 11:54:31PM +, Kieran Kunhya wrote:
> >
> > commit 0ca7a8deeffd33e05ae15a447259b32b6678c727 (HEAD -> master)
> > Author: Michael Niedermayer 
> > Date:   Mon Dec 24 01:14:50 2018 +0100
> >
> > avcodec/lagarith: Optimize case with singleton probability distribution
> >
> > Fixes: Timeout
> > Fixes:
> > 10554/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_LAGARITH_fuzzer-5739938067251200
> >
> > In case of a Denial of Service attack, the attacker wants to maximize
> > the load on the target
> > per byte transmitted from the attacker.
> > For such a DoS attack it is best for the attacker to setup the
> > probabilities so that the
> > arithmetic decoder does not advance in the bytestream that way the
> > attacker only needs to
> > transmit the initial bytes and header for an arbitrary large frame.
> > This patch here optimizes this codepath and avoids executing the
> > arithmetic decoder more than
> > once. It thus reduces the load causes by this codepath on the target.
> > We also could completely disallow this codepath but it appears such
> > odd probability
> > distributions are not invalid.
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> >
> 
> This is a nonsense argument, a user could send a frame that was
> x in dimensions, would have the same effect.

This suggested attack would not work, a user wanting to minimize these
DoS issues would have set AVCodecContext.max_pixels which would block this.


> The calling application should manage timeouts themselves in a sandbox or
> container or similar.

Its always possible and also a very good idea to have a 2nd line of defense
like a sandbox / VM, ... as you suggest here, I did and do agree here.
And also a 3rd line of defense, ...

But this doesnt mean we should not attempt to fix or mitigate 
security (or other) issues directly in the code.

I think the point you are raising has been raised previously, so let me
argue a little broader here and not specific to just what you suggest.

If you compare a native fix in the code with a fix by a timeout, a
fix by a timeout causes:
* The whole process to be killed, so any application using libavcodec
  would basically "crash" and would not neccessarily save its state,
  flush out buffers, write any trailers or do proper protocol shutdowns
  or save any unsafed data. This is a outcome that should be minimized
* Using a timeout as the main way to block DoS is difficult as there is
  often no good timeout value. Its not unexpected that a system may need to
  support processing large videos taking several hours, thats a long 
  time for a file of a hundread bytes in a DoS attack 
  100bytes from an attacker could cause the same load as 10 bytes from
  a user.
* Worst maybe is that a tight timeout likely makes the system more vulnerable
  not less. because during an attack all the processes would likely slow
  down and real users would be pushed into the timeout even if the system
  without the user processes timeouting would still function correctly
  
Iam sure there are more arguments but above are the ones that came to my mind
ATM.

> 
> Merry Xmas.

Merry Xmas as well!

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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


Re: [FFmpeg-devel] [PATCH V1] avfilter: Add tonemap vaapi filter

2018-12-25 Thread Moritz Barsnick
On Tue, Dec 25, 2018 at 15:16:17 +0800, Zachary Zhou wrote:
> It supports ICL platform.
> H2H (HDR to HDR): P010 -> RGB10
> H2S (HDR to SDR): P010 -> RGB8
> 
> libva commit for HDR10
> https://github.com/intel/libva/commit/cf11abe5e1b9c93ee75cf974076957162c1605b9

BTW, there's a typo in your libva doxygen comments ("berief" <->
"brief"). Did doxygen even run successfully?

[...]
> +check_struct "va/va.h va/va_vpp.h" "VAProcPipelineParameterBuffer" 
> output_hdr_metadata

Does libva really not do any micro version bumping or the likes when
adding features?

> +int ff_vaapi_vpp_make_param_buffers2(AVFilterContext *avctx,

Shouldn't ff_vaapi_vpp_make_param_buffers() be converted to use this?
Eventually?

> +#define LAV_UINT32(a) (a.num)
> +in_metadata->max_display_mastering_luminance =
> +LAV_UINT32(hdr_meta->max_luminance);
> +in_metadata->min_display_mastering_luminance =
> +LAV_UINT32(hdr_meta->min_luminance);
> +#undef LAV_UINT16

What are you undefining here? Didn't you mean to undef LAV_UINT32?

> +if (!metadata) {
> +av_log(avctx, AV_LOG_ERROR, "Can't new side data for 
> output\n");

"new" is not really a verb. ;-) "Can't create new ..."? ("Cannot"
preferred, actually.)

> +if (!metadata_lt) {
> +av_log(avctx, AV_LOG_ERROR, "Can't new side data for 
> output\n");

Ditto.

> +if ((err = ff_formats_ref(ff_make_format_list(pix_in_fmts),
> +  >inputs[0]->out_formats)) < 0)
> +return err;

There was some discussion here that the style
err = ff_formats_ref(ff_make_format_list(pix_in_fmts), 
>inputs[0]->out_formats)
if (err < 0)
return err;

is preferred for readability.

Sorry that some remarks are in form of questions. I'm not totally sure.

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


Re: [FFmpeg-devel] [PATCH v2] avcodec/fft_template: libavcodec/fft_template: improve performance of the ff_fft_init in fft_template

2018-12-25 Thread Paul B Mahol
On 12/25/18, Liu Steven  wrote:
>
>
>> 在 2018年12月25日,上午1:28,Paul B Mahol  写道:
>>
>> On 12/24/18, Michael Niedermayer  wrote:
>>> On Fri, Dec 21, 2018 at 06:09:50PM +0800, Steven Liu wrote:
 Before patch:
 init nbits = 17, get 1 samples, average cost: 16105 us
 After patch:
 init nbits = 17, get 1 samples, average cost: 15221 us

 Signed-off-by: Steven Liu 
 ---
 libavcodec/fft_template.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

 diff --git a/libavcodec/fft_template.c b/libavcodec/fft_template.c
 index 762c014bc8..2b528be882 100644
 --- a/libavcodec/fft_template.c
 +++ b/libavcodec/fft_template.c
 @@ -261,17 +261,21 @@ av_cold int ff_fft_init(FFTContext *s, int nbits,
 int inverse)
 if (s->fft_permutation == FF_FFT_PERM_AVX) {
 fft_perm_avx(s);
 } else {
 -for(i=0; i>>> -int k;
 -j = i;
 -if (s->fft_permutation == FF_FFT_PERM_SWAP_LSBS)
 -j = (j&~3) | ((j>>1)&1) | ((j<<1)&2);
 -k = -split_radix_permutation(i, n, s->inverse) & (n-1);
 -if (s->revtab)
 -s->revtab[k] = j;
 -if (s->revtab32)
 -s->revtab32[k] = j;
 -}
 +#define SPLIT_RADIX_PERMUTATION(num) do { \
 +for(i=0; i>>> +int k;\
 +j = i;\
>>>
 +if (s->fft_permutation == FF_FFT_PERM_SWAP_LSBS)\
>>>
>>> maybe this if() should be factored out too ?
>>>
>>> the change looks good though, and iam not sure this is speed relevant
>>> enough
>>
>> Well, it helps. But this code is partially going to be rewritten
>> anyway in future.
>
> Hi Paul,
>
> So what i should do? Go on update this patch with Michael’s comment  or
> waiting for your rewritten?

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


Re: [FFmpeg-devel] [PATCH 4/4] zmbvenc: use unsigned values for score calculations

2018-12-25 Thread Tomas Härdin
lör 2018-12-22 klockan 15:54 + skrev Matthew Fearnley:
> > On 22 Dec 2018, at 12:15, Tomas Härdin  wrote:
> > 
> > tor 2018-12-20 klockan 17:48 + skrev Matthew Fearnley:
> > > > > > On Thu, 20 Dec 2018 at 16:30, Tomas Härdin  > > > > > .se> wrote:
> > > > 
> > > > Trivial enough. You could probably roll many or even all of
> > > > these
> > > > patches together
> > > 
> > > Thanks for your feedback.  I was reluctant to submit so many
> > > patches, but I
> > > worried at the time that combining them would require an overly
> > > long commit
> > > message.  Maybe I'm being too verbose though...
> > > What's the best way to submit a combined patch - from a
> > > Patchwork/mailing
> > > list perspective?
> > 
> > How large a patch should be is somewhat arbitrary. I tend to try to
> > keep each patch related to a single "concept". And of course
> > separating
> > cosmetic and functional changes
> > 
> > It's mostly the having to have PATCH 3/4 before PATCH 1/4 prompting
> > this suggestion. Or maybe rolling them together
> 
> If I roll the patches together (and include the additional
> suggestions in PATCH 1), would it be best to just email a new patch,
> and have these closed?

If you have a new set of patches then a new thread is best, yes

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


Re: [FFmpeg-devel] [PATCH 1/4] zmbvenc: don't sum the entropy when blocks are equal

2018-12-25 Thread Tomas Härdin
lör 2018-12-22 klockan 15:32 + skrev Matthew Fearnley:
> > > > 
> > > Note that bw,bh aren't guaranteed to equal ZMBV_BLOCK, so `histogram[0] ==
> > > bw*bh` would have to be used to guard against those (literal) edge cases.
> > 
> > Right, yes. But if we have block sizes other than ZMBV_BLOCK^2 then we
> > need score tables for those sizes too.
> 
> I’ve thought about that a bit. I wondered if it would be worth it given:
> - the extra code, memory and logic needed

If you have a huge amount of DOS captures to optimize then it might be
worth it, else probably questionable

> - it would only improve the edge blocks

I imagine large blocks would be good for scenes with mostly global
motion. You cut down on the number of MVs and thus the amount of data
zlib has to compress, if the block size is a good fit.

> - the existing score table isn’t catastrophically bad for short blocks, and 
> would still favour blocks with more common pixels.
> 
> It would be better from a correctness perspective though, and effects on 
> running time should be negligible.

Good point. There's also no telling whether the current model is
actually an accurate prediction of how zlib behaves :)

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


Re: [FFmpeg-devel] [PATCH 2/3] avformat/mxfdec: support subsecond precision of decoded timestamps

2018-12-25 Thread Tomas Härdin
sön 2018-12-23 klockan 01:12 +0100 skrev Marton Balint:
> > Signed-off-by: Marton Balint 
> ---
>  libavformat/mxfdec.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index d78f8ad2e4..0553adcb06 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -2556,12 +2556,14 @@ fail_and_free:
>  static int64_t mxf_timestamp_to_int64(uint64_t timestamp)
>  {
>  struct tm time = { 0 };
> +int msecs;
>  time.tm_year = (timestamp >> 48) - 1900;
>  time.tm_mon  = (timestamp >> 40 & 0xFF) - 1;
>  time.tm_mday = (timestamp >> 32 & 0xFF);
>  time.tm_hour = (timestamp >> 24 & 0xFF);
>  time.tm_min  = (timestamp >> 16 & 0xFF);
>  time.tm_sec  = (timestamp >> 8  & 0xFF);
> +msecs= (timestamp & 0xFF) * 4;
>  
>  /* Clip values for legacy reasons. Maybe we should return error instead? 
> */
>  time.tm_mon  = av_clip(time.tm_mon,  0, 11);
> @@ -2569,8 +2571,9 @@ static int64_t mxf_timestamp_to_int64(uint64_t 
> timestamp)
>  time.tm_hour = av_clip(time.tm_hour, 0, 23);
>  time.tm_min  = av_clip(time.tm_min,  0, 59);
>  time.tm_sec  = av_clip(time.tm_sec,  0, 59);
> +msecs= av_clip(msecs, 0, 999);
>  
> -return (int64_t)av_timegm() * 100;
> +return (int64_t)av_timegm() * 100 + msecs * 1000;

Looks OK

I kinda wonder how this and the muxer code could have ignored the
milliseconds. Did the old creation time metadata use seconds only?

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


Re: [FFmpeg-devel] [PATCH 3/3] avformat/mxfenc: support writing subsecond precision timestamps

2018-12-25 Thread Tomas Härdin
sön 2018-12-23 klockan 01:12 +0100 skrev Marton Balint:
> > Signed-off-by: Marton Balint 
> ---
>  libavformat/mxfenc.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 5e7bd212dc..032ee3bf3d 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -2332,8 +2332,9 @@ static int mxf_parse_mpeg2_frame(AVFormatContext *s, 
> AVStream *st,
>  return !!sc->codec_ul;
>  }
>  
> -static uint64_t mxf_parse_timestamp(time_t timestamp)
> +static uint64_t mxf_parse_timestamp(int64_t timestamp64)
>  {
> +time_t timestamp = timestamp64 / 100;
>  struct tm tmbuf;
>  struct tm *time = gmtime_r(, );
>  if (!time)
> @@ -2343,7 +2344,8 @@ static uint64_t mxf_parse_timestamp(time_t timestamp)
> (uint64_t) time->tm_mday   << 32 |
>    time->tm_hour   << 24 |
>    time->tm_min<< 16 |
> -  time->tm_sec<< 8;
> +  time->tm_sec<< 8  |
> +  (timestamp64 % 100) / 4000;
>  }
>  
>  static void mxf_gen_umid(AVFormatContext *s)
> @@ -2580,7 +2582,7 @@ static int mxf_write_header(AVFormatContext *s)
>  sc->order = AV_RB32(sc->track_essence_element_key+12);
>  }
>  
> -if (ff_parse_creation_time_metadata(s, , 1) > 0)
> +if (ff_parse_creation_time_metadata(s, , 0) > 0)
>  mxf->timestamp = mxf_parse_timestamp(timestamp);
>  mxf->duration = -1;

Looks correct to me

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