[FFmpeg-devel] [PATCH] avcodec/h264_parse: no need check ref list1 for P slices.

2019-01-30 Thread Decai Lin
This is robust for some corner case there is incorrect list1 count
in pps header, but it's a P slice and can be decoded well.

Signed-off-by: Decai Lin 
---
 libavcodec/h264_parse.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index 290ab68..a42cc29 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -242,7 +242,12 @@ int ff_h264_parse_ref_count(int *plist_count, int 
ref_count[2],
 ref_count[1] = 1;
 }
 
-if (ref_count[0] - 1 > max[0] || ref_count[1] - 1 > max[1]) {
+if (slice_type_nos == AV_PICTURE_TYPE_B)
+list_count = 2;
+else
+list_count = 1;
+
+if (ref_count[0] - 1 > max[0] || (list_count == 2 && (ref_count[1] - 1 
> max[1]))) {
 av_log(logctx, AV_LOG_ERROR, "reference overflow %u > %u or %u > 
%u\n",
ref_count[0] - 1, max[0], ref_count[1] - 1, max[1]);
 ref_count[0] = ref_count[1] = 0;
@@ -250,10 +255,6 @@ int ff_h264_parse_ref_count(int *plist_count, int 
ref_count[2],
 goto fail;
 }
 
-if (slice_type_nos == AV_PICTURE_TYPE_B)
-list_count = 2;
-else
-list_count = 1;
 } else {
 list_count   = 0;
 ref_count[0] = ref_count[1] = 0;
-- 
1.8.3.1

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


Re: [FFmpeg-devel] [PATCH] avformat/dashenc: Skip writing trailer for MP4 output when in streaming mode

2019-01-30 Thread myp...@gmail.com
On Thu, Jan 24, 2019 at 2:01 PM Karthick J  wrote:
>
> In streaming mode mp4 trailer is not required for playout.
> ---
>  libavformat/dashenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index 9c90cf17e5..6299e179c2 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -1210,7 +1210,7 @@ static int dash_init(AVFormatContext *s)
>
>  if (os->segment_type == SEGMENT_TYPE_MP4) {
>  if (c->streaming)
Can we add some comments when setting some special flags in the code,
not just comments in the commit message?
> -av_dict_set(, "movflags", 
> "frag_every_frame+dash+delay_moov+skip_sidx", 0);
> +av_dict_set(, "movflags", 
> "frag_every_frame+dash+delay_moov+skip_sidx+skip_trailer", 0);
>  else
>  av_dict_set(, "movflags", 
> "frag_custom+dash+delay_moov", 0);
>  } else {
> --
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread James Almer
On 1/30/2019 10:20 PM, Chris Cunningham wrote:
>>
 And this definitely means there's a bug elsewhere. This parser should
 have not been used for codecs ids other than GSM and GSM_MS. That's
 precisely what this assert() is making sure of.

>>>
>>> I'll take a closer look at how this parser was selected.
>>
> 
> Ok, here are full details of how this happens:
> 
>1. AV_CODEC_ID_GSM_MS is assigned to on st->codecpar->codec_id
>in ogm_header() (oggparseogm.c:75)
>2. The (deprecated) st->codec->codec_id is not assigned at that time and
>remains 0 (UNKNOWN)
>3. AV_CODEC_ID_GSM_MS from st->codecpar is passed to av_parser_init,
>selecting the gsm parser (in read_frame_internal())
>4. The fuzzer next (only) packet has a size of 0, so the first call to
>parse_packet() (still in read_frame_internal()) does nothing
>5. After this call we assign several members of st->internal->avctx to
>st->codecpar. This leaves codecpar->codec_id = UNKNOWN. Interestingly, we
>do not reset the st->parser at this point (maybe we should?)

Where does this happen? A call to avcodec_parameters_from_context()
should also copy codec_id.

>6. Next iteration of the read_frame_internal() loop we get EOF from
>ff_read_packet. This triggers the "flush the parsers" call to
>parse_packet() which finally reaches gsm_parse() to trigger the abort
>(avctx->codec_id is still 0).
> 
> Questions (guessing at the fix):
> 
>- At what point should st->codec->codec_id be synchronized with
>st->codecpar->codec_id? It never is in this test.

In avformat_find_stream_info(), i think. In any case, st->codec should
have no effect on parsers. What is used in parse_packet() however is
st->internal->avctx, so that context is the one that evidently contains
codec_id == UNKNOWN when it should be in sync with codecpar.

>- Should we reset st->parser whenever we reset st->codecpar->codec_id
>(step 5 above)?
> 
> Thanks,
> Chris
> ___
> 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] lavfi/vf_nlmeans: Remove the pdiff_lut_scale to improve the performance

2019-01-30 Thread myp...@gmail.com
On Wed, Jan 30, 2019 at 10:24 PM Carl Eugen Hoyos  wrote:
>
> 2019-01-30 11:56 GMT+01:00, Jun Zhao :
> > Remove the pdiff_lut_scale in nlmeans
>
> This sentence is very misleading.
>
> > and this change will avoid
> > using pdiff_lut_scale in the exp table search in nlmean_slice, it's will
> > improve the performance about 12%.
>
> Please mention in the commit message that you increase
> the context size including the amount (and probably remove
> the part above that you "remove" something from the context).
>

Looks like I need to improve the commit message, will update in the V2. :)

In fact, when I try to use nlmeans for denoising  a 1080P size pictures, I found
nlmeans is really SO slow, then I profiling the code, found nlmeans_slice is the
bottleneck, this is the  to remove the pdiff_lut_scale when search the
exp() table in nlmeans_slice function.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread Chris Cunningham
>
> >> And this definitely means there's a bug elsewhere. This parser should
> >> have not been used for codecs ids other than GSM and GSM_MS. That's
> >> precisely what this assert() is making sure of.
> >>
> >
> > I'll take a closer look at how this parser was selected.
>

Ok, here are full details of how this happens:

   1. AV_CODEC_ID_GSM_MS is assigned to on st->codecpar->codec_id
   in ogm_header() (oggparseogm.c:75)
   2. The (deprecated) st->codec->codec_id is not assigned at that time and
   remains 0 (UNKNOWN)
   3. AV_CODEC_ID_GSM_MS from st->codecpar is passed to av_parser_init,
   selecting the gsm parser (in read_frame_internal())
   4. The fuzzer next (only) packet has a size of 0, so the first call to
   parse_packet() (still in read_frame_internal()) does nothing
   5. After this call we assign several members of st->internal->avctx to
   st->codecpar. This leaves codecpar->codec_id = UNKNOWN. Interestingly, we
   do not reset the st->parser at this point (maybe we should?)
   6. Next iteration of the read_frame_internal() loop we get EOF from
   ff_read_packet. This triggers the "flush the parsers" call to
   parse_packet() which finally reaches gsm_parse() to trigger the abort
   (avctx->codec_id is still 0).

Questions (guessing at the fix):

   - At what point should st->codec->codec_id be synchronized with
   st->codecpar->codec_id? It never is in this test.
   - Should we reset st->parser whenever we reset st->codecpar->codec_id
   (step 5 above)?

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


[FFmpeg-devel] [PATCH] avcodec/ac3: Explicitly return to discard large amounts of nonsense bytes

2019-01-30 Thread Michael Niedermayer
Changes 19sec to 10ms (12559) runtime, 17sec to 177ms (12570)
Fixes: Timeout
Fixes: 
12559/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AC3_fuzzer-5666516266123264
Fixes: 
12561/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AC3_FIXED_fuzzer-5682923041193984
Fixes: 
12570/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_EAC3_fuzzer-5194734308425728

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

diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
index f844a463ee..eaa327a3ee 100644
--- a/libavcodec/ac3dec.c
+++ b/libavcodec/ac3dec.c
@@ -1490,6 +1490,8 @@ static int ac3_decode_frame(AVCodecContext * avctx, void 
*data,
 }
 if (i >= buf_size)
 return AVERROR_INVALIDDATA;
+if (i > 10)
+return i;
 buf += i;
 buf_size -= i;
 
-- 
2.20.1

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


Re: [FFmpeg-devel] [PATCH] 'UDP' protocol (in upper case) in SDP m= section was parsed correctly

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 16:57 GMT+01:00, Yannick Poirier :
> ---
>  libavformat/rtsp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index ceb770a3a4..4899e4e790 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -480,7 +480,7 @@ static void sdp_parse_line(AVFormatContext *s,
> SDPParseState *s1,
>  rtsp_st->sdp_port = atoi(buf1);
>
>  get_word(buf1, sizeof(buf1), ); /* protocol */
> -if (!strcmp(buf1, "udp"))
> +if (!av_strcasecmp(buf1, "udp"))

Is there an rfc or similar that supports this?

In any case, the commit message can be improved.

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


Re: [FFmpeg-devel] [PATCH 2/3] flvdec: Mark the demuxer as allowing discontinuous timestamps

2019-01-30 Thread Michael Niedermayer
On Thu, Jan 17, 2019 at 09:45:04PM +, Derek Buitenhuis wrote:
> On 15/01/2019 21:24, Michael Niedermayer wrote:
> > Heres a better patch which may work with seeking as long as there are only
> > 2 files concatenated
> > 
> > i think completely discarding the parts after the concatenation would cause
> > user complaints and they would have a point, if the data is in there we
> > should be able to recover it for muxing it in a better file at least
> 
> No strong opinion. You can push.

will apply

thanks

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

Does the universe only have a finite lifespan? No, its going to go on
forever, its just that you wont like living in it. -- Hiranya Peiri


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


[FFmpeg-devel] [PATCH] fix sidx size being doubled in offset. fixes an issue where if the video size was very specific, ffmpeg would hang from not filling the sidx_pts for all streams, due to not read

2019-01-30 Thread agrecascino123
From: mptcultist 

---
 libavformat/mov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..c222582886 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4933,7 +4933,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 
 static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-int64_t offset = avio_tell(pb) + atom.size, pts, timestamp;
+int64_t offset = avio_tell(pb), pts, timestamp;
 uint8_t version;
 unsigned i, j, track_id, item_count;
 AVStream *st = NULL;
-- 
2.20.1

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


[FFmpeg-devel] [PATCH] avformat/mov: fix sidx size being doubled in offset.

2019-01-30 Thread agrecascino123
From: mptcultist 

fixes an issue where if the video size was very specific, ffmpeg would hang 
from not filling the sidx_pts for all streams, due to not reading the last sidx 
lump. for #7572

---
 libavformat/mov.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..c222582886 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4933,7 +4933,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)
 
 static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
-int64_t offset = avio_tell(pb) + atom.size, pts, timestamp;
+int64_t offset = avio_tell(pb), pts, timestamp;
 uint8_t version;
 unsigned i, j, track_id, item_count;
 AVStream *st = NULL;
-- 
2.20.1

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


[FFmpeg-devel] [PATCH] Replaces a couple of git attributes to simple "binary"

2019-01-30 Thread vonorfasyexela
From: Alexey Safronov 

It slightly improves the readability of the code
---
 .gitattributes | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.gitattributes b/.gitattributes
index 5a19b963b6..ca7879c8a6 100644
--- a/.gitattributes
+++ b/.gitattributes
@@ -1,2 +1,2 @@
-*.pnm -diff -text
+*.pnm binary
 tests/ref/fate/sub-scc eol=crlf
-- 
2.19.0

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


[FFmpeg-devel] [PATCH] 'UDP' protocol (in upper case) in SDP m= section was parsed correctly

2019-01-30 Thread Yannick Poirier
---
 libavformat/rtsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index ceb770a3a4..4899e4e790 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -480,7 +480,7 @@ static void sdp_parse_line(AVFormatContext *s, 
SDPParseState *s1,
 rtsp_st->sdp_port = atoi(buf1);
 
 get_word(buf1, sizeof(buf1), ); /* protocol */
-if (!strcmp(buf1, "udp"))
+if (!av_strcasecmp(buf1, "udp"))
 rt->transport = RTSP_TRANSPORT_RAW;
 else if (strstr(buf1, "/AVPF") || strstr(buf1, "/SAVPF"))
 rtsp_st->feedback = 1;
-- 
2.20.1

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


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread James Almer
On 1/30/2019 6:44 PM, Chris Cunningham wrote:
> On Wed, Jan 30, 2019 at 1:40 PM James Almer  wrote:
> 
>> Parsers can't return negative values, only the output packet size. For
>> the purpose of errors, they usually return the entire untouched packet
>> size.
>>
> 
> Understood. Is this documented somewhere? I noted the comment in
> av_paser_parse2(), "/* WARNING: the returned index can be negative */", and
> guessed that signaled an error.

/* This callback never returns an error, a negative value means that
 * the frame start was in a previous packet. */
int (*parser_parse)(AVCodecParserContext *s,
AVCodecContext *avctx,
const uint8_t **poutbuf, int *poutbuf_size,
const uint8_t *buf, int buf_size);

So i was wrong in that it's not that it can't return negative values,
just not for the purpose of signaling errors.

> 
> 
>> And this definitely means there's a bug elsewhere. This parser should
>> have not been used for codecs ids other than GSM and GSM_MS. That's
>> precisely what this assert() is making sure of.
>>
> 
> I'll take a closer look at how this parser was selected.
> 
> Thanks for the quick reply.
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

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


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread Chris Cunningham
On Wed, Jan 30, 2019 at 1:40 PM James Almer  wrote:

> Parsers can't return negative values, only the output packet size. For
> the purpose of errors, they usually return the entire untouched packet
> size.
>

Understood. Is this documented somewhere? I noted the comment in
av_paser_parse2(), "/* WARNING: the returned index can be negative */", and
guessed that signaled an error.


> And this definitely means there's a bug elsewhere. This parser should
> have not been used for codecs ids other than GSM and GSM_MS. That's
> precisely what this assert() is making sure of.
>

I'll take a closer look at how this parser was selected.

Thanks for the quick reply.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 1/2] avformat/rtsp: Clear reply in every iteration in ff_rtsp_connect()

2019-01-30 Thread Michael Niedermayer
On Mon, Jan 28, 2019 at 12:53:22AM +0100, Michael Niedermayer wrote:
> Fixes: Infinite loop
> 
> Found-by: Michael Hanselmann 
> Reviewed-by: Michael Hanselmann 
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/rtsp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

will apply patchset


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

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.


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


Re: [FFmpeg-devel] [PATCH 1/3] avcodec/rasc: Check uncompressed dlta size

2019-01-30 Thread Michael Niedermayer
On Wed, Jan 23, 2019 at 02:10:18AM +0100, Michael Niedermayer wrote:
> We assume that if the compressed size is bigger than if each byte is encoded 
> in a single raw packet
> that the data is invalid.
> 
> Fixes: Out of memory
> Fixes: 
> 12208/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_RASC_fuzzer-5648916473708544
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/rasc.c | 2 ++
>  1 file changed, 2 insertions(+)

will apply patchset

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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.


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


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread James Almer
On 1/30/2019 6:27 PM, chcunningham wrote:
> Return replaces an assert0. libfuzzer generated a testcase that
> triggered this assert (codec=0), causing a crash of chrome's renderer.
> ---
>  libavcodec/gsm_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/gsm_parser.c b/libavcodec/gsm_parser.c
> index 1054a30ca9..5cf2235f73 100644
> --- a/libavcodec/gsm_parser.c
> +++ b/libavcodec/gsm_parser.c
> @@ -56,7 +56,7 @@ static int gsm_parse(AVCodecParserContext *s1, 
> AVCodecContext *avctx,
>  s->duration   = GSM_FRAME_SIZE * 2;
>  break;
>  default:
> -av_assert0(0);
> +return -1;
>  }
>  }

Parsers can't return negative values, only the output packet size. For
the purpose of errors, they usually return the entire untouched packet size.

And this definitely means there's a bug elsewhere. This parser should
have not been used for codecs ids other than GSM and GSM_MS. That's
precisely what this assert() is making sure of.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread Nicolas George
chcunningham (12019-01-30):
> Return replaces an assert0. libfuzzer generated a testcase that
> triggered this assert (codec=0), causing a crash of chrome's renderer.
> ---
>  libavcodec/gsm_parser.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/gsm_parser.c b/libavcodec/gsm_parser.c
> index 1054a30ca9..5cf2235f73 100644
> --- a/libavcodec/gsm_parser.c
> +++ b/libavcodec/gsm_parser.c
> @@ -56,7 +56,7 @@ static int gsm_parse(AVCodecParserContext *s1, 
> AVCodecContext *avctx,
>  s->duration   = GSM_FRAME_SIZE * 2;
>  break;
>  default:
> -av_assert0(0);
> +return -1;
>  }
>  }

-1 is not a correct error code.

Also, an assert() means the code was supposed to be unreachable. If it
is not, that means a bug is lurking somewhere else, it must be found,
not just hidden.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 22:27 GMT+01:00, chcunningham :
> Return replaces an assert0. libfuzzer generated a testcase that

> triggered this assert (codec=0), causing a crash of chrome's renderer.

Wouldn't this indicate a bug somewhere else?

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


[FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-01-30 Thread chcunningham
Return replaces an assert0. libfuzzer generated a testcase that
triggered this assert (codec=0), causing a crash of chrome's renderer.
---
 libavcodec/gsm_parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/gsm_parser.c b/libavcodec/gsm_parser.c
index 1054a30ca9..5cf2235f73 100644
--- a/libavcodec/gsm_parser.c
+++ b/libavcodec/gsm_parser.c
@@ -56,7 +56,7 @@ static int gsm_parse(AVCodecParserContext *s1, AVCodecContext 
*avctx,
 s->duration   = GSM_FRAME_SIZE * 2;
 break;
 default:
-av_assert0(0);
+return -1;
 }
 }
 
-- 
2.20.1.495.gaa96b0ce6b-goog

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


Re: [FFmpeg-devel] [PATCH] avformat/hcom: Fix "set but not used" warnings

2019-01-30 Thread Michael Niedermayer
On Sat, Jan 19, 2019 at 09:12:41PM -0300, James Almer wrote:
> On 1/19/2019 7:54 PM, Michael Niedermayer wrote:
> > On Sat, Jan 19, 2019 at 04:11:07PM -0300, James Almer wrote:
> >> On 1/19/2019 4:08 PM, Michael Niedermayer wrote:
> >>> On Sat, Jan 19, 2019 at 03:53:55PM -0300, James Almer wrote:
>  On 1/19/2019 3:50 PM, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/hcom.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/hcom.c b/libavformat/hcom.c
> > index 35515cc5b2..6d3d726fa5 100644
> > --- a/libavformat/hcom.c
> > +++ b/libavformat/hcom.c
> > @@ -38,7 +38,7 @@ static int hcom_probe(AVProbeData *p)
> >  static int hcom_read_header(AVFormatContext *s)
> >  {
> >  AVStream *st;
> > -unsigned data_size, rsrc_size, huffcount;
> > +av_unused unsigned data_size, rsrc_size, huffcount;
> >  unsigned compresstype, divisor;
> >  unsigned dict_entries;
> >  int ret;
> 
>  No, they should be removed and the relevant avio_rb32() replaced with
>  avio_skip() alongside a comment to document what those bytes represent.
> >>>
> >>> I thought there might be some future code or checks that will use these
> >>> but if thats not the case then sure i can remove them
> >>>
> >>> thx
> >>
> >> data_size could i guess be used to set stream duration, 
> > 
> > where can i find hcom samples ?
> > do you have some URLs ? 
> > (for testing such use of the field)
> 
> Paul should know since he wrote both the decoder and demuxer. Also Compn
> i think.

ping, does someone have a sample for this ?

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

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


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


Re: [FFmpeg-devel] gdigrab: fix HIDPI support for mouse positioning

2019-01-30 Thread Carl Eugen Hoyos
2019-01-27 20:15 GMT+01:00, Dilshod Mukhtarov :
> New version, which uses integer arithmetics

Patch applied.

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


Re: [FFmpeg-devel] gdigrab: fix HIDPI support

2019-01-30 Thread Carl Eugen Hoyos
2019-01-27 20:14 GMT+01:00, Dilshod Mukhtarov :
>
> On 27.01.2019 22:51, Carl Eugen Hoyos wrote:
>> 2019-01-27 1:45 GMT+01:00, Carl Eugen Hoyos :
>>> 2019-01-26 18:53 GMT+01:00, Dilshod Mukhtarov :
 HI, this is the patch that fixes HIDPI support in gdigrab
 +double h_dpr;   // Horizontal device pixel ratio
 +double v_dpr;   // Vertical device pixel ratio
>>> I would expect these to be AVRational, if this is not
>>> possible, it should be explained why.
>> Sorry if this was not an ideal comment, it should have been:
>> All aspect ratio calculations (and all calculations related to
>> aspect ratio) do not need double (or floats) but should be
>> done as integer calculations. AVRational structs are often
>> used, theoretically they may not be needed.
> I attached new version of the patch, which uses integer arithmetics

Patch applied.

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


Re: [FFmpeg-devel] configure: Fix decklink license dependency

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 18:08 GMT+01:00, Elliott Balsley :
>
>> The BlackMagic Decklink drivers are not system libraries.
>
> Thank you for clarifying about the headers vs the library, I was
> confused about that at first.  The GPL definition of system library
> is hard to understand, but I was mostly basing my opinion on
> this discussion of NVENC. It relies on the proprietary Nvidia
> driver, which you agreed was a system library.

> How is the Decklink driver different?

Imo, it is different in (nearly) every regard but as said, I don't see
how this discussion can be fruitful in any way.

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


Re: [FFmpeg-devel] [PATCH]lavf:Constify AVInputFormat pointer

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 17:03 GMT+01:00, James Almer :
> On 1/30/2019 12:33 PM, Michael Niedermayer wrote:
>> On Sun, Jan 27, 2019 at 10:38:13PM +0100, Carl Eugen Hoyos wrote:

>>> Attached patch was requested in ticket #7220 iiuc.
>>>
>>> Please review, Carl Eugen
>>
>>>  avformat.h |8 
>>>  format.c   |4 ++--
>>>  hls.c  |2 +-
>>>  utils.c|4 ++--
>>>  4 files changed, 9 insertions(+), 9 deletions(-)
>>> 0a59a10c224ba092d8b7b61e3cac78a93c23bee2
>>> 0001-lavf-Constify-AVInputFormat-pointer.patch
>>> From a713b58767e8d77b641d1b87e68de11c176fd454 Mon Sep 17 00:00:00 2001
>>> From: Carl Eugen Hoyos 
>>> Date: Sun, 27 Jan 2019 22:35:51 +0100
>>> Subject: [PATCH] lavf: Constify AVInputFormat pointer.
>>
>> there is some tiny chance this could break some user apps
>> build i think so if done strictly correct it would need to be
>> delayed to the next bump i think

Yes, it unfortunately breaks C++ compilation:
#define __STDC_CONSTANT_MACROS
#include "libavformat/avformat.h"
int main()
{
/*const*/ AVInputFormat **dummy = NULL;
return av_probe_input_buffer2(NULL, dummy, NULL, NULL, 0, 0);
}

> We have tied constifying (or the opposite) to major bumps before,
> so i agree.

It depended on the placement of const, I believe this patch did not
break anything:
http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=f2c86705

Will try to create a new patch with a version dependency, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread jannis_wth
30.01.19 21:02 - James Almer:
> On 1/30/2019 4:19 PM, jannis_wth wrote:
>> 30.01.19 19:51 James Almer:
>>> On 1/30/2019 3:43 PM, jannis_wth wrote:
 Okay, so how about this one?
 This functionality is important if you don't know the packet size you
 are feeding. It allows you to reconstruct the size.
>>>
>>> In what scenario you can't know the size of the AVPacket you're feeding
>>> to avcodec_send_packet().
>>>
>> For example when a mp4 container lost its moov atom.
> 
> What value is stored in pkt->size in this scenario at the time you feed
> it to libavcodec? This function will return that value after all,
> provided it's valid data.
> 
> Also, AVCodecParsers are the usual way to reconstruct/assemble broken or
> incomplete packets.

pkt->size and its offset can be used to rebuild the moov atom.
I tried av_parser_parse2() but it I think it's not what I need. Its
return value does not match the true packet size, not even close.

>>> This is only used by decoders implementing the AVCodec.decode()
>>> callback. It will always be zero for any decoder using
>>> AVCodec.receive_frame(), like Bink Audio, libdav1d, etc.
>>
>> Well, what would be the correct way then?
>> Or is there no good way and thus you won't accept such interface?
> 
> Accepting it or not depends on what other developers think about such a
> function. Unlike the old API, the new one always fully consumes all
> packets you feed to it, so there's no real need to check how many bytes
> were consumed.
> Trying to expose the internals for this purpose is not really feasible,
> as you could see with all the different layers of internal buffering.
> 
> I guess avctx->internal->last_pkt_props could work for this, but it may
> not be consistent across decoders.

avctx->internal->last_pkt_props stores the properties of the last passed
packet, and does not get updated on decoding frames. Since I set the
input-packets size to a constant (size is unknown) this is not very
helpfull..

One could add a warning to avcodec_get_remaining_size() saying it only
works for some codecs.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Icecast protocol implementation doesn't parse "/" as a valid mountpoint

2019-01-30 Thread Marvin Scholz


On 30 Jan 2019, at 21:15, Carl Eugen Hoyos wrote:

> 2019-01-30 16:14 GMT+01:00, c0re :
>
>> diff --git a/libavformat/icecast.c b/libavformat/icecast.c
>
> We can only read patches made with "git format-patch".

I think this was just to illustrate how easy it
would be to change the behavior to work in the
expected way.

So far there did not seem to be much demand
in being able to do this and due to the mentioned
concerns (see my previous mail) I am not really
convinced this should be changed. But I am happy
to discuss this.

This was the first time someone actually asked
to allow to stream to mount '/' with the Icecast
protocol in ffmpeg, at least that I am aware of.

Icecast has mountpoints for a reason and not
using them seems quite weird.

>
> Carl Eugen
> ___
> 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/mov: fix sidx size being doubled in offset.

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 19:44 GMT+01:00, no pls :
> From: mptcultist 
>
> fixes an issue where if the video size was very specific,
> ffmpeg would hang from not filling the sidx_pts for all
> streams, due to not reading the last sidx lump. for #7572

Please wait for a review from John who has promised to look at
this issue "on Wednesday".

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


Re: [FFmpeg-devel] Icecast protocol implementation doesn't parse "/" as a valid mountpoint

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 16:14 GMT+01:00, c0re :

> diff --git a/libavformat/icecast.c b/libavformat/icecast.c

We can only read patches made with "git format-patch".

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


Re: [FFmpeg-devel] [PATCH]lavc/amrwbdec: Do not ignore NO_DATA frames

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 20:33 GMT+01:00, Paul B Mahol :
> On 1/30/19, Carl Eugen Hoyos  wrote:
>> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos :

>>> Attached patch fixes decoding NO_DATA amr-wb frames.
>>
>> Now with patch.

> Are you sure that reference decoder gives complete silence
> for such frames?

Probably not, the specification suggests "random signal" and
"a muting technique" to "gradually decrease the output level".

As said, the issue here is that current FFmpeg shortens the output
making it unusable, my patch fixes this.

> Please use memset.

The C standard apparently that memset(, 0, ) is not
required to work for float, do we require this?

Carl Eugen

The calloc function allocates space for an array of nmemb
objects, each of whose size is size. The space is initialized
to all bits zero. Note that this need not be the same as the
representation of floating-point zero...
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread James Almer
On 1/30/2019 4:19 PM, jannis_wth wrote:
> 30.01.19 19:51 James Almer:
>> On 1/30/2019 3:43 PM, jannis_wth wrote:
>>> Okay, so how about this one?
>>> This functionality is important if you don't know the packet size you
>>> are feeding. It allows you to reconstruct the size.
>>
>> In what scenario you can't know the size of the AVPacket you're feeding
>> to avcodec_send_packet().
>>
> For example when a mp4 container lost its moov atom.

What value is stored in pkt->size in this scenario at the time you feed
it to libavcodec? This function will return that value after all,
provided it's valid data.

Also, AVCodecParsers are the usual way to reconstruct/assemble broken or
incomplete packets.

> 
>>>
>>> 0001-avcodec-Allow-to-query-number-of-consumed-bytes-usin.patch
>>>
>>> From bad739e4f5e9e78cc6fa799287d758bf27db8208 Mon Sep 17 00:00:00 2001
>>> From: user 
>>> Date: Wed, 30 Jan 2019 19:33:08 +0100
>>> Subject: [PATCH 1/1] avcodec: Allow to query number of consumed bytes using
>>>  the new API
>>>
>>> Currently the only way to get the number of consumed bytes is by using
>>> the deprecated decode_audio4() API. This patch adds a simple function
>>> to fix this.
>>>
>>> Signed-off-by: Jannis Kambs 
>>> ---
>>>  libavcodec/avcodec.h | 9 +
>>>  libavcodec/utils.c   | 5 +
>>>  2 files changed, 14 insertions(+)
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index f554c53f0e..43bf84e58b 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -5714,6 +5714,15 @@ int av_get_audio_frame_duration(AVCodecContext 
>>> *avctx, int frame_bytes);
>>>   */
>>>  int av_get_audio_frame_duration2(AVCodecParameters *par, int frame_bytes);
>>>  
>>> +/**
>>> + * This function allows to get the number of remaining bytes after
>>> + * avcodec_receive_frame() has been called.
>>> + *
>>> + * @param avctx the codec context
>>> + * @return number of remaining bytes from the input AVPacket.
>>> + */
>>> +int avcodec_get_remaining_size(AVCodecContext *avctx);
>>> +
>>>  #if FF_API_OLD_BSF
>>>  typedef struct AVBitStreamFilterContext {
>>>  void *priv_data;
>>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>>> index d519b16092..638154f974 100644
>>> --- a/libavcodec/utils.c
>>> +++ b/libavcodec/utils.c
>>> @@ -1727,6 +1727,11 @@ int av_get_audio_frame_duration2(AVCodecParameters 
>>> *par, int frame_bytes)
>>>  frame_bytes);
>>>  }
>>>  
>>> +int avcodec_get_remaining_size(AVCodecContext *avctx)
>>> +{
>>> +return avctx->internal->ds.in_pkt->size;
>>
>> This is only used by decoders implementing the AVCodec.decode()
>> callback. It will always be zero for any decoder using
>> AVCodec.receive_frame(), like Bink Audio, libdav1d, etc.
> 
> Well, what would be the correct way then?
> Or is there no good way and thus you won't accept such interface?

Accepting it or not depends on what other developers think about such a
function. Unlike the old API, the new one always fully consumes all
packets you feed to it, so there's no real need to check how many bytes
were consumed.
Trying to expose the internals for this purpose is not really feasible,
as you could see with all the different layers of internal buffering.

I guess avctx->internal->last_pkt_props could work for this, but it may
not be consistent across decoders.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Decklink Output Problem

2019-01-30 Thread Deron

On 1/29/19 2:21 PM, Marton Balint wrote:



On Tue, 29 Jan 2019, Deron wrote:


Hello,

A little history: Many years back I started using a modified version 
of an unaccepted Decklink output patch, updated it to work for me, 
and reposted a newer patch. Of course, the patch was not accepted for 
many valid reasons like formatting and working only on Linux but it 
continued to work well for me so I have stuck with it. Once an 
accepted Decklink "encoder" was introduced, I gave it a try but could 
never get it to work right under Ubuntu. Always crashed or did 
something crazy. I figured it was me. Well, I've tried it out again 
and it work now for me. Almost. This time, instead of just shrugging 
my shoulders I'm asking if maybe it is not me since I can get it to 
work after a minor patch.


The Problem: When I output any video, the last 60 frames are lost. 
90% of the time I would not even notice it, but the other 10% is 
another story. No error is produced, and it is not file or command 
specific. What appears to me to be happening is that the buffered 
frames are tossed out at the end.


The variable "frames_buffer_available_spots" is set to the array size 
"frames_buffer" in the function "decklink_setup_video". Minimally, 
"frames_buffer" is set to 60. Then the function 
"decklink_write_video_packet" will wait if the number of available 
frame buffers is zero. All fine and good until it gets to the closing 
function "ff_decklink_write_trailer" where an abrupt call to the 
Decklink function "StopScheduledPlayback" will do just that and toss 
out all buffered frames. I have fixed this issue for me by including 
the following ugly busy loop in "ff_decklink_write_trailer".


        do {
ctx->dlo->GetBufferedVideoFrameCount();
            av_log(avctx, AV_LOG_DEBUG, "ff_decklink_write_trailer 
frames buffered: %d\n", buffered);

        } while (buffered > 1);

A conditional wait would obviously be the proper solution, but my 
point was to test my observations.


Another less serious problem I have observed is that if less than 60 
frames are buffered to begin with, playback is not even begun. This 
would require in the function "ff_decklink_write_trailer" that if 
"playback_started" is not true, but frames are buffered, than 
playback is started anyway.


Well, you can always use a shorter preroll interval, because buffered 
frames = 2 * preroll, but never more than 60. This way you can have a 
shorter supported minumum clip size and also less lost frames at the end.


Otherwise you are right, the code works as you described. Nobody cared 
so far, in fact, when "writing the trailer" it might be more desirable 
for the user to abort playback ASAP. It is rare that people play a 
single clip out with decklink, I am curious what is your use case.


Anyway, a patch can be made to fix the issues you reported, although I 
probably would insist on a user option to control write_trailer 
behaviour (quit immediately or wait for playout of buffered frames).


You probably have to factorize the code which starts the playback to a 
function and call that from write_trailer as well if needed. Since you 
know the frame rate it is enough to query buffered farmes once and 
sleep for (buffered + 1) * frame_time seconds before disabling the 
video/audio output.


Regards,
Marton 



In my original development, I found that the Decklink device was very 
picky about getting too few preroll frames. As recall, less than 30 
would produce erratic behavior (dropped frames). Perhaps that has 
changed in newer releases of the SDK.


Either way, why would the default behavior ever be to drop even a single 
frame unless specifically aborted by the user? Seems sloppy, and when 
half second fades at the end are dropped it makes the video ending 
abrupt. The current driver would be useful only in a scenario where 
ffmpeg was used to generate a final HD-SDI video stream. The tool is 
much more capable than that.


Deron

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


Re: [FFmpeg-devel] [PATCH]lavc/amrwbdec: Do not ignore NO_DATA frames

2019-01-30 Thread Paul B Mahol
On 1/30/19, Carl Eugen Hoyos  wrote:
> 2019-01-29 22:47 GMT+01:00, Carl Eugen Hoyos :
>> Hi!
>>
>> Attached patch fixes decoding NO_DATA amr-wb frames.
>
> Now with patch.
>
> Carl Eugen
>

Are you sure that reference decoder gives complete silence for such frames?

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


Re: [FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread jannis_wth
30.01.19 19:51 James Almer:
> On 1/30/2019 3:43 PM, jannis_wth wrote:
>> Okay, so how about this one?
>> This functionality is important if you don't know the packet size you
>> are feeding. It allows you to reconstruct the size.
> 
> In what scenario you can't know the size of the AVPacket you're feeding
> to avcodec_send_packet().
> 
For example when a mp4 container lost its moov atom.

>>
>> 0001-avcodec-Allow-to-query-number-of-consumed-bytes-usin.patch
>>
>> From bad739e4f5e9e78cc6fa799287d758bf27db8208 Mon Sep 17 00:00:00 2001
>> From: user 
>> Date: Wed, 30 Jan 2019 19:33:08 +0100
>> Subject: [PATCH 1/1] avcodec: Allow to query number of consumed bytes using
>>  the new API
>>
>> Currently the only way to get the number of consumed bytes is by using
>> the deprecated decode_audio4() API. This patch adds a simple function
>> to fix this.
>>
>> Signed-off-by: Jannis Kambs 
>> ---
>>  libavcodec/avcodec.h | 9 +
>>  libavcodec/utils.c   | 5 +
>>  2 files changed, 14 insertions(+)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index f554c53f0e..43bf84e58b 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -5714,6 +5714,15 @@ int av_get_audio_frame_duration(AVCodecContext 
>> *avctx, int frame_bytes);
>>   */
>>  int av_get_audio_frame_duration2(AVCodecParameters *par, int frame_bytes);
>>  
>> +/**
>> + * This function allows to get the number of remaining bytes after
>> + * avcodec_receive_frame() has been called.
>> + *
>> + * @param avctx the codec context
>> + * @return number of remaining bytes from the input AVPacket.
>> + */
>> +int avcodec_get_remaining_size(AVCodecContext *avctx);
>> +
>>  #if FF_API_OLD_BSF
>>  typedef struct AVBitStreamFilterContext {
>>  void *priv_data;
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index d519b16092..638154f974 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -1727,6 +1727,11 @@ int av_get_audio_frame_duration2(AVCodecParameters 
>> *par, int frame_bytes)
>>  frame_bytes);
>>  }
>>  
>> +int avcodec_get_remaining_size(AVCodecContext *avctx)
>> +{
>> +return avctx->internal->ds.in_pkt->size;
> 
> This is only used by decoders implementing the AVCodec.decode()
> callback. It will always be zero for any decoder using
> AVCodec.receive_frame(), like Bink Audio, libdav1d, etc.

Well, what would be the correct way then?
Or is there no good way and thus you won't accept such interface?

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


Re: [FFmpeg-devel] [PATCH] avformat/tee : Pass standards compliance value to slave muxers as well

2019-01-30 Thread Nicolas George
Karthick J (12019-01-30):
> ---
>  libavformat/tee.c | 1 +
>  1 file changed, 1 insertion(+)

Pushed, thanks.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread James Almer
On 1/30/2019 3:43 PM, jannis_wth wrote:
> 30.01.19 18:30 - James Almer:
>> On 1/30/2019 2:19 PM, Nicolas George wrote:
>>> James Almer (12019-01-30):
 compat_decode_consumed doesn't look to have any significance when using
 the new API. It's working as an accumulator for all bytes consumed from
 all frames and it's never cleared (sounds like a potential overflow for
 that matter in long lasting decoding scenarios like streaming), whereas
 for the old API it's effectively keeping track of bytes consumed in one
 frame, and cleared after every avcodec_decode_* call. So I guess that's
 why you're clearing it here even though this function should by no means
 do that.

 Clearing compat_decode_consumed for the new API should be done somewhere
 in decode.c where it doesn't break the logic for the old API, regardless
 of using the AVCodec.decode() or AVCodec.receive_frame() callbacks.
>>> I have reservations about adding an API to query something that is
>>> considered deprecated.
>>>
>>> Regards,
>> I also don't think this is too good of an idea. The new API always
>> consumes and takes ownership of the full packet when you feed it to
>> avcodec_send_packet(), even if it ends up splitting it internally in
>> order to return more than one frame, so in theory just keeping track of
>> what you feed to it in your own code is enough, regardless of how many
>> frames avcodec_receive_frame() gives you back.
>>
>> What this function would do for some decoders however is giving the API
>> user knowledge of said internal splitting after each
>> avcodec_receive_frame() call (an example being VP9).
>> If that's useful or not for the user, i don't know.
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> Okay, so how about this one?
> This functionality is important if you don't know the packet size you
> are feeding. It allows you to reconstruct the size.

In what scenario you can't know the size of the AVPacket you're feeding
to avcodec_send_packet().

> 
> 
> 0001-avcodec-Allow-to-query-number-of-consumed-bytes-usin.patch
> 
> From bad739e4f5e9e78cc6fa799287d758bf27db8208 Mon Sep 17 00:00:00 2001
> From: user 
> Date: Wed, 30 Jan 2019 19:33:08 +0100
> Subject: [PATCH 1/1] avcodec: Allow to query number of consumed bytes using
>  the new API
> 
> Currently the only way to get the number of consumed bytes is by using
> the deprecated decode_audio4() API. This patch adds a simple function
> to fix this.
> 
> Signed-off-by: Jannis Kambs 
> ---
>  libavcodec/avcodec.h | 9 +
>  libavcodec/utils.c   | 5 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index f554c53f0e..43bf84e58b 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -5714,6 +5714,15 @@ int av_get_audio_frame_duration(AVCodecContext *avctx, 
> int frame_bytes);
>   */
>  int av_get_audio_frame_duration2(AVCodecParameters *par, int frame_bytes);
>  
> +/**
> + * This function allows to get the number of remaining bytes after
> + * avcodec_receive_frame() has been called.
> + *
> + * @param avctx the codec context
> + * @return number of remaining bytes from the input AVPacket.
> + */
> +int avcodec_get_remaining_size(AVCodecContext *avctx);
> +
>  #if FF_API_OLD_BSF
>  typedef struct AVBitStreamFilterContext {
>  void *priv_data;
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index d519b16092..638154f974 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -1727,6 +1727,11 @@ int av_get_audio_frame_duration2(AVCodecParameters 
> *par, int frame_bytes)
>  frame_bytes);
>  }
>  
> +int avcodec_get_remaining_size(AVCodecContext *avctx)
> +{
> +return avctx->internal->ds.in_pkt->size;

This is only used by decoders implementing the AVCodec.decode()
callback. It will always be zero for any decoder using
AVCodec.receive_frame(), like Bink Audio, libdav1d, etc.

For that matter, You sent this message to me alone. Make sure to send it
to the mailing list when you reply.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avformat/mov: fix sidx size being doubled in offset.

2019-01-30 Thread no pls
From: mptcultist 

fixes an issue where if the video size was very specific, ffmpeg would hang 
from not filling the sidx_pts for all streams, due to not reading the last sidx 
lump. for #7572

---
libavformat/mov.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 9b9739f788..c222582886 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4933,7 +4933,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, 
MOVAtom atom)

static int mov_read_sidx(MOVContext *c, AVIOContext *pb, MOVAtom atom)
{
-int64_t offset = avio_tell(pb) + atom.size, pts, timestamp;
+int64_t offset = avio_tell(pb), pts, timestamp;
uint8_t version;
unsigned i, j, track_id, item_count;
AVStream *st = NULL;
-- 
2.20.1

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


Re: [FFmpeg-devel] configure: Fix decklink license dependency

2019-01-30 Thread Elliott Balsley

> The BlackMagic Decklink drivers are not system libraries.

Thank you for clarifying about the headers vs the library, I was confused about 
that at first.  The GPL definition of system library is hard to understand, but 
I was mostly basing my opinion on this discussion of NVENC.  It relies on the 
proprietary Nvidia driver, which you agreed was a system library.  How is the 
Decklink driver different?

http://ffmpeg.org/pipermail/ffmpeg-devel/2016-April/193549.html 

configure: Don't require nonfree for nvenc
pr 26 19:21:37 CEST 2016
Timo Rothenpieler  rothenpieler.org> writes:

> As the nvEncodeApi.h header is now MIT licensed, this can be dropped.
> The loaded CUDA and NVENC libraries are part of the nvidia driver, and
> thus count as system libraries.

If the explanation is correct, the patch is ok imo.

Thank you, Carl Eugen

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


Re: [FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread James Almer
On 1/30/2019 2:19 PM, Nicolas George wrote:
> James Almer (12019-01-30):
>> compat_decode_consumed doesn't look to have any significance when using
>> the new API. It's working as an accumulator for all bytes consumed from
>> all frames and it's never cleared (sounds like a potential overflow for
>> that matter in long lasting decoding scenarios like streaming), whereas
>> for the old API it's effectively keeping track of bytes consumed in one
>> frame, and cleared after every avcodec_decode_* call. So I guess that's
>> why you're clearing it here even though this function should by no means
>> do that.
>>
>> Clearing compat_decode_consumed for the new API should be done somewhere
>> in decode.c where it doesn't break the logic for the old API, regardless
>> of using the AVCodec.decode() or AVCodec.receive_frame() callbacks.
> 
> I have reservations about adding an API to query something that is
> considered deprecated.
> 
> Regards,

I also don't think this is too good of an idea. The new API always
consumes and takes ownership of the full packet when you feed it to
avcodec_send_packet(), even if it ends up splitting it internally in
order to return more than one frame, so in theory just keeping track of
what you feed to it in your own code is enough, regardless of how many
frames avcodec_receive_frame() gives you back.

What this function would do for some decoders however is giving the API
user knowledge of said internal splitting after each
avcodec_receive_frame() call (an example being VP9).
If that's useful or not for the user, i don't know.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread Nicolas George
James Almer (12019-01-30):
> compat_decode_consumed doesn't look to have any significance when using
> the new API. It's working as an accumulator for all bytes consumed from
> all frames and it's never cleared (sounds like a potential overflow for
> that matter in long lasting decoding scenarios like streaming), whereas
> for the old API it's effectively keeping track of bytes consumed in one
> frame, and cleared after every avcodec_decode_* call. So I guess that's
> why you're clearing it here even though this function should by no means
> do that.
> 
> Clearing compat_decode_consumed for the new API should be done somewhere
> in decode.c where it doesn't break the logic for the old API, regardless
> of using the AVCodec.decode() or AVCodec.receive_frame() callbacks.

I have reservations about adding an API to query something that is
considered deprecated.

Regards,

-- 
  Nicolas George


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


Re: [FFmpeg-devel] [PATCH]lavf:Constify AVInputFormat pointer

2019-01-30 Thread James Almer
On 1/30/2019 12:33 PM, Michael Niedermayer wrote:
> On Sun, Jan 27, 2019 at 10:38:13PM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Attached patch was requested in ticket #7220 iiuc.
>>
>> Please review, Carl Eugen
> 
>>  avformat.h |8 
>>  format.c   |4 ++--
>>  hls.c  |2 +-
>>  utils.c|4 ++--
>>  4 files changed, 9 insertions(+), 9 deletions(-)
>> 0a59a10c224ba092d8b7b61e3cac78a93c23bee2  
>> 0001-lavf-Constify-AVInputFormat-pointer.patch
>> From a713b58767e8d77b641d1b87e68de11c176fd454 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos 
>> Date: Sun, 27 Jan 2019 22:35:51 +0100
>> Subject: [PATCH] lavf: Constify AVInputFormat pointer.
> 
> there is some tiny chance this could break some user apps build i think
> so if done strictly correct it would need to be delayed to the next
> bump i think

We have tied constifying (or the opposite) to major bumps before, so i
agree.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] avcodec: Allow to query number of consumed bytes using the new API

2019-01-30 Thread jannis_wth
This patch fixes the issue discussed in ticket #7708.
>From 756b3b59ac491bdbf01de4f399f5eeb74db8861a Mon Sep 17 00:00:00 2001
From: user 
Date: Wed, 30 Jan 2019 16:48:58 +0100
Subject: [PATCH 1/1] avcodec: Allow to query number of consumed bytes using
 the new API

Currently the only way to get the number of consumed bytes is by using
the deprecated decode_audio4() API. This patch adds a simple function
to fix this.

Signed-off-by: Jannis Kambs 
---
 libavcodec/avcodec.h | 9 +
 libavcodec/utils.c   | 7 +++
 2 files changed, 16 insertions(+)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index f554c53f0e..54f48754e4 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -5714,6 +5714,15 @@ int av_get_audio_frame_duration(AVCodecContext *avctx, int frame_bytes);
  */
 int av_get_audio_frame_duration2(AVCodecParameters *par, int frame_bytes);
 
+/**
+ * This function allows to get the number of consumed bytes, analogous to the
+ * old decode API. Call it after avcodec_receive_frame().
+ *
+ * @param avctx the codec context
+ * @return number of bytes consumed from the input AVPacket.
+ */
+int av_get_num_consumed(AVCodecContext *avctx);
+
 #if FF_API_OLD_BSF
 typedef struct AVBitStreamFilterContext {
 void *priv_data;
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index d519b16092..a3cb8fef3f 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1727,6 +1727,13 @@ int av_get_audio_frame_duration2(AVCodecParameters *par, int frame_bytes)
 frame_bytes);
 }
 
+int av_get_num_consumed(AVCodecContext *avctx)
+{
+int ret = avctx->internal->compat_decode_consumed;
+avctx->internal->compat_decode_consumed = 0;
+return ret;
+}
+
 #if !HAVE_THREADS
 int ff_thread_init(AVCodecContext *s)
 {
-- 
2.11.0

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


Re: [FFmpeg-devel] [PATCH]lavf:Constify AVInputFormat pointer

2019-01-30 Thread Michael Niedermayer
On Sun, Jan 27, 2019 at 10:38:13PM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Attached patch was requested in ticket #7220 iiuc.
> 
> Please review, Carl Eugen

>  avformat.h |8 
>  format.c   |4 ++--
>  hls.c  |2 +-
>  utils.c|4 ++--
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 0a59a10c224ba092d8b7b61e3cac78a93c23bee2  
> 0001-lavf-Constify-AVInputFormat-pointer.patch
> From a713b58767e8d77b641d1b87e68de11c176fd454 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Sun, 27 Jan 2019 22:35:51 +0100
> Subject: [PATCH] lavf: Constify AVInputFormat pointer.

there is some tiny chance this could break some user apps build i think
so if done strictly correct it would need to be delayed to the next
bump i think

[...]

-- 
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: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] Icecast protocol implementation doesn't parse "/" as a valid mountpoint

2019-01-30 Thread c0re
On Tue, 2019-01-22 at 22:06 +0100, Marvin Scholz wrote:
> On 22 Jan 2019, at 20:29, c0re wrote:
> 
> > Hi all,
> > 
> > Today as I tried to implement transcoding a videofeed to an
> > icecast
> > audio-only stream I have encountered some behavior that, in my
> > opionion, leaves room for improvement.
> > 
> > The setup used in this scenario consists of 
> > * an Open Broadcaster Studio (OBS) instance sending a video/audio
> > feed
> > * an nginx endpoint to receive and re-stream said feed to various
> > hosts (including localhost) 
> > * and lastly an AzuraCast 
> > (https://github.com/AzuraCast/AzuraCast)instance which employs 
> > Icecast2.4 for audio broadcasting.
> > 
> > AzuraCast allows streamer/DJ accounts to live-broadcast to any of
> > its
> > radio-stations by connecting through a Broadcasting software of
> > your
> > choice (noteworthy mentions are BUTT or Mixxx) to a dedicated port
> > (e.g. 8005) and a static[1] mountpoint "/" and providing
> > credentials.
> 
> This sounds quite weird, it would make a lot more sense to just use
> the
> same port and different mountpoints for this.
> The lack of multiple mountpoints is an old SHOUTcast limitation and
> Icecast does not suffer from it.

Having looked further into the issue I can now say that the behavior
is derived from Liquidsoaps' handling of so-called harbors.
If you google around a bit you'll see that the trailing slash (harbor
named = "") is quite a popular scheme around there. 
So indeed the issue isn't _exactly_ icecast specific. 

> > 
> > [1]static as in there is no easy way for me to change this design
> > consideration)
> > 
> > The way in which the icecast protocol is currently implemented in
> > ffmpeg, it will check for characters after the trailing slash and
> > assume this as a valid mountpoint.
> > 
> > A trailing slash by itself will always be discarded with the
> > following
> > error: 
> > 
> > "No mountpoint (path) specified!" 
> > 
> > see: https://ffmpeg.org/doxygen/2.5/icecast_8c_source.html 
> > [lines 157-161]
> > 
> > so icecast://source:pass@host:port/ is considered invalid by
> > ffmpeg (I
> > cannot think of a quick and dirty workaround either).
> 
> Hi,
> yes indeed Icecast does allow a mountpoint of /, but I would rather
> not
> allow that in the FFMpeg Icecast protocol helper, as IMO using / for
> a
> mountpoint is not a good idea. Users could accidentally use / as
> mount-
> point without this check and would get confused why it does not work
> out
> of the box (as by default / is an internal redirect to the status
> page).
> 
> I wonder why AzuraCast decides to use / as mountpoint and why it
> does
> not even offer you a way to change it.
> 
> Regards,
> Marvin Scholz

I filed an issue with AzuraCast which now has been resolved and an
implementation for custom named Liquidsoap harbors (which translate to
icecast streaming mount-points) is now fully implemented. 

For what it's worth I also gave the ol' hack a go and patched
icecast.c and compiled from source.
Lo and behold... the change of a single character delivered the
desired result without any apparent drawbacks. 


==

diff --git a/libavformat/icecast.c b/libavformat/icecast.c
index c93b06b553..aca043f59b 100644
--- a/libavformat/icecast.c
+++ b/libavformat/icecast.c
@@ -155,7 +155,7 @@ static int icecast_open(URLContext *h, const char
*uri, int flags)
  s->pass ? s->pass : "");
 
 // Check for mountpoint (path)
-if (!path[0] || strcmp(path, "/") == 0) {
+if (!path[0] || strcmp(path, "") == 0) {
 av_log(h, AV_LOG_ERROR, "No mountpoint (path) specified!\n");
 ret = AVERROR(EIO);
 goto cleanup;

==

I leave it up to anyone whether or not that is the more sane
implementation (according to spec) or not. :)

Kind Regards
c0re

> > 
> > To my understanding assigning a lone slash as a mountpoint name
> > isn't
> > considered invalid by the IceCast specification and therefore I
> > suggest that it would be an enhancement to lift this current
> > limitation on the way that mountpoints are parsed in an
> > ffmpeg+icecast
> > instruction.
> > 
> > kind regards,
> > c0re___
> > 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

signature.asc
Description: This is a digitally signed message part
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] lavfi/vf_nlmeans: Remove the pdiff_lut_scale to improve the performance

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 11:56 GMT+01:00, Jun Zhao :
> Remove the pdiff_lut_scale in nlmeans

This sentence is very misleading.

> and this change will avoid
> using pdiff_lut_scale in the exp table search in nlmean_slice, it's will
> improve the performance about 12%.

Please mention in the commit message that you increase
the context size including the amount (and probably remove
the part above that you "remove" something from the context).

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


Re: [FFmpeg-devel] configure: Fix decklink license dependency

2019-01-30 Thread Carl Eugen Hoyos
2019-01-30 1:53 GMT+01:00, Elliott Balsley :
>
>> It is not Blackmagic who does not want you to distribute FFmpeg
>> with their (proprietary) library, it is us who do not allow distribution
>> of FFmpeg binaries that link against Blackmagic libraries.

The wording here was not ideal:
FFmpeg is by default licensed under LGPL and the LGPL allows to
combine open-source binaries with proprietary binaries.
So it is less "us" (the FFmpeg developers) and more the developers
of other open source projects licensed under the GPL (like especially
x264) who do not allow distribution of binaries linked against
proprietary drivers.

> When you say proprietary library, do you mean the Desktop Video
> driver?  I believe this qualifies as a system library

The BlackMagic Decklink drivers are not system libraries.

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


[FFmpeg-devel] [PATCH] avformat/tee : Pass standards compliance value to slave muxers as well

2019-01-30 Thread Karthick J
---
 libavformat/tee.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/tee.c b/libavformat/tee.c
index ef3b113a47..89a4ceb280 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -236,6 +236,7 @@ static int open_slave(AVFormatContext *avf, char *slave, 
TeeSlave *tee_slave)
 avf2->io_close = avf->io_close;
 avf2->interrupt_callback = avf->interrupt_callback;
 avf2->flags = avf->flags;
+avf2->strict_std_compliance = avf->strict_std_compliance;
 
 tee_slave->stream_map = av_calloc(avf->nb_streams, 
sizeof(*tee_slave->stream_map));
 if (!tee_slave->stream_map) {
-- 
2.17.1 (Apple Git-112)

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


Re: [FFmpeg-devel] [PATCH]lavc/amrwbdec: Do not ignore NO_DATA frames

2019-01-30 Thread Paul B Mahol
On 1/29/19, Carl Eugen Hoyos  wrote:
> Hi!
>
> Attached patch fixes decoding NO_DATA amr-wb frames.
>
> Please comment, Carl Eugen
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

You forgot to attach pach.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] lavfi/vf_nlmeans: Remove the pdiff_lut_scale to improve the performance

2019-01-30 Thread Jun Zhao
Remove the pdiff_lut_scale in nlmeans, and this change will avoid
using pdiff_lut_scale in the exp table search in nlmean_slice, it's will
improve the performance about 12%.

Use the profiling cmd like:
perf stat -a -d -r 5 ./ffmpeg -i input -an -vf nlmeans=s=30 -vframes 10 \
-f null /dev/null

without this change:
when s=1.0(default value) 63s
 s=30.0   72s
after this change:
 s=1.0(default value) 56s
 s=30.0   63s

Signed-off-by: Jun Zhao 
---
 libavfilter/vf_nlmeans.c |   12 
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/libavfilter/vf_nlmeans.c b/libavfilter/vf_nlmeans.c
index 82e779c..72eb819 100644
--- a/libavfilter/vf_nlmeans.c
+++ b/libavfilter/vf_nlmeans.c
@@ -43,8 +43,7 @@ struct weighted_avg {
 float sum;
 };
 
-#define WEIGHT_LUT_NBITS 9
-#define WEIGHT_LUT_SIZE  (1<  300 * 300 * log(255)
 
 typedef struct NLMeansContext {
 const AVClass *class;
@@ -63,7 +62,6 @@ typedef struct NLMeansContext {
 struct weighted_avg *wa;// weighted average of every 
pixel
 ptrdiff_t wa_linesize;  // linesize for wa in struct 
size unit
 float weight_lut[WEIGHT_LUT_SIZE];  // lookup table mapping 
(scaled) patch differences to their associated weights
-float pdiff_lut_scale;  // scale factor for patch 
differences before looking into the LUT
 uint32_t max_meaningful_diff;   // maximum difference 
considered (if the patch difference is too high we ignore the pixel)
 NLMeansDSPContext dsp;
 } NLMeansContext;
@@ -401,8 +399,7 @@ static int nlmeans_slice(AVFilterContext *ctx, void *arg, 
int jobnr, int nb_jobs
 const uint32_t patch_diff_sq = e - d - b + a;
 
 if (patch_diff_sq < s->max_meaningful_diff) {
-const unsigned weight_lut_idx = patch_diff_sq * 
s->pdiff_lut_scale;
-const float weight = s->weight_lut[weight_lut_idx]; // 
exp(-patch_diff_sq * s->pdiff_scale)
+const float weight = s->weight_lut[patch_diff_sq]; // 
exp(-patch_diff_sq * s->pdiff_scale)
 wa[x].total_weight += weight;
 wa[x].sum += weight * src[x];
 }
@@ -527,10 +524,9 @@ static av_cold int init(AVFilterContext *ctx)
 
 s->pdiff_scale = 1. / (h * h);
 s->max_meaningful_diff = -log(1/255.) / s->pdiff_scale;
-s->pdiff_lut_scale = 1./s->max_meaningful_diff * WEIGHT_LUT_SIZE;
-av_assert0((s->max_meaningful_diff - 1) * s->pdiff_lut_scale < 
FF_ARRAY_ELEMS(s->weight_lut));
+av_assert0((s->max_meaningful_diff - 1) < FF_ARRAY_ELEMS(s->weight_lut));
 for (i = 0; i < WEIGHT_LUT_SIZE; i++)
-s->weight_lut[i] = exp(-i / s->pdiff_lut_scale * s->pdiff_scale);
+s->weight_lut[i] = exp(-i * s->pdiff_scale);
 
 CHECK_ODD_FIELD(research_size,   "Luma research window");
 CHECK_ODD_FIELD(patch_size,  "Luma patch");
-- 
1.7.1

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


[FFmpeg-devel] [RFC] lavfi: add scale_opencl filter.

2019-01-30 Thread Ruiling Song
Signed-off-by: Ruiling Song 
---
This patch depends on the colorspace patchset I sent before
(https://patchwork.ffmpeg.org/patch/11820/)
Although I am still working on some minor functionality,
hope somebody could give some comments about the overall design.

Ruiling

 configure |   1 +
 libavfilter/Makefile  |   2 +
 libavfilter/allfilters.c  |   1 +
 libavfilter/opencl/scale.cl   | 252 
 libavfilter/opencl_source.h   |   1 +
 libavfilter/vf_scale_opencl.c | 682 ++
 6 files changed, 939 insertions(+)
 create mode 100644 libavfilter/opencl/scale.cl
 create mode 100644 libavfilter/vf_scale_opencl.c

diff --git a/configure b/configure
index ec8f70d..5640137 100755
--- a/configure
+++ b/configure
@@ -3450,6 +3450,7 @@ rubberband_filter_deps="librubberband"
 sab_filter_deps="gpl swscale"
 scale2ref_filter_deps="swscale"
 scale_filter_deps="swscale"
+scale_opencl_filter_deps="opencl"
 scale_qsv_filter_deps="libmfx"
 select_filter_select="scene_sad"
 sharpness_vaapi_filter_deps="vaapi"
diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index bc642ac..9de7d44 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -343,6 +343,8 @@ OBJS-$(CONFIG_SCALE_FILTER)  += vf_scale.o 
scale.o
 OBJS-$(CONFIG_SCALE_CUDA_FILTER) += vf_scale_cuda.o 
vf_scale_cuda.ptx.o \
 cuda_check.o
 OBJS-$(CONFIG_SCALE_NPP_FILTER)  += vf_scale_npp.o scale.o 
cuda_check.o
+OBJS-$(CONFIG_SCALE_OPENCL_FILTER)   += vf_scale_opencl.o opencl.o \
+opencl/scale.o
 OBJS-$(CONFIG_SCALE_QSV_FILTER)  += vf_scale_qsv.o
 OBJS-$(CONFIG_SCALE_VAAPI_FILTER)+= vf_scale_vaapi.o scale.o 
vaapi_vpp.o
 OBJS-$(CONFIG_SCALE2REF_FILTER)  += vf_scale.o scale.o
diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
index c51ae0f..5708d16 100644
--- a/libavfilter/allfilters.c
+++ b/libavfilter/allfilters.c
@@ -325,6 +325,7 @@ extern AVFilter ff_vf_sab;
 extern AVFilter ff_vf_scale;
 extern AVFilter ff_vf_scale_cuda;
 extern AVFilter ff_vf_scale_npp;
+extern AVFilter ff_vf_scale_opencl;
 extern AVFilter ff_vf_scale_qsv;
 extern AVFilter ff_vf_scale_vaapi;
 extern AVFilter ff_vf_scale2ref;
diff --git a/libavfilter/opencl/scale.cl b/libavfilter/opencl/scale.cl
new file mode 100644
index 000..5d3deda
--- /dev/null
+++ b/libavfilter/opencl/scale.cl
@@ -0,0 +1,252 @@
+/*
+ * 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
+ */
+
+extern float3 yuv2rgb(float, float, float);
+extern float3 rgb2yuv(float, float, float);
+
+const sampler_t sampler_nearest = (CLK_NORMALIZED_COORDS_FALSE |
+   CLK_ADDRESS_CLAMP |
+   CLK_FILTER_NEAREST);
+
+const sampler_t sampler_linear = (CLK_NORMALIZED_COORDS_FALSE |
+  CLK_ADDRESS_CLAMP |
+  CLK_FILTER_LINEAR);
+
+float4 neighbor(image2d_t img, float vscale,
+float hscale, int x, int y,
+__constant float *coff_x,
+__constant float *coff_y,
+int2 filter_size)
+{
+float xi = ((float)x + 0.5f) * hscale;
+float yi = ((float)y + 0.5f) * vscale;
+
+return read_imagef(img, sampler_nearest, (float2)(xi, yi));
+}
+
+float4 bilinear(image2d_t img, float vscale,
+float hscale, int x, int y,
+__constant float *coff_x,
+__constant float *coff_y,
+int2 filter_size)
+{
+float xi = ((float)x + 0.5f) * hscale;
+float yi = ((float)y + 0.5f) * vscale;
+
+return read_imagef(img, sampler_linear, (float2)(xi, yi));
+}
+
+float4 generic_filter(image2d_t img, float vscale, float hscale, int x, int y,
+  __constant float *coff_x, __constant float *coff_y,
+  int2 filter_size)
+{
+int2 dst_pos = (int2)(x, y);
+float2 src_coord = (convert_float2(dst_pos) + 0.5f) *
+   (float2)(hscale, vscale);
+int2 src_pos = convert_int2(floor(src_coord - 0.5f));
+
+float4 color = 0.0f;
+for (int i = 0; i < filter_size.y; ++i) {
+