Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions

2020-09-26 Thread Mark Reid
On Mon, Sep 14, 2020 at 6:31 PM Mark Reid  wrote:

>
>
> On Mon, Sep 14, 2020 at 2:44 PM Michael Niedermayer 
> wrote:
>
>> On Sun, Sep 13, 2020 at 04:04:42PM -0700, Mark Reid wrote:
>> > On Sun, Sep 13, 2020 at 8:55 AM Michael Niedermayer
>> 
>> > wrote:
>> >
>> > > On Sat, Sep 12, 2020 at 02:07:14AM -0700, mindm...@gmail.com wrote:
>> > > > From: Mark Reid 
>> > > >
>> > > > ---
>> > > >  libswscale/input.c  | 12 +---
>> > > >  tests/ref/fate/filter-pixfmts-scale |  8 
>> > > >  2 files changed, 9 insertions(+), 11 deletions(-)
>> > >
>> > > Can you provide some tests that show that this is better ?
>> > > Iam asking that because some of the numbers in some of the code
>> > > (i dont remember which) where tuned to give more accurate overall
>> results
>> > >
>> > > an example for tests would be converting from A->B->A then compare to
>> the
>> > > orig
>> > >
>> > > thx
>> > >
>> > >
>> > Hopefully i can explain this clearly!
>> >
>> > It's easier to see the error if you run a black image through the old
>> > conversion.
>> > zero values don't get mapped to zero. (attached sample image)
>> >
>> > ffmpeg -i 4x4_zero.exr -pix_fmt rgb48le -f rawvideo 4x4_zero.rawvideo
>> > The image should be rgb 0, 0, 0  everywhere but instead it's 353, 0, 407
>> >
>> >
>> > I think this is a error in fixed point rounding, the issue is basically
>> > boils down to
>> >
>> > 128 << 8 != 257 << 7
>> > and
>> > 16 << 8  != 33 << 7
>> >
>> > https://en.wikipedia.org/wiki/YUV#Studio_swing_for_BT.601
>> > the 8 bit rgb to yuv formula is
>> >
>> > Y = ry*r + gy*g + by*b  + 16
>> > U = ru*r + gu*g + bu*b + 128
>> > V = rv*r + gv*g + bv*b + 128
>> >
>> > I think the studio swing offsets at the end are calculated wrong in the
>> old
>> > code.
>> > (257 << (RGB2YUV_SHIFT + bpc - 9)))
>> > 257 is correct for 8 bit rounding but not for 16-bit.
>> >
>> > the 257 i believe is from (128 << 1) + 1
>> > the +1 is for rounding
>> >
>> > for rounding 16-bit (128 << 9) + 1 = 0x10001
>> >
>> > therefore I think the correct rounding any bit depth with the old
>> formula
>> > would be (untested)
>> > (((128 << (bpc - 7)) + 1) << (RGB2YUV_SHIFT-1) )
>> >
>> > I just simplified it to
>> > (0x10001 << (RGB2YUV_SHIFT - 1))
>> >
>> > The rgb48ToUV and rgb48ToY funcs in input.c use the formula I'm using.
>>
>> You quite possibly are correct, can you test that this actually works
>> out. The test sample only covers 1 color (black)
>> a testsample covering a wide range of the color cube would be more
>> convincing that this change is needed and sufficient to fix this.
>>
>> thx
>>
>
> I wrote a small python script to compare the raw gbrpf32le images if that
> works? I attached it and also a more colorful test pattern.
>
> it runs these two commands and compares the 2 raw float images
> ffmpeg -y -i test_pattern.exr -f rawvideo original.gbrpf32le
> ffmpeg -y -i test_pattern.exr -vf
> format=pix_fmts=rgb48le,format=pix_fmts=gbrpf32le -f rawvideo
> converted.gbrpf32le
>
> python gbrpf32le_diff.py test_pattern.exr
>
> without patch:
> avg error: 237.445495855
> min error: 0.0
> max error: 468.399102688
>
> with patch:
> avg error: 15.9312244829
> min error: 0.0
> max error: 69.467689991
>
>
> These are floating points scaled to 16-bit values.
> Even with my patch, I'm kinda disturbed how much error there is.
>

ping
I re-wrote the python script as a c swscale test, if that helps
replicate my results. attached is patch for that.
it generates an image of random float values and does the
conversion/comparison .

before patch:
gbrpf32le -> yuva444p16le -> gbrpf32le
avg diff: 0.003852
min diff: 0.00
max diff: 0.006638

after patch:
gbrpf32le -> yuva444p16le -> gbrpf32le
avg diff: 0.000125
min diff: 0.00
max diff: 0.000501


>
>
>>
>> [...]
>>
>> --
>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> There will always be a question for which you do not know the correct
>> answer.
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
>


0001-libswscale-tests-add-floatimg_cmp-test.patch
Description: Binary data
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avformat/rtsp: allocate correct max number of pollfds

2020-09-26 Thread Andriy Gelman
From: Andriy Gelman 

There is one general rtsp connection plus two connections per stream (rtp/rtcp).

Signed-off-by: Andriy Gelman 
---
 libavformat/rtsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 5d8491b74b..90f912feb9 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1994,7 +1994,7 @@ static int udp_read_packet(AVFormatContext *s, RTSPStream 
**prtsp_st,
 int *fds = NULL, fdsnum, fdsidx;
 
 if (!p) {
-p = rt->p = av_malloc_array(2 * (rt->nb_rtsp_streams + 1), 
sizeof(struct pollfd));
+p = rt->p = av_malloc_array(2 * rt->nb_rtsp_streams + 1, sizeof(struct 
pollfd));
 if (!p)
 return AVERROR(ENOMEM);
 
-- 
2.27.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 1/2] avformat/rtspdec: add newline in log message

2020-09-26 Thread Andriy Gelman
From: Andriy Gelman 

Signed-off-by: Andriy Gelman 
---
 libavformat/rtspdec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
index dfa29913bf..ef084a8b2b 100644
--- a/libavformat/rtspdec.c
+++ b/libavformat/rtspdec.c
@@ -291,7 +291,7 @@ static int rtsp_read_setup(AVFormatContext *s, char* host, 
char *controlurl)
 AVDictionary *opts = NULL;
 av_dict_set_int(, "buffer_size", rt->buffer_size, 0);
 ff_url_join(url, sizeof(url), "rtp", NULL, host, localport, NULL);
-av_log(s, AV_LOG_TRACE, "Opening: %s", url);
+av_log(s, AV_LOG_TRACE, "Opening: %s\n", url);
 ret = ffurl_open_whitelist(_st->rtp_handle, url, 
AVIO_FLAG_READ_WRITE,
>interrupt_callback, ,
s->protocol_whitelist, 
s->protocol_blacklist, NULL);
@@ -304,7 +304,7 @@ static int rtsp_read_setup(AVFormatContext *s, char* host, 
char *controlurl)
 return ret;
 }
 
-av_log(s, AV_LOG_TRACE, "Listening on: %d",
+av_log(s, AV_LOG_TRACE, "Listening on: %d\n",
 ff_rtp_get_local_rtp_port(rtsp_st->rtp_handle));
 if ((ret = ff_rtsp_open_transport_ctx(s, rtsp_st))) {
 rtsp_send_reply(s, RTSP_STATUS_TRANSPORT, NULL, request.seq);
-- 
2.27.0

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/rtsp: fix infinite loop with udp transport

2020-09-26 Thread Andriy Gelman
On Sun, 27. Sep 00:31, Zhao Zhili wrote:
> Ping for review, thanks.
> 
> > On Sep 9, 2020, at 12:10 AM, Zhao Zhili  wrote:
> > 
> > sender:
> > ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  
> > rtsp://localhost:12345/live.sdp
> > 
> > receiver:
> > ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i 
> > rtsp://localhost:12345/live.sdp -c copy test.mp4

Mention ticket 8840.

> > ---
> > libavformat/rtsp.c| 2 ++
> > libavformat/rtsp.h| 1 +
> > libavformat/rtspdec.c | 2 +-
> > 3 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index 5d8491b74b..597413803f 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -2051,6 +2051,8 @@ static int udp_read_packet(AVFormatContext *s, 
> > RTSPStream **prtsp_st,
> > if ((ret = parse_rtsp_message(s)) < 0) {
> > return ret;
> > }
> > +if (rt->state == RTSP_STATE_TEARDOWN)
> > +return AVERROR_EOF;
> > }
> > #endif
> > } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
> > diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
> > index 54a9a30c16..481cc0c3ce 100644
> > --- a/libavformat/rtsp.h
> > +++ b/libavformat/rtsp.h
> > @@ -198,6 +198,7 @@ enum RTSPClientState {
> > RTSP_STATE_STREAMING, /**< initialized and sending/receiving data */
> > RTSP_STATE_PAUSED,  /**< initialized, but not receiving data */
> > RTSP_STATE_SEEKING, /**< initialized, requesting a seek */
> > +RTSP_STATE_TEARDOWN,/**< initialized, in teardown state */
> > };
> > 
> > /**
> > diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
> > index dfa29913bf..ec786a469a 100644
> > --- a/libavformat/rtspdec.c
> > +++ b/libavformat/rtspdec.c
> > @@ -494,7 +494,7 @@ int ff_rtsp_parse_streaming_commands(AVFormatContext *s)
> >   "Public: ANNOUNCE, PAUSE, SETUP, TEARDOWN, "
> >   "RECORD\r\n", request.seq);
> > } else if (methodcode == TEARDOWN) {
> > -rt->state = RTSP_STATE_IDLE;
> > +rt->state = RTSP_STATE_TEARDOWN;
> > ret   = rtsp_send_reply(s, RTSP_STATUS_OK, NULL , request.seq);
> > }
> > return ret;
> > -- 
> > 2.25.1
> > 

Looks ok to me. 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_delogo: remove duplicated code

2020-09-26 Thread Paul B Mahol
On Sun, Sep 27, 2020 at 12:38:57AM +0800, Zhao Zhili wrote:
> Ping.
> 
> > On Sep 16, 2020, at 1:09 PM, quinkbl...@foxmail.com wrote:
> > 
> > From: Zhao Zhili 
> > 
> > 1. Remove the modification of x, y, w and h parameters since they
> >   are reset by filter_frame.
> > 2. config_input leads to error out when logo area is outside of the
> >   frame, while filter_frame fix the parameters automatically. Make
> >   config_input don't return error to keep the logic consistent.
> > ---

Is this causing overreads? Check with valgrind.

> > libavfilter/vf_delogo.c | 39 +++
> > 1 file changed, 15 insertions(+), 24 deletions(-)
> > 
> > diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
> > index 6069c30163..39f06512fa 100644
> > --- a/libavfilter/vf_delogo.c
> > +++ b/libavfilter/vf_delogo.c
> > @@ -271,11 +271,6 @@ static av_cold int init(AVFilterContext *ctx)
> > av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n",
> >s->x, s->y, s->w, s->h, s->band, s->show);
> > 
> > -s->w += s->band*2;
> > -s->h += s->band*2;
> > -s->x -= s->band;
> > -s->y -= s->band;
> > -
> > return 0;
> > }
> > 
> > @@ -284,10 +279,21 @@ static int config_input(AVFilterLink *inlink)
> > DelogoContext *s = inlink->dst->priv;
> > 
> > /* Check whether the logo area fits in the frame */
> > -if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > 
> > inlink->w ||
> > -s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > 
> > inlink->h) {
> > -av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n");
> > -return AVERROR(EINVAL);
> > +if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > 
> > inlink->w ||
> > +s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > 
> > inlink->h) {
> > +av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame,"
> > +   " logo(x:%d y:%d w:%d h:%d), frame(%dx%d),"
> > +   " auto set the area inside of the frame."
> > +   " Note: x and y must be 1 at least.\n",
> > +   s->x, s->y, s->w, s->h, inlink->w, inlink->h);
> > +if (s->x + (s->band - 1) <= 0)
> > +s->x = 1 + s->band;
> > +if (s->y + (s->band - 1) <= 0)
> > +s->y = 1 + s->band;
> > +if (s->x + s->w - (s->band*2 - 2) > inlink->w)
> > +s->w = inlink->w - s->x - (s->band*2 - 2);
> > +if (s->y + s->h - (s->band*2 - 2) > inlink->h)
> > +s->h = inlink->h - s->y - (s->band*2 - 2);
> > }
> > 
> > return 0;
> > @@ -313,21 +319,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> > *in)
> > s->w = av_expr_eval(s->w_pexpr, s->var_values, s);
> > s->h = av_expr_eval(s->h_pexpr, s->var_values, s);
> > 
> > -if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > 
> > inlink->w ||
> > -s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > 
> > inlink->h) {
> > -av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame,"
> > -   " auto set the area inside of the frame\n");
> > -}
> > -
> > -if (s->x + (s->band - 1) <= 0)
> > -s->x = 1 + s->band;
> > -if (s->y + (s->band - 1) <= 0)
> > -s->y = 1 + s->band;
> > -if (s->x + s->w - (s->band*2 - 2) > inlink->w)
> > -s->w = inlink->w - s->x - (s->band*2 - 2);
> > -if (s->y + s->h - (s->band*2 - 2) > inlink->h)
> > -s->h = inlink->h - s->y - (s->band*2 - 2);
> > -
> > ret = config_input(inlink);
> > if (ret < 0) {
> > av_frame_free();
> > -- 
> > 2.17.1
> > 
> > 
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] avcodec/dxtory: Fix negative shift in dx2_decode_slice_410()

2020-09-26 Thread Michael Niedermayer
On Sat, Sep 26, 2020 at 09:40:43AM +0200, Paul B Mahol wrote:
> On Sat, Sep 26, 2020 at 12:26:35AM +0200, Michael Niedermayer wrote:
> > Fixes: left shift of negative value -768
> > Fixes: 
> > 25574/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXTORY_fuzzer-6012596027916288
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/dxtory.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> lgtm

will apply

thx

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/dxtory: Fix negative shift in dxtory_decode_v1_410()

2020-09-26 Thread Michael Niedermayer
On Sat, Sep 26, 2020 at 09:40:23AM +0200, Paul B Mahol wrote:
> On Sat, Sep 26, 2020 at 12:26:34AM +0200, Michael Niedermayer wrote:
> > Fixes: left shift of negative value -256
> > Fixes: 
> > 25460/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXTORY_fuzzer-5073252341514240
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/dxtory.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> lgtm

will apply

thx

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

No snowflake in an avalanche ever feels responsible. -- Voltaire


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] avcodec/dxtory: Fix get_raw_size() for YUV

2020-09-26 Thread Michael Niedermayer
On Sat, Sep 26, 2020 at 09:39:45AM +0200, Paul B Mahol wrote:
> On Sat, Sep 26, 2020 at 12:26:33AM +0200, Michael Niedermayer wrote:
> > Fixes: out of array read
> > Fixes: 
> > 25455/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXTORY_fuzzer-6327985731534848
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavcodec/dxtory.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> lgtm

will apply

thx

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

Elect your leaders based on what they did after the last election, not
based on what they say before an election.



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/aviobuf: fix checks in ffio_ensure_seekback

2020-09-26 Thread Marton Balint



On Sat, 26 Sep 2020, Michael Niedermayer wrote:


On Sat, Sep 26, 2020 at 12:46:40PM +0200, Marton Balint wrote:



On Sat, 26 Sep 2020, Michael Niedermayer wrote:


On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:

Signed-off-by: Marton Balint 
---
 libavformat/aviobuf.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)


The commit message is too terse. It is not clear from it what the problem
is that is being fixed and how it is fixed.
Also this feels like it fixes multiple issues so it probably should be spit
unless iam missing some interdependancies


It can be seperated to two patches if that is preferred:

1) existing code does not check if the requested seekback buffer is already
read entirely. In this case, nothing has to be done. This is the newly added
if() at the beginning of the patch.

2) the new buf_size is detemined too conservatively, maybe because of the
issue which was fixed in patch 1/3. So we can safely substract 1 more from
the new buffer size. Comparing the new buf_size against filled does not make
a lot of sense to me, that just seems wrong. What makes sense is that we
want to reallocate the buffer if the new buf_size is bigger than the old,
therefore the change in the check.


i would prefer it split yes


Ok, will send new series.








ill also take a look at the "competing" patch, so i dont yet know
how they relate ...




diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 9675425349..aa1d6c0830 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,9 +999,12 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
 int filled = s->buf_end - s->buffer;
 ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;

-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
+if (buf_size <= s->buf_end - s->buf_ptr)
+return 0;
+
+buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;




-if (buf_size < filled || s->seekable || !s->read_packet)
+if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
 return 0;


Did you check that this doesnt interfere with the s->buffer_size reduction
we do when it was larger from probing ?
Its maybe ok, just checking that this was considered


I don't see how that can be affected by this change, so I am not sure what
you mean here. If the new buffer size is bigger than s->orig_buffer_size,
then eventually it will be reduced, it does not seem more complicated than
that to me.


what i meant was that if its reduced then at that point the seekback
gurantee changes relative to it never being reduced.
I think you consider this in your code, i just wanted to double check.


I think that is already ensured by the check in fill_buffer before 
decreaseing the buffer:


if (dst == s->buffer && s->buf_ptr != dst)

This checks that buffer size decrease can only happen when we are writing 
to the start of the buffer and buf_ptr > buffer, so we are flushing 
anyways.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/http: ensure reply code in the range of 100..599

2020-09-26 Thread Zhao Zhili
Ping for review, thanks.

> On Sep 1, 2020, at 12:34 PM, quinkbl...@foxmail.com wrote:
> 
> From: Zhao Zhili 
> 
> ---
> libavformat/http.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 6c39da1a8b..b77bdf1567 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -168,7 +168,7 @@ static const AVOption options[] = {
> { "reconnect_delay_max", "max reconnect delay in seconds after which to 
> give up", OFFSET(reconnect_delay_max), AV_OPT_TYPE_INT, { .i64 = 120 }, 0, 
> UINT_MAX/1000/1000, D },
> { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 
> }, 0, 2, D | E },
> { "resource", "The resource requested by a client", OFFSET(resource), 
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
> -{ "reply_code", "The http status code to return to a client", 
> OFFSET(reply_code), AV_OPT_TYPE_INT, { .i64 = 200}, INT_MIN, 599, E},
> +{ "reply_code", "The http status code to return to a client", 
> OFFSET(reply_code), AV_OPT_TYPE_INT, { .i64 = 200}, 100, 599, E},
> { NULL }
> };
> 
> -- 
> 2.17.1
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avfilter/vf_delogo: remove duplicated code

2020-09-26 Thread Zhao Zhili
Ping.

> On Sep 16, 2020, at 1:09 PM, quinkbl...@foxmail.com wrote:
> 
> From: Zhao Zhili 
> 
> 1. Remove the modification of x, y, w and h parameters since they
>   are reset by filter_frame.
> 2. config_input leads to error out when logo area is outside of the
>   frame, while filter_frame fix the parameters automatically. Make
>   config_input don't return error to keep the logic consistent.
> ---
> libavfilter/vf_delogo.c | 39 +++
> 1 file changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/libavfilter/vf_delogo.c b/libavfilter/vf_delogo.c
> index 6069c30163..39f06512fa 100644
> --- a/libavfilter/vf_delogo.c
> +++ b/libavfilter/vf_delogo.c
> @@ -271,11 +271,6 @@ static av_cold int init(AVFilterContext *ctx)
> av_log(ctx, AV_LOG_VERBOSE, "x:%d y:%d, w:%d h:%d band:%d show:%d\n",
>s->x, s->y, s->w, s->h, s->band, s->show);
> 
> -s->w += s->band*2;
> -s->h += s->band*2;
> -s->x -= s->band;
> -s->y -= s->band;
> -
> return 0;
> }
> 
> @@ -284,10 +279,21 @@ static int config_input(AVFilterLink *inlink)
> DelogoContext *s = inlink->dst->priv;
> 
> /* Check whether the logo area fits in the frame */
> -if (s->x + (s->band - 1) < 0 || s->x + s->w - (s->band*2 - 2) > 
> inlink->w ||
> -s->y + (s->band - 1) < 0 || s->y + s->h - (s->band*2 - 2) > 
> inlink->h) {
> -av_log(s, AV_LOG_ERROR, "Logo area is outside of the frame.\n");
> -return AVERROR(EINVAL);
> +if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > 
> inlink->w ||
> +s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > 
> inlink->h) {
> +av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame,"
> +   " logo(x:%d y:%d w:%d h:%d), frame(%dx%d),"
> +   " auto set the area inside of the frame."
> +   " Note: x and y must be 1 at least.\n",
> +   s->x, s->y, s->w, s->h, inlink->w, inlink->h);
> +if (s->x + (s->band - 1) <= 0)
> +s->x = 1 + s->band;
> +if (s->y + (s->band - 1) <= 0)
> +s->y = 1 + s->band;
> +if (s->x + s->w - (s->band*2 - 2) > inlink->w)
> +s->w = inlink->w - s->x - (s->band*2 - 2);
> +if (s->y + s->h - (s->band*2 - 2) > inlink->h)
> +s->h = inlink->h - s->y - (s->band*2 - 2);
> }
> 
> return 0;
> @@ -313,21 +319,6 @@ static int filter_frame(AVFilterLink *inlink, AVFrame 
> *in)
> s->w = av_expr_eval(s->w_pexpr, s->var_values, s);
> s->h = av_expr_eval(s->h_pexpr, s->var_values, s);
> 
> -if (s->x + (s->band - 1) <= 0 || s->x + s->w - (s->band*2 - 2) > 
> inlink->w ||
> -s->y + (s->band - 1) <= 0 || s->y + s->h - (s->band*2 - 2) > 
> inlink->h) {
> -av_log(s, AV_LOG_WARNING, "Logo area is outside of the frame,"
> -   " auto set the area inside of the frame\n");
> -}
> -
> -if (s->x + (s->band - 1) <= 0)
> -s->x = 1 + s->band;
> -if (s->y + (s->band - 1) <= 0)
> -s->y = 1 + s->band;
> -if (s->x + s->w - (s->band*2 - 2) > inlink->w)
> -s->w = inlink->w - s->x - (s->band*2 - 2);
> -if (s->y + s->h - (s->band*2 - 2) > inlink->h)
> -s->h = inlink->h - s->y - (s->band*2 - 2);
> -
> ret = config_input(inlink);
> if (ret < 0) {
> av_frame_free();
> -- 
> 2.17.1
> 
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/videotoolboxenc: fix align issue

2020-09-26 Thread Zhao Zhili
Ping for the patch set.

> On Aug 27, 2020, at 5:38 AM, quinkbl...@foxmail.com wrote:
> 
> From: Zhao Zhili 
> 
> bool a53_cc is accessed as int:
> src/libavutil/opt.c:129:9: runtime error: store to misaligned
> address 0x7fbf454121a3 for type 'int', which requires 4 byte alignment
> ---
> libavcodec/videotoolboxenc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
> index e89cfaeed8..988782f10d 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -226,7 +226,9 @@ typedef struct VTEncContext {
> bool flushing;
> bool has_b_frames;
> bool warned_color_range;
> -bool a53_cc;
> +
> +/* can't be bool type since AVOption will access it as int */
> +int a53_cc;
> } VTEncContext;
> 
> static int vtenc_populate_extradata(AVCodecContext   *avctx,
> -- 
> 2.28.0
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2 1/2] avformat/rtsp: fix infinite loop with udp transport

2020-09-26 Thread Zhao Zhili
Ping for review, thanks.

> On Sep 9, 2020, at 12:10 AM, Zhao Zhili  wrote:
> 
> sender:
> ./ffmpeg -i test.mp4 -c copy -f rtsp -rtsp_transport udp  
> rtsp://localhost:12345/live.sdp
> 
> receiver:
> ./ffmpeg_g -y -rtsp_flags listen -timeout 100 -i 
> rtsp://localhost:12345/live.sdp -c copy test.mp4
> ---
> libavformat/rtsp.c| 2 ++
> libavformat/rtsp.h| 1 +
> libavformat/rtspdec.c | 2 +-
> 3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 5d8491b74b..597413803f 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -2051,6 +2051,8 @@ static int udp_read_packet(AVFormatContext *s, 
> RTSPStream **prtsp_st,
> if ((ret = parse_rtsp_message(s)) < 0) {
> return ret;
> }
> +if (rt->state == RTSP_STATE_TEARDOWN)
> +return AVERROR_EOF;
> }
> #endif
> } else if (n == 0 && ++timeout_cnt >= MAX_TIMEOUTS) {
> diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
> index 54a9a30c16..481cc0c3ce 100644
> --- a/libavformat/rtsp.h
> +++ b/libavformat/rtsp.h
> @@ -198,6 +198,7 @@ enum RTSPClientState {
> RTSP_STATE_STREAMING, /**< initialized and sending/receiving data */
> RTSP_STATE_PAUSED,  /**< initialized, but not receiving data */
> RTSP_STATE_SEEKING, /**< initialized, requesting a seek */
> +RTSP_STATE_TEARDOWN,/**< initialized, in teardown state */
> };
> 
> /**
> diff --git a/libavformat/rtspdec.c b/libavformat/rtspdec.c
> index dfa29913bf..ec786a469a 100644
> --- a/libavformat/rtspdec.c
> +++ b/libavformat/rtspdec.c
> @@ -494,7 +494,7 @@ int ff_rtsp_parse_streaming_commands(AVFormatContext *s)
>   "Public: ANNOUNCE, PAUSE, SETUP, TEARDOWN, "
>   "RECORD\r\n", request.seq);
> } else if (methodcode == TEARDOWN) {
> -rt->state = RTSP_STATE_IDLE;
> +rt->state = RTSP_STATE_TEARDOWN;
> ret   = rtsp_send_reply(s, RTSP_STATUS_OK, NULL , request.seq);
> }
> return ret;
> -- 
> 2.25.1
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/http: set hostname and lower_url buffer size properly

2020-09-26 Thread Zhao Zhili
Ping for review, thanks.

> On Aug 24, 2020, at 11:10 PM, Zhao Zhili  wrote:
> 
> 1. The buffer size of lower_url shouldn't be smaller than hostname
> 2. The maximum length of a DNS name is 255 octets
> ---
> libavformat/http.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 3d25d652d3..9c40a82a5b 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -525,8 +525,8 @@ static int http_listen(URLContext *h, const char *uri, 
> int flags,
>AVDictionary **options) {
> HTTPContext *s = h->priv_data;
> int ret;
> -char hostname[1024], proto[10];
> -char lower_url[100];
> +char hostname[256], proto[10];
> +char lower_url[512];
> const char *lower_proto = "tcp";
> int port;
> av_url_split(proto, sizeof(proto), NULL, 0, hostname, sizeof(hostname), 
> ,
> -- 
> 2.25.1
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avformat/utils: remove always true if()

2020-09-26 Thread Zhao Zhili
Ping for the patch set.

> On Aug 25, 2020, at 1:16 AM, Zhao Zhili  wrote:
> 
> Ping again.
> 
>> On Aug 5, 2020, at 11:16 PM, Zhao Zhili  wrote:
>> 
>> Ping for the trivial patch.
>> 
>>> On Jul 29, 2020, at 1:02 AM, Zhao Zhili  wrote:
>>> 
>>> ---
>>> libavformat/utils.c | 12 
>>> 1 file changed, 4 insertions(+), 8 deletions(-)
>>> 
>>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>>> index 807d9f10cb..af1cca0c7d 100644
>>> --- a/libavformat/utils.c
>>> +++ b/libavformat/utils.c
>>> @@ -2543,6 +2543,8 @@ int av_seek_frame(AVFormatContext *s, int 
>>> stream_index,
>>> int avformat_seek_file(AVFormatContext *s, int stream_index, int64_t min_ts,
>>>  int64_t ts, int64_t max_ts, int flags)
>>> {
>>> +int ret;
>>> +int dir;
>>>   if (min_ts > ts || max_ts < ts)
>>>   return -1;
>>>   if (stream_index < -1 || stream_index >= (int)s->nb_streams)
>>> @@ -2553,7 +2555,6 @@ int avformat_seek_file(AVFormatContext *s, int 
>>> stream_index, int64_t min_ts,
>>>   flags &= ~AVSEEK_FLAG_BACKWARD;
>>> 
>>>   if (s->iformat->read_seek2) {
>>> -int ret;
>>>   ff_read_frame_flush(s);
>>> 
>>>   if (stream_index == -1 && s->nb_streams == 1) {
>>> @@ -2582,19 +2583,14 @@ int avformat_seek_file(AVFormatContext *s, int 
>>> stream_index, int64_t min_ts,
>>> 
>>>   // Fall back on old API if new is not implemented but old is.
>>>   // Note the old API has somewhat different semantics.
>>> -if (s->iformat->read_seek || 1) {
>>> -int dir = (ts - (uint64_t)min_ts > (uint64_t)max_ts - ts ? 
>>> AVSEEK_FLAG_BACKWARD : 0);
>>> -int ret = av_seek_frame(s, stream_index, ts, flags | dir);
>>> +dir = (ts - (uint64_t)min_ts > (uint64_t)max_ts - ts ? 
>>> AVSEEK_FLAG_BACKWARD : 0);
>>> +ret = av_seek_frame(s, stream_index, ts, flags | dir);
>>>   if (ret<0 && ts != min_ts && max_ts != ts) {
>>>   ret = av_seek_frame(s, stream_index, dir ? max_ts : min_ts, flags 
>>> | dir);
>>>   if (ret >= 0)
>>>   ret = av_seek_frame(s, stream_index, ts, flags | 
>>> (dir^AVSEEK_FLAG_BACKWARD));
>>>   }
>>>   return ret;
>>> -}
>>> -
>>> -// try some generic seek like seek_frame_generic() but with new ts 
>>> semantics
>>> -return -1; //unreachable
>>> }
>>> 
>>> int avformat_flush(AVFormatContext *s)
>>> -- 
>>> 2.25.1
>>> 
>>> ___
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>> 
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check avctx->hwaccel when hwaccel pix_fmt selected

2020-09-26 Thread James Almer
On 9/25/2020 4:35 AM, Xiang, Haihao wrote:
> On Fri, 2020-09-25 at 06:10 +, Wang, Fei W wrote:
>>> -Original Message-
>>> From: ffmpeg-devel  On Behalf Of Wang,
>>> Fei W
>>> Sent: Tuesday, September 22, 2020 11:22 AM
>>> To: FFmpeg development discussions and patches 
>>> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check avctx->hwaccel
>>> when hwaccel pix_fmt selected
>>>
>>>
>>>
 -Original Message-
 From: ffmpeg-devel  On Behalf Of
 James Almer
 Sent: Friday, September 18, 2020 8:57 PM
 To: ffmpeg-devel@ffmpeg.org
 Subject: Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check
 avctx->hwaccel when hwaccel pix_fmt selected

 On 9/18/2020 2:40 AM, Wang, Fei W wrote:
>
>
>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
>> Hendrik Leppkes
>> Sent: Thursday, September 17, 2020 5:21 PM
>> To: FFmpeg development discussions and patches
>> 
>> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check
>> avctx->hwaccel when hwaccel pix_fmt selected
>>
>> On Thu, Sep 17, 2020 at 10:38 AM Fei Wang 
>>>
>>> wrote:
>>>
>>> Pix fmt with hwaccel flag may not be chosen in format probing, in
>>> this case avctx->hwaccel will not be inited.
>>>
>>> Signed-off-by: Fei Wang 
>>> ---
>>>  libavcodec/av1dec.c | 12 
>>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index
>>> 0bb04a3e44..cdcc618013 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -251,6 +251,7 @@ static int get_pixel_format(AVCodecContext
>>> *avctx) {
>>>  AV1DecContext *s = avctx->priv_data;
>>>  const AV1RawSequenceHeader *seq = s->raw_seq;
>>> +const AVPixFmtDescriptor *desc;
>>>  uint8_t bit_depth;
>>>  int ret;
>>>  enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE; @@ -327,10
>>> +328,13 @@ static int get_pixel_format(AVCodecContext *avctx)
>>>   * Since now the av1 decoder doesn't support native decode, if
>>> it will be
>>>   * implemented in the future, need remove this check.
>>>   */
>>> -if (!avctx->hwaccel) {
>>> -av_log(avctx, AV_LOG_ERROR, "Your platform doesn't
>>> suppport"
>>> -   " hardware accelerated AV1 decoding.\n");
>>> -return AVERROR(ENOSYS);
>>> +desc = av_pix_fmt_desc_get(ret);
>>> +if (desc && (desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
>>> +if (!avctx->hwaccel) {
>>> +av_log(avctx, AV_LOG_ERROR, "Your platform doesn't
>>> suppport"
>>> +   " hardware accelerated AV1 decoding.\n");
>>> +return AVERROR(ENOSYS);
>>> +}
>>>  }
>>>
>>
>> Isn't it supposed to quit here, because we do not have software
>> decoding?
>
> Since now av1 decoder allow probe, that will try to decode first
> frame to find stream info in open_file stage. While the hwaccel will not
> be
>>>
>>> inited.
> If without change here, the error log will be print out but later
> during transcode stage, it can gives out the correct output.

 If you let it choose a software pix_fmt, it will think the decoder can
 actually output something, which is not true.
>>>
>>> It's better not let ff_thread_get_format choose a software pix fmt here, but
>>> for
>>> VAAPI the avctx->hw_device_ctx hasn't been created in probing so that
>>> hwaccel
>>> can not be inited and hwaccel pix fmt will not be chosen(same mechanism with
>>> other codecs). And then next available pix fmt in input array will be
>>> considered.
>>>
 Probing works fine even if it fails at this point. It will have set
 enough AVCodecContext fields to allow things like remuxing, hence the
 relevant fate tests succeeding as is.
>>>
>>> Yes, probing allows fail here. How about to keep the original check(!avctx-
 hwaccel), and change log context as like "Hardware context not created or
>>>
>>> your platform doesn't support hardware accelerated AV1 decoding" and change
>>> log level to WARNING ? It's really confused if error log printed but decode
>>> works
>>> correctly.
>>
>> Kindly ping @James Almer, any comments to change log context and level here?
> 
> 
> Since commit e46f34e8 was merged, I also saw the same error message when I
> tested my QSV av1 patch (
> http://ffmpeg.org/pipermail/ffmpeg-devel/2020-September/270234.html) however 
> the
> command worked well for me.
> 
> [av1 @ 0x55b1604c42d0] Your platform doesn't suppport hardware accelerated AV1
> decoding.
> [av1 @ 0x55b1604c42d0] Failed to get pixel format.
> [av1 @ 0x55b1604c42d0] video_get_buffer: image parameters invalid
> [av1 @ 0x55b1604c42d0] get_buffer() failed
> [av1 @ 0x55b1604c42d0] thread_get_buffer() failed
> [av1 @ 0x55b1604c42d0] Failed to allocate space for current 

Re: [FFmpeg-devel] [PATCH 2/3] avformat/aviobuf: fix checks in ffio_ensure_seekback

2020-09-26 Thread Michael Niedermayer
On Sat, Sep 26, 2020 at 12:46:40PM +0200, Marton Balint wrote:
> 
> 
> On Sat, 26 Sep 2020, Michael Niedermayer wrote:
> 
> > On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:
> > > Signed-off-by: Marton Balint 
> > > ---
> > >  libavformat/aviobuf.c | 7 +--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > The commit message is too terse. It is not clear from it what the problem
> > is that is being fixed and how it is fixed.
> > Also this feels like it fixes multiple issues so it probably should be spit
> > unless iam missing some interdependancies
> 
> It can be seperated to two patches if that is preferred:
> 
> 1) existing code does not check if the requested seekback buffer is already
> read entirely. In this case, nothing has to be done. This is the newly added
> if() at the beginning of the patch.
> 
> 2) the new buf_size is detemined too conservatively, maybe because of the
> issue which was fixed in patch 1/3. So we can safely substract 1 more from
> the new buffer size. Comparing the new buf_size against filled does not make
> a lot of sense to me, that just seems wrong. What makes sense is that we
> want to reallocate the buffer if the new buf_size is bigger than the old,
> therefore the change in the check.

i would prefer it split yes


> 
> > 
> > ill also take a look at the "competing" patch, so i dont yet know
> > how they relate ...
> > 
> > 
> > > 
> > > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> > > index 9675425349..aa1d6c0830 100644
> > > --- a/libavformat/aviobuf.c
> > > +++ b/libavformat/aviobuf.c
> > > @@ -999,9 +999,12 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t 
> > > buf_size)
> > >  int filled = s->buf_end - s->buffer;
> > >  ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
> > > s->buffer : -1;
> > > 
> > > -buf_size += s->buf_ptr - s->buffer + max_buffer_size;
> > > +if (buf_size <= s->buf_end - s->buf_ptr)
> > > +return 0;
> > > +
> > > +buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;
> > > 
> > 
> > > -if (buf_size < filled || s->seekable || !s->read_packet)
> > > +if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
> > >  return 0;
> > 
> > Did you check that this doesnt interfere with the s->buffer_size reduction
> > we do when it was larger from probing ?
> > Its maybe ok, just checking that this was considered
> 
> I don't see how that can be affected by this change, so I am not sure what
> you mean here. If the new buffer size is bigger than s->orig_buffer_size,
> then eventually it will be reduced, it does not seem more complicated than
> that to me.

what i meant was that if its reduced then at that point the seekback 
gurantee changes relative to it never being reduced.
I think you consider this in your code, i just wanted to double check.

thx


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

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


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] Question about HEIF/HEIC support

2020-09-26 Thread Tom Needham
Hello

I have spent some time researching the possibility of adding demuxing and
muxing support for the HEIF image format into FFmpeg when I came across
this GSOC project from last year.
https://summerofcode.withgoogle.com/archive/2019/projects/5632663078043648/.
Although it appears the resulting work was never sent back to FFmpeg.

After doing a bit more digging I found the Github repository of the author
of the original work
https://github.com/Swaraj1998/FFmpeg/commit/9a885cddb3550ab863a60d02c5fb78e4ae206cf1
and there is a commit there that seems to contain the required code for
HEIF demuxing, I have compiled that branch locally and it seems to handle
all the samples I could throw at it.

Was there a reason this work was not pushed back to FFmpeg? If it is just a
matter of updating the patch to work on the latest master. I would be happy
to help out.

Thanks
Tom
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/4] avcodec/svq3: dont crash on free_picture(NULL)

2020-09-26 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Andreas Rheinhardt:
>> Michael Niedermayer:
>>> Fixes: NULL dereference
>>> Fixes: 
>>> 25762/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SVQ3_fuzzer-5716279070294016
>>>
>>> Found-by: continuous fuzzing process 
>>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer 
>>> ---
>>>  libavcodec/svq3.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/libavcodec/svq3.c b/libavcodec/svq3.c
>>> index fb7b992496..2da757bee9 100644
>>> --- a/libavcodec/svq3.c
>>> +++ b/libavcodec/svq3.c
>>> @@ -1323,6 +1323,10 @@ static av_cold int svq3_decode_init(AVCodecContext 
>>> *avctx)
>>>  static void free_picture(AVCodecContext *avctx, SVQ3Frame *pic)
>>>  {
>>>  int i;
>>> +
>>> +if (!pic)
>>> +return;
>>> +
>>>  for (i = 0; i < 2; i++) {
>>>  av_freep(>motion_val_buf[i]);
>>>  }
>>>
>> This surprises me a lot: Since 96061c5a4f690c3ab49e4458701bb013fd3dd57f,
>> the pointers to the pictures are set to something different from NULL
>> directly at the beginning of the init function; they are never set to
>> NULL afterwards, they are only swapped with pointers to other pictures.
>> So how can they ever be NULL? Has the init function not been properly
>> called? (And if any of these pictures were NULL, then svq3_decode_end()
>> will call av_frame_free(NULL), which is unsafe, but doesn't crash
>> currently; the situation would change if the AVFrame * were somewhere
>> else than the beginning of SVQ3Frame.)
>>
>> - Andreas
>>
> I just noticed that avcodec_open2() will call the codec's close function
> even when init has never been called when the FF_CODEC_CAP_INIT_CLEANUP
> flag is set. This is against the documentation of said flag and it is
> also complete nonsense. Will send a patch for this.
> 
Done: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-September/270336.html

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 10/25] avcodec/magicyuv: Fix building Huffman table

2020-09-26 Thread Andreas Rheinhardt
Paul B Mahol:
> On Sat, Sep 26, 2020 at 12:27:49PM +0200, Andreas Rheinhardt wrote:
>> The MagicYUV format stores Huffman tables in its bitstream by coding
>> the length of a given symbol; it does not code the actual code directly,
>> instead this is to be inferred by the rule that a symbol is to the left
>> of every shorter symbol in the Huffman tree and that for symbols of the
>> same length the symbol is ascending from left to right. With one
>> exception, this is also what our decoder did.
>>
>> The exception only matters when there are codes of length 32, because
>> in this case the first symbol of this length did not get the code 0,
>> but 1; e.g. if there were exactly two nodes of length 32, then they
>> would get assigned the codes 1 and 2 and a node of length 31 will get
>> the 31-bit code 1 which is a prefix of the 32 bit code 2, making the
>> Huffman table invalid. On the other hand, if there were only one symbol
>> with the length 32, the earlier code would accept this un-Huffman-tree.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/magicyuv.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> should be ok with subject fixed, it is not fix, its just fixes single
> case that never happens in reality.
> 

I know that this does not affect normal files, but I have trouble
finding an accurate commit message that avoids "fix". So how about "Fix
edge case when building Huffman tables"?

>>
>> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
>> index 1b3f4cfc6b..17dea69d76 100644
>> --- a/libavcodec/magicyuv.c
>> +++ b/libavcodec/magicyuv.c
>> @@ -86,7 +86,7 @@ static int huff_build(HuffEntry he[], VLC *vlc, int 
>> nb_elems)
>>  
>>  AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
>>  
>> -code = 1;
>> +code = 0;
>>  for (unsigned i = 0; i < nb_elems; i++) {
>>  he[i].code = code >> (32 - he[i].len);
>>  code += 0x8000u >> (he[i].len - 1);
>> -- 
>> 2.25.1
>>
>> ___
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:50:34PM +0200, Michael Niedermayer wrote:
> On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> > This removes big CPU overhead for demuxing chained ogg streams.
> > 
> > Signed-off-by: Paul B Mahol 
> > ---
> >  libavformat/aviobuf.c | 10 +-
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> I think we need some fate test for ffio_ensure_seekback()
> as our tests are ATM we generally have fully seekable inputs so
> failure of seekback would not be noticed.
> 
> This is a problem because the buffer IO stuff is not easy to
> understand and so it is time consuming and error prone to review these patches
> just by eye. And as we see developers disagree on what is correct
> 
> Such fate test should at least test various sequentially occuring
> ffio_ensure_seekback(), a possibly larger initial buffer from probing
> as well as its later reduction. And also reading requests failing, early EOF
> seeking, and IO errors

Well, if its documented that ffio_ensure_seekback() can not flush buffer
than there should be added code/function to do allow flushing.

Because oggdec works fine with assumed flushing and my patch.

> 
> thanks
> 
> [...]
> 
> -- 
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> The day soldiers stop bringing you their problems is the day you have stopped 
> leading them. They have either lost confidence that you can help or concluded 
> you do not care. Either case is a failure of leadership. - Colin Powell



> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 18/25] avcodec/utils: Only call codec->close if init has been called

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:57PM +0200, Andreas Rheinhardt wrote:
> avcodec_open2() also called the AVCodec's close function if an error
> happened before init had ever been called if the AVCodec has the
> FF_CODEC_CAP_INIT_CLEANUP flag set. This is against the documentation of
> said flag: "The codec allows calling the close function for deallocation
> even if the init function returned a failure."
> 
> E.g. the SVQ3 decoder is not ready to be closed if init has never been
> called.
> 
> Fixes: NULL dereference
> Fixes: 
> 25762/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SVQ3_fuzzer-5716279070294016
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> If someone were to call avcodec_alloc_context3(NULL) and manually fill
> in avctx->codec, then the close function might even be called before
> avctx->priv_data has been allocated.
>

probably ok. please wait for others to also comment.

>  libavcodec/utils.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index cf0a55f26d..94b9e94584 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -931,6 +931,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  || avci->frame_thread_encoder)) {
>  ret = avctx->codec->init(avctx);
>  if (ret < 0) {
> +codec_init_ok = -1;
>  goto free_and_end;
>  }
>  codec_init_ok = 1;
> @@ -1022,8 +1023,8 @@ end:
>  return ret;
>  free_and_end:
>  if (avctx->codec && avctx->codec->close &&
> -(codec_init_ok ||
> - (avctx->codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP)))
> +(codec_init_ok > 0 || (codec_init_ok < 0 &&
> + avctx->codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP)))
>  avctx->codec->close(avctx);
>  
>  if (HAVE_THREADS && avci->thread_ctx)
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 09/25] avcodec/magicyuv: Don't invert order unnecessarily

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:48PM +0200, Andreas Rheinhardt wrote:
> The MagicYUV decoder currently sets both the length and the symbol field
> of an array of HuffEntries; hereby the symbol of the ith entry (0-based)
> is just i. Then said array gets sorted so that entries with greater
> length are at the end and entries with the same length are ordered so
> that those with smaller symbols are at the end. Afterwards the newly
> sorted array is traversed in reverse order. This commit instead inverts
> the ordering and traverses the array in its ordinary order in order to
> simplify understanding.

apply as you wish.

> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> This commit actually only exists to simplify understanding of the next
> two commits; apart from that, it is unnecessary.
> 
>  libavcodec/magicyuv.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 93ee739093..1b3f4cfc6b 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -77,24 +77,23 @@ typedef struct MagicYUVContext {
>  static int huff_cmp_len(const void *a, const void *b)
>  {
>  const HuffEntry *aa = a, *bb = b;
> -return (aa->len - bb->len) * 4096 + bb->sym - aa->sym;
> +return (bb->len - aa->len) * 4096 + aa->sym - bb->sym;
>  }
>  
>  static int huff_build(HuffEntry he[], VLC *vlc, int nb_elems)
>  {
>  uint32_t code;
> -int i;
>  
>  AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
>  
>  code = 1;
> -for (i = nb_elems - 1; i >= 0; i--) {
> +for (unsigned i = 0; i < nb_elems; i++) {
>  he[i].code = code >> (32 - he[i].len);
>  code += 0x8000u >> (he[i].len - 1);
>  }
>  
>  ff_free_vlc(vlc);
> -return ff_init_vlc_sparse(vlc, FFMIN(he[nb_elems - 1].len, 12), nb_elems,
> +return ff_init_vlc_sparse(vlc, FFMIN(he[0].len, 12), nb_elems,
>[0].len,  sizeof(he[0]), sizeof(he[0].len),
>[0].code, sizeof(he[0]), sizeof(he[0].code),
>[0].sym,  sizeof(he[0]), sizeof(he[0].sym), 
>  0);
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 17/25] avcodec/utvideo: Move stuff only used by Ut encoder to Ut encoder

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:56PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/Makefile |  4 ++--
>  libavcodec/utvideo.c| 39 ---
>  libavcodec/utvideo.h| 12 
>  libavcodec/utvideoenc.c | 23 +--
>  4 files changed, 23 insertions(+), 55 deletions(-)
>  delete mode 100644 libavcodec/utvideo.c

lgtm

> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 9b120a6f64..bee2335a5a 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -664,8 +664,8 @@ OBJS-$(CONFIG_TTA_ENCODER) += ttaenc.o 
> ttaencdsp.o ttadata.o
>  OBJS-$(CONFIG_TWINVQ_DECODER)  += twinvqdec.o twinvq.o
>  OBJS-$(CONFIG_TXD_DECODER) += txd.o
>  OBJS-$(CONFIG_ULTI_DECODER)+= ulti.o
> -OBJS-$(CONFIG_UTVIDEO_DECODER) += utvideodec.o utvideo.o utvideodsp.o
> -OBJS-$(CONFIG_UTVIDEO_ENCODER) += utvideoenc.o utvideo.o
> +OBJS-$(CONFIG_UTVIDEO_DECODER) += utvideodec.o utvideodsp.o
> +OBJS-$(CONFIG_UTVIDEO_ENCODER) += utvideoenc.o
>  OBJS-$(CONFIG_V210_DECODER)+= v210dec.o
>  OBJS-$(CONFIG_V210_ENCODER)+= v210enc.o
>  OBJS-$(CONFIG_V210X_DECODER)   += v210x.o
> diff --git a/libavcodec/utvideo.c b/libavcodec/utvideo.c
> deleted file mode 100644
> index 0cf0cbcd8b..00
> --- a/libavcodec/utvideo.c
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -/*
> - * Common Ut Video code
> - * Copyright (c) 2011 Konstantin Shishkov
> - *
> - * This file is part of FFmpeg.
> - *
> - * FFmpeg is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU Lesser General Public
> - * License as published by the Free Software Foundation; either
> - * version 2.1 of the License, or (at your option) any later version.
> - *
> - * FFmpeg is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> - * Lesser General Public License for more details.
> - *
> - * You should have received a copy of the GNU Lesser General Public
> - * License along with FFmpeg; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> - */
> -
> -/**
> - * @file
> - * Common Ut Video code
> - */
> -
> -#include "utvideo.h"
> -
> -#if FF_API_PRIVATE_OPT
> -const int ff_ut_pred_order[5] = {
> -PRED_LEFT, PRED_MEDIAN, PRED_MEDIAN, PRED_NONE, PRED_GRADIENT
> -};
> -#endif
> -
> -int ff_ut_huff_cmp_len(const void *a, const void *b)
> -{
> -const HuffEntry *aa = a, *bb = b;
> -return (aa->len - bb->len)*256 + aa->sym - bb->sym;
> -}
> diff --git a/libavcodec/utvideo.h b/libavcodec/utvideo.h
> index 2975f287a7..9da9329ff3 100644
> --- a/libavcodec/utvideo.h
> +++ b/libavcodec/utvideo.h
> @@ -61,9 +61,6 @@ enum {
>  UTVIDEO_444  = MKTAG('Y', 'V', '2', '4'),
>  };
>  
> -/* Mapping of libavcodec prediction modes to Ut Video's */
> -extern const int ff_ut_pred_order[5];
> -
>  typedef struct UtvideoContext {
>  const AVClass *class;
>  AVCodecContext *avctx;
> @@ -91,13 +88,4 @@ typedef struct UtvideoContext {
>  size_t control_stream_size[4][256];
>  } UtvideoContext;
>  
> -typedef struct HuffEntry {
> -uint16_t sym;
> -uint8_t  len;
> -uint32_t code;
> -} HuffEntry;
> -
> -/* Compare huffman tree nodes */
> -int ff_ut_huff_cmp_len(const void *a, const void *b);
> -
>  #endif /* AVCODEC_UTVIDEO_H */
> diff --git a/libavcodec/utvideoenc.c b/libavcodec/utvideoenc.c
> index 05a9614036..5c87eb50ac 100644
> --- a/libavcodec/utvideoenc.c
> +++ b/libavcodec/utvideoenc.c
> @@ -37,6 +37,25 @@
>  #include "utvideo.h"
>  #include "huffman.h"
>  
> +typedef struct HuffEntry {
> +uint16_t sym;
> +uint8_t  len;
> +uint32_t code;
> +} HuffEntry;
> +
> +#if FF_API_PRIVATE_OPT
> +static const int ut_pred_order[5] = {
> +PRED_LEFT, PRED_MEDIAN, PRED_MEDIAN, PRED_NONE, PRED_GRADIENT
> +};
> +#endif
> +
> +/* Compare huffman tree nodes */
> +static int ut_huff_cmp_len(const void *a, const void *b)
> +{
> +const HuffEntry *aa = a, *bb = b;
> +return (aa->len - bb->len)*256 + aa->sym - bb->sym;
> +}
> +
>  /* Compare huffentry symbols */
>  static int huff_cmp_sym(const void *a, const void *b)
>  {
> @@ -139,7 +158,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
>  
>  /* Convert from libavcodec prediction type to Ut Video's */
>  if (avctx->prediction_method)
> -c->frame_pred = ff_ut_pred_order[avctx->prediction_method];
> +c->frame_pred = ut_pred_order[avctx->prediction_method];
>  FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
>  
> @@ -340,7 +359,7 @@ static void calculate_codes(HuffEntry *he)
>  int last, i;
>  uint32_t code;
>  
> -qsort(he, 256, sizeof(*he), ff_ut_huff_cmp_len);
> +qsort(he, 256, sizeof(*he), ut_huff_cmp_len);
>  
>  last = 255;
>  

Re: [FFmpeg-devel] [PATCH 16/25] avcodec/utvideo: Remove unused array

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:55PM +0200, Andreas Rheinhardt wrote:
> Unused since 3594788b713e76449eda0bc9d64b38258c86a594.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/utvideo.c | 2 --
>  1 file changed, 2 deletions(-)
> 

lgtm

> diff --git a/libavcodec/utvideo.c b/libavcodec/utvideo.c
> index b14e56e0d8..0cf0cbcd8b 100644
> --- a/libavcodec/utvideo.c
> +++ b/libavcodec/utvideo.c
> @@ -32,8 +32,6 @@ const int ff_ut_pred_order[5] = {
>  };
>  #endif
>  
> -const int ff_ut_rgb_order[4]  = { 1, 2, 0, 3 }; // G, B, R, A
> -
>  int ff_ut_huff_cmp_len(const void *a, const void *b)
>  {
>  const HuffEntry *aa = a, *bb = b;
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 01/25] avcodec/photocd: Simplify parsing Huffman tables a bit

2020-09-26 Thread Andreas Rheinhardt
Paul B Mahol:
> On Sat, Sep 26, 2020 at 12:27:40PM +0200, Andreas Rheinhardt wrote:
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/photocd.c | 13 ++---
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>>
> 
> should be ok if tested.
> 

Tested with the (NSFW) files from
http://cd.textfiles.com/prettywomen/IMAGES/

>> diff --git a/libavcodec/photocd.c b/libavcodec/photocd.c
>> index 057c9d33d4..8fd4536a65 100644
>> --- a/libavcodec/photocd.c
>> +++ b/libavcodec/photocd.c
>> @@ -245,21 +245,20 @@ static av_noinline int decode_huff(AVCodecContext 
>> *avctx, AVFrame *frame,
>>  int x2, idx;
>>  
>>  for (; get_bits_left() > 0;) {
>> -if ((show_bits(, 24) & 0xfff000) == 0xfff000)
>> +if (show_bits(, 12) == 0xfff)
>>  break;
>>  skip_bits(, 8);
>>  }
>>  
>> -shiftreg = show_bits_long(, 32) & 0xff00;
>> -while (shiftreg != 0xfe00) {
>> +shiftreg = show_bits(, 24);
>> +while (shiftreg != 0xfe) {
>>  if (get_bits_left() <= 0)
>>  return AVERROR_INVALIDDATA;
>>  skip_bits(, 1);
>> -shiftreg = show_bits_long(, 32) & 0xff00;
>> +shiftreg = show_bits(, 24);
>>  }
>> -skip_bits(, 16);
>> -y = show_bits_long(, 23) & 0x1fff;
>> -skip_bits(, 8);
>> +skip_bits(, 24);
>> +y = show_bits(, 15) & 0x1fff;
>>  if (y >= height)
>>  break;
>>  type = get_bits(, 2);
>> -- 
>> 2.25.1
>>

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 15/25] avcodec/utvideodec: Avoid qsort when creating Huffman tables

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:54PM +0200, Andreas Rheinhardt wrote:
> The Ut video format uses Huffman trees which are only implicitly coded
> in the bitstream: Only the lengths of the codes are coded, the rest has
> to be inferred by the decoder according to the rule that the longer
> codes are to the left of shorter codes in the tree and on each level the
> symbols are descending from left to right.
> 
> Because longer codes are to the left of shorter codes, one needs to know
> how many non-leaf nodes there are on each level in order to know the
> code of the next left-most leaf (which belongs to the highest symbol on
> that level). The current code does this by sorting the entries to be
> ascending according to length and (for entries with the same length)
> ascending according to their symbols. This array is then traversed in
> reverse order, so that the lowest level is dealt with first, so that the
> number of non-leaf nodes of the next higher level is known when
> processing said level.
> 
> But this can also be calculated without sorting: Simply count how many
> leaf nodes there are on each level. Then one can calculate the number of
> non-leaf nodes on each level iteratively from the lowest level upwards:
> It is just half the number of nodes of the level below.
> 
> This improves performance: For the sample from ticket #4044 the amount
> of decicycles for one call to build_huff() decreased from 1055489 to
> 446310 for Clang 10 and from 1080306 to 535155 for GCC 9.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/utvideo.c|  6 -
>  libavcodec/utvideo.h|  1 -
>  libavcodec/utvideodec.c | 53 -
>  3 files changed, 26 insertions(+), 34 deletions(-)
>

ok, i guess fate passes.

> diff --git a/libavcodec/utvideo.c b/libavcodec/utvideo.c
> index 5828d5ec0d..b14e56e0d8 100644
> --- a/libavcodec/utvideo.c
> +++ b/libavcodec/utvideo.c
> @@ -39,9 +39,3 @@ int ff_ut_huff_cmp_len(const void *a, const void *b)
>  const HuffEntry *aa = a, *bb = b;
>  return (aa->len - bb->len)*256 + aa->sym - bb->sym;
>  }
> -
> -int ff_ut10_huff_cmp_len(const void *a, const void *b)
> -{
> -const HuffEntry *aa = a, *bb = b;
> -return (aa->len - bb->len)*1024 + aa->sym - bb->sym;
> -}
> diff --git a/libavcodec/utvideo.h b/libavcodec/utvideo.h
> index cf0bb28c44..2975f287a7 100644
> --- a/libavcodec/utvideo.h
> +++ b/libavcodec/utvideo.h
> @@ -99,6 +99,5 @@ typedef struct HuffEntry {
>  
>  /* Compare huffman tree nodes */
>  int ff_ut_huff_cmp_len(const void *a, const void *b);
> -int ff_ut10_huff_cmp_len(const void *a, const void *b);
>  
>  #endif /* AVCODEC_UTVIDEO_H */
> diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
> index f014e90606..8b47c14d98 100644
> --- a/libavcodec/utvideodec.c
> +++ b/libavcodec/utvideodec.c
> @@ -43,45 +43,44 @@
>  static int build_huff(const uint8_t *src, VLC *vlc, int *fsym, unsigned 
> nb_elems)
>  {
>  int i;
> -HuffEntry he[1024];
> -int last;
>  uint32_t codes[1024];
>  uint8_t bits[1024];
> -uint16_t syms[1024];
> -uint32_t code;
> +uint16_t codes_count[33] = { 0 };
>  
>  *fsym = -1;
>  for (i = 0; i < nb_elems; i++) {
> -he[i].sym = i;
> -he[i].len = *src++;
> -}
> -qsort(he, nb_elems, sizeof(*he), ff_ut10_huff_cmp_len);
> +if (src[i] == 0) {
> +*fsym = i;
> +return 0;
> +} else if (src[i] == 255) {
> +bits[i] = 0;
> +} else if (src[i] <= 32) {
> +bits[i] = src[i];
> +} else
> +return AVERROR_INVALIDDATA;
>  
> -if (!he[0].len) {
> -*fsym = he[0].sym;
> -return 0;
> +codes_count[bits[i]]++;
>  }
> +if (codes_count[0] == nb_elems)
> +return AVERROR_INVALIDDATA;
>  
> -last = nb_elems - 1;
> -while (he[last].len == 255 && last)
> -last--;
> -
> -if (he[last].len > 32) {
> -return -1;
> +for (unsigned i = 32, nb_codes = 0; i > 0; i--) {
> +uint16_t curr = codes_count[i];   // # of leafs of length i
> +codes_count[i] = nb_codes / 2;// # of non-leaf nodes on level i
> +nb_codes = codes_count[i] + curr; // # of nodes on level i
>  }
>  
> -code = 0;
> -for (i = last; i >= 0; i--) {
> -codes[i] = code >> (32 - he[i].len);
> -bits[i]  = he[i].len;
> -syms[i]  = he[i].sym;
> -code += 0x8000u >> (he[i].len - 1);
> +for (unsigned i = nb_elems; i-- > 0;) {
> +if (!bits[i]) {
> +codes[i] = 0;
> +continue;
> +}
> +codes[i] = codes_count[bits[i]]++;
>  }
>  #define VLC_BITS 11
> -return ff_init_vlc_sparse(vlc, VLC_BITS, last + 1,
> -  bits,  sizeof(*bits),  sizeof(*bits),
> -  codes, sizeof(*codes), sizeof(*codes),
> -  syms,  sizeof(*syms),  sizeof(*syms), 0);
> +

Re: [FFmpeg-devel] [PATCH 12/25] avcodec/magicyuv: Don't waste stack space

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:51PM +0200, Andreas Rheinhardt wrote:
> Now that the HuffEntries are no longer sorted by the MagicYUV decoder,
> their symbols are trivial: The symbol of the element with index i is i.
> They can therefore be removed. Furthermore, despite the length of the
> codes being in the range 1..32 bits, the actual value of the codes is
> <= 4096 (for 12 bit content). The reason for this is that the longer
> codes are on the left side of the tree, so that the higher bits of
> these codes are simply zero. By using an uint16_t for the codes and
> removing the symbols entry, the size of each HuffEntry is decreased from
> eight to four, saving 16KB of stack space.

lgtm

> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 14 +-
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 7dded9b457..ea1f727e5c 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -46,9 +46,8 @@ typedef enum Prediction {
>  } Prediction;
>  
>  typedef struct HuffEntry {
> -uint16_t sym;
>  uint8_t  len;
> -uint32_t code;
> +uint16_t code;
>  } HuffEntry;
>  
>  typedef struct MagicYUVContext {
> @@ -90,10 +89,9 @@ static int huff_build(HuffEntry he[], uint16_t 
> codes_count[33],
>  he[i].code = codes_count[he[i].len];
>  codes_count[he[i].len]++;
>  }
> -return ff_init_vlc_sparse(vlc, FFMIN(max, 12), nb_elems,
> -  [0].len,  sizeof(he[0]), sizeof(he[0].len),
> -  [0].code, sizeof(he[0]), sizeof(he[0].code),
> -  [0].sym,  sizeof(he[0]), sizeof(he[0].sym), 
>  0);
> +return init_vlc(vlc, FFMIN(max, 12), nb_elems,
> +[0].len,  sizeof(he[0]), sizeof(he[0].len),
> +[0].code, sizeof(he[0]), sizeof(he[0].code), 0);
>  }
>  
>  static void magicyuv_median_pred16(uint16_t *dst, const uint16_t *src1,
> @@ -408,10 +406,8 @@ static int build_huffman(AVCodecContext *avctx, const 
> uint8_t *table,
>  }
>  
>  length_count[x] += l;
> -for (; j < k; j++) {
> -he[j].sym = j;
> +for (; j < k; j++)
>  he[j].len = x;
> -}
>  
>  if (j == max) {
>  j = 0;
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 14/25] avcodec/utvideodec: Remove code duplication when creating Huffman tables

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:53PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/utvideodec.c | 55 +
>  1 file changed, 6 insertions(+), 49 deletions(-)
> 

lgtm

> diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
> index b3c4c3519b..f014e90606 100644
> --- a/libavcodec/utvideodec.c
> +++ b/libavcodec/utvideodec.c
> @@ -40,7 +40,7 @@
>  #include "thread.h"
>  #include "utvideo.h"
>  
> -static int build_huff10(const uint8_t *src, VLC *vlc, int *fsym)
> +static int build_huff(const uint8_t *src, VLC *vlc, int *fsym, unsigned 
> nb_elems)
>  {
>  int i;
>  HuffEntry he[1024];
> @@ -51,18 +51,18 @@ static int build_huff10(const uint8_t *src, VLC *vlc, int 
> *fsym)
>  uint32_t code;
>  
>  *fsym = -1;
> -for (i = 0; i < 1024; i++) {
> +for (i = 0; i < nb_elems; i++) {
>  he[i].sym = i;
>  he[i].len = *src++;
>  }
> -qsort(he, 1024, sizeof(*he), ff_ut10_huff_cmp_len);
> +qsort(he, nb_elems, sizeof(*he), ff_ut10_huff_cmp_len);
>  
>  if (!he[0].len) {
>  *fsym = he[0].sym;
>  return 0;
>  }
>  
> -last = 1023;
> +last = nb_elems - 1;
>  while (he[last].len == 255 && last)
>  last--;
>  
> @@ -84,49 +84,6 @@ static int build_huff10(const uint8_t *src, VLC *vlc, int 
> *fsym)
>syms,  sizeof(*syms),  sizeof(*syms), 0);
>  }
>  
> -static int build_huff(const uint8_t *src, VLC *vlc, int *fsym)
> -{
> -int i;
> -HuffEntry he[256];
> -int last;
> -uint32_t codes[256];
> -uint8_t bits[256];
> -uint8_t syms[256];
> -uint32_t code;
> -
> -*fsym = -1;
> -for (i = 0; i < 256; i++) {
> -he[i].sym = i;
> -he[i].len = *src++;
> -}
> -qsort(he, 256, sizeof(*he), ff_ut_huff_cmp_len);
> -
> -if (!he[0].len) {
> -*fsym = he[0].sym;
> -return 0;
> -}
> -
> -last = 255;
> -while (he[last].len == 255 && last)
> -last--;
> -
> -if (he[last].len > 32)
> -return -1;
> -
> -code = 0;
> -for (i = last; i >= 0; i--) {
> -codes[i] = code >> (32 - he[i].len);
> -bits[i]  = he[i].len;
> -syms[i]  = he[i].sym;
> -code += 0x8000u >> (he[i].len - 1);
> -}
> -
> -return ff_init_vlc_sparse(vlc, VLC_BITS, last + 1,
> -  bits,  sizeof(*bits),  sizeof(*bits),
> -  codes, sizeof(*codes), sizeof(*codes),
> -  syms,  sizeof(*syms),  sizeof(*syms), 0);
> -}
> -
>  static int decode_plane10(UtvideoContext *c, int plane_no,
>uint16_t *dst, ptrdiff_t stride,
>int width, int height,
> @@ -139,7 +96,7 @@ static int decode_plane10(UtvideoContext *c, int plane_no,
>  GetBitContext gb;
>  int prev, fsym;
>  
> -if ((ret = build_huff10(huff, , )) < 0) {
> +if ((ret = build_huff(huff, , , 1024)) < 0) {
>  av_log(c->avctx, AV_LOG_ERROR, "Cannot build Huffman codes\n");
>  return ret;
>  }
> @@ -299,7 +256,7 @@ static int decode_plane(UtvideoContext *c, int plane_no,
>  return 0;
>  }
>  
> -if (build_huff(src, , )) {
> +if (build_huff(src, , , 256)) {
>  av_log(c->avctx, AV_LOG_ERROR, "Cannot build Huffman codes\n");
>  return AVERROR_INVALIDDATA;
>  }
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 11/25] avcodec/magicyuv: Avoid AV_QSORT when creating Huffman table

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:50PM +0200, Andreas Rheinhardt wrote:
> The MagicYUV format stores Huffman tables in its bitstream by coding
> the length of a given symbol; it does not code the actual code directly,
> instead this is to be inferred by the rule that a symbol is to the left
> of every shorter symbol in the Huffman tree and that for symbols of the
> same length the symbol is ascending from left to right.
> 
> Our decoder implemented this by first sorting the array containing
> length and symbol of each element according to descending length and
> for equal length, according to ascending symbol. Afterwards, the current
> state in the tree got encoded in a variable code; if the next array entry
> had length len, then the len most significant bits of code contained
> the code of this entry. Whenever an entry of the array of length
> len was processed, code was incremented by 1U << (32 - len). So two
> entries of length len have the same effect as incrementing code by
> 1U << (32 - (len - 1)), which corresponds to the parent node of length
> len - 1 of the two nodes of length len etc.
> 
> This commit modifies this to avoid sorting the entries before
> calculating the codes. This is done by calculating how many non-leaf
> nodes there are on each level of the tree before calculating the codes.
> Afterwards every leaf node on this level gets assigned the number of
> nodes already on this level as code. This of course works only because
> the entries are already sorted by their symbol initially, so that this
> algorithm indeed gives ascending symbols from left to right on every
> level.
> 
> This offers both speed- as well as (obvious) codesize advantages. With
> Clang 10 the number of decicycles for build_huffman decreased from
> 1561987 to 1228405; for GCC 9 it went from 1825096 decicyles to 1429921.
> These tests were carried out with a sample with 150 frames that was
> looped 13 times; and this was iterated 10 times. The earlier reference
> point here is from the point when the loop generating the codes was
> traversed in reverse order (as the patch reversing the order led to
> performance penalties).

ok

> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 36 ++--
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 17dea69d76..7dded9b457 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -25,7 +25,6 @@
>  #define CACHED_BITSTREAM_READER !ARCH_X86_32
>  
>  #include "libavutil/pixdesc.h"
> -#include "libavutil/qsort.h"
>  
>  #include "avcodec.h"
>  #include "bytestream.h"
> @@ -74,26 +73,24 @@ typedef struct MagicYUVContext {
>  LLVidDSPContext   llviddsp;
>  } MagicYUVContext;
>  
> -static int huff_cmp_len(const void *a, const void *b)
> +static int huff_build(HuffEntry he[], uint16_t codes_count[33],
> +  VLC *vlc, int nb_elems)
>  {
> -const HuffEntry *aa = a, *bb = b;
> -return (bb->len - aa->len) * 4096 + aa->sym - bb->sym;
> -}
> -
> -static int huff_build(HuffEntry he[], VLC *vlc, int nb_elems)
> -{
> -uint32_t code;
> -
> -AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
> +unsigned nb_codes = 0, max = 0;
> +
> +for (int i = 32; i > 0; i--) {
> +uint16_t curr = codes_count[i];   // # of leafs of length i
> +codes_count[i] = nb_codes / 2;// # of non-leaf nodes on level i
> +nb_codes = codes_count[i] + curr; // # of nodes on level i
> +if (curr && !max)
> +max = i;
> +}
>  
> -code = 0;
>  for (unsigned i = 0; i < nb_elems; i++) {
> -he[i].code = code >> (32 - he[i].len);
> -code += 0x8000u >> (he[i].len - 1);
> +he[i].code = codes_count[he[i].len];
> +codes_count[he[i].len]++;
>  }
> -
> -ff_free_vlc(vlc);
> -return ff_init_vlc_sparse(vlc, FFMIN(he[0].len, 12), nb_elems,
> +return ff_init_vlc_sparse(vlc, FFMIN(max, 12), nb_elems,
>[0].len,  sizeof(he[0]), sizeof(he[0].len),
>[0].code, sizeof(he[0]), sizeof(he[0].code),
>[0].sym,  sizeof(he[0]), sizeof(he[0].sym), 
>  0);
> @@ -389,6 +386,7 @@ static int build_huffman(AVCodecContext *avctx, const 
> uint8_t *table,
>  MagicYUVContext *s = avctx->priv_data;
>  GetByteContext gb;
>  HuffEntry he[4096];
> +uint16_t length_count[33] = { 0 };
>  int i = 0, j = 0, k;
>  
>  bytestream2_init(, table, table_size);
> @@ -409,6 +407,7 @@ static int build_huffman(AVCodecContext *avctx, const 
> uint8_t *table,
>  return AVERROR_INVALIDDATA;
>  }
>  
> +length_count[x] += l;
>  for (; j < k; j++) {
>  he[j].sym = j;
>  he[j].len = x;
> @@ -416,7 +415,7 @@ static int build_huffman(AVCodecContext *avctx, const 
> uint8_t *table,
>  
>  if (j == max) {
>  

Re: [FFmpeg-devel] [PATCH 10/25] avcodec/magicyuv: Fix building Huffman table

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:49PM +0200, Andreas Rheinhardt wrote:
> The MagicYUV format stores Huffman tables in its bitstream by coding
> the length of a given symbol; it does not code the actual code directly,
> instead this is to be inferred by the rule that a symbol is to the left
> of every shorter symbol in the Huffman tree and that for symbols of the
> same length the symbol is ascending from left to right. With one
> exception, this is also what our decoder did.
> 
> The exception only matters when there are codes of length 32, because
> in this case the first symbol of this length did not get the code 0,
> but 1; e.g. if there were exactly two nodes of length 32, then they
> would get assigned the codes 1 and 2 and a node of length 31 will get
> the 31-bit code 1 which is a prefix of the 32 bit code 2, making the
> Huffman table invalid. On the other hand, if there were only one symbol
> with the length 32, the earlier code would accept this un-Huffman-tree.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

should be ok with subject fixed, it is not fix, its just fixes single
case that never happens in reality.

> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 1b3f4cfc6b..17dea69d76 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -86,7 +86,7 @@ static int huff_build(HuffEntry he[], VLC *vlc, int 
> nb_elems)
>  
>  AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
>  
> -code = 1;
> +code = 0;
>  for (unsigned i = 0; i < nb_elems; i++) {
>  he[i].code = code >> (32 - he[i].len);
>  code += 0x8000u >> (he[i].len - 1);
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 08/25] avcodec/magicyuv: Replace implicit checks for overread by explicit ones

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:43:17PM +0200, Andreas Rheinhardt wrote:
> Paul B Mahol:
> > On Sat, Sep 26, 2020 at 12:27:47PM +0200, Andreas Rheinhardt wrote:
> >> Signed-off-by: Andreas Rheinhardt 
> >> ---
> >>  libavcodec/magicyuv.c | 49 ---
> >>  1 file changed, 27 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> >> index 3413d8f298..93ee739093 100644
> >> --- a/libavcodec/magicyuv.c
> >> +++ b/libavcodec/magicyuv.c
> >> @@ -442,23 +442,26 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> >> void *data,
> >>  MagicYUVContext *s = avctx->priv_data;
> >>  ThreadFrame frame = { .f = data };
> >>  AVFrame *p = data;
> >> -GetByteContext gbyte;
> >> +GetByteContext gb;
> >>  uint32_t first_offset, offset, next_offset, header_size, slice_width;
> >>  int width, height, format, version, table_size;
> >>  int ret, i, j;
> >>  
> >> -bytestream2_init(, avpkt->data, avpkt->size);
> >> -if (bytestream2_get_le32() != MKTAG('M', 'A', 'G', 'Y'))
> >> +if (avpkt->size < 36)
> >> +return AVERROR_INVALIDDATA;
> >> +
> >> +bytestream2_init(, avpkt->data, avpkt->size);
> >> +if (bytestream2_get_le32u() != MKTAG('M', 'A', 'G', 'Y'))
> >>  return AVERROR_INVALIDDATA;
> >>  
> >> -header_size = bytestream2_get_le32();
> >> +header_size = bytestream2_get_le32u();
> >>  if (header_size < 32 || header_size >= avpkt->size) {
> >>  av_log(avctx, AV_LOG_ERROR,
> >> "header or packet too small %"PRIu32"\n", header_size);
> >>  return AVERROR_INVALIDDATA;
> >>  }
> >>  
> >> -version = bytestream2_get_byte();
> >> +version = bytestream2_get_byteu();
> >>  if (version != 7) {
> >>  avpriv_request_sample(avctx, "Version %d", version);
> >>  return AVERROR_PATCHWELCOME;
> >> @@ -471,7 +474,7 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> >> void *data,
> >>  s->decorrelate = 0;
> >>  s->bps = 8;
> >>  
> >> -format = bytestream2_get_byte();
> >> +format = bytestream2_get_byteu();
> >>  switch (format) {
> >>  case 0x65:
> >>  avctx->pix_fmt = AV_PIX_FMT_GBRP;
> >> @@ -552,34 +555,34 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> >> void *data,
> >>  s->magy_decode_slice = s->bps == 8 ? magy_decode_slice : 
> >> magy_decode_slice10;
> >>  s->planes = av_pix_fmt_count_planes(avctx->pix_fmt);
> >>  
> >> -bytestream2_skip(, 1);
> >> -s->color_matrix = bytestream2_get_byte();
> >> -s->flags= bytestream2_get_byte();
> >> +bytestream2_skipu(, 1);
> >> +s->color_matrix = bytestream2_get_byteu();
> >> +s->flags= bytestream2_get_byteu();
> >>  s->interlaced   = !!(s->flags & 2);
> >> -bytestream2_skip(, 3);
> >> +bytestream2_skipu(, 3);
> >>  
> >> -width  = bytestream2_get_le32();
> >> -height = bytestream2_get_le32();
> >> +width  = bytestream2_get_le32u();
> >> +height = bytestream2_get_le32u();
> >>  ret = ff_set_dimensions(avctx, width, height);
> >>  if (ret < 0)
> >>  return ret;
> >>  
> >> -slice_width = bytestream2_get_le32();
> >> +slice_width = bytestream2_get_le32u();
> >>  if (slice_width != avctx->coded_width) {
> >>  avpriv_request_sample(avctx, "Slice width %"PRIu32, slice_width);
> >>  return AVERROR_PATCHWELCOME;
> >>  }
> >> -s->slice_height = bytestream2_get_le32();
> >> +s->slice_height = bytestream2_get_le32u();
> >>  if (s->slice_height <= 0 || s->slice_height > INT_MAX - 
> >> avctx->coded_height) {
> >>  av_log(avctx, AV_LOG_ERROR,
> >> "invalid slice height: %d\n", s->slice_height);
> >>  return AVERROR_INVALIDDATA;
> >>  }
> >>  
> >> -bytestream2_skip(, 4);
> >> +bytestream2_skipu(, 4);
> >>  
> >>  s->nb_slices = (avctx->coded_height + s->slice_height - 1) / 
> >> s->slice_height;
> >> -if (s->nb_slices > INT_MAX / sizeof(Slice)) {
> >> +if (s->nb_slices > INT_MAX / FFMAX(sizeof(Slice), 4 * 5)) {
> >>  av_log(avctx, AV_LOG_ERROR,
> >> "invalid number of slices: %d\n", s->nb_slices);
> >>  return AVERROR_INVALIDDATA;
> >> @@ -596,12 +599,14 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> >> void *data,
> >>  }
> >>  }
> >>  
> >> +if (bytestream2_get_bytes_left() <= s->nb_slices * s->planes * 5)
> >> +return AVERROR_INVALIDDATA;
> > 
> > From where you picked this 5 number?
> > 
> 
> 5 = 4 + 1. The four corresponds to the four byte read performed in the
> loop below; the 1 corresponds to the skip below. <= instead of < because
> of the byte containing the number of planes.

patch is ok

> 
> >>  for (i = 0; i < s->planes; i++) {
> >>  av_fast_malloc(>slices[i], >slices_size[i], s->nb_slices * 
> >> sizeof(Slice));
> >>  if (!s->slices[i])
> >> 

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-26 Thread Michael Niedermayer
On Thu, Sep 17, 2020 at 12:31:06PM +0200, Paul B Mahol wrote:
> This removes big CPU overhead for demuxing chained ogg streams.
> 
> Signed-off-by: Paul B Mahol 
> ---
>  libavformat/aviobuf.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)

I think we need some fate test for ffio_ensure_seekback()
as our tests are ATM we generally have fully seekable inputs so
failure of seekback would not be noticed.

This is a problem because the buffer IO stuff is not easy to
understand and so it is time consuming and error prone to review these patches
just by eye. And as we see developers disagree on what is correct

Such fate test should at least test various sequentially occuring
ffio_ensure_seekback(), a possibly larger initial buffer from probing
as well as its later reduction. And also reading requests failing, early EOF
seeking, and IO errors

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The day soldiers stop bringing you their problems is the day you have stopped 
leading them. They have either lost confidence that you can help or concluded 
you do not care. Either case is a failure of leadership. - Colin Powell


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 06/25] avcodec/magicyuv: Don't use GetBit API for byte-aligned reads

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:45PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 45 +--
>  1 file changed, 22 insertions(+), 23 deletions(-)

lgtm

> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 6c29efc9f4..f7dfef0eb8 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -270,27 +270,26 @@ static int magy_decode_slice(AVCodecContext *avctx, 
> void *tdata,
>  int sheight = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]);
>  ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
>  ptrdiff_t stride = p->linesize[i];
> +const uint8_t *slice = s->buf + s->slices[i][j].start;
>  int flags, pred;
> -int ret = init_get_bits8(, s->buf + s->slices[i][j].start,
> - s->slices[i][j].size);
> -
> -if (ret < 0)
> -return ret;
>  
> -flags = get_bits(, 8);
> -pred  = get_bits(, 8);
> +flags = bytestream_get_byte();
> +pred  = bytestream_get_byte();
>  
>  dst = p->data[i] + j * sheight * stride;
>  if (flags & 1) {
> -if (get_bits_left() < 8* width * height)
> +if (s->slices[i][j].size - 2 < width * height)
>  return AVERROR_INVALIDDATA;
>  for (k = 0; k < height; k++) {
> -for (x = 0; x < width; x++)
> -dst[x] = get_bits(, 8);
> -
> +bytestream_get_buffer(, dst, width);
>  dst += stride;
>  }
>  } else {
> +int ret = init_get_bits8(, slice, s->slices[i][j].size - 2);
> +
> +if (ret < 0)
> +return ret;
> +
>  for (k = 0; k < height; k++) {
>  for (x = 0; x < width; x++) {
>  int pix;
> @@ -385,21 +384,25 @@ static int magy_decode_slice(AVCodecContext *avctx, 
> void *tdata,
>  return 0;
>  }
>  
> -static int build_huffman(AVCodecContext *avctx, GetBitContext *gbit, int max)
> +static int build_huffman(AVCodecContext *avctx, const uint8_t *table,
> + int table_size, int max)
>  {
>  MagicYUVContext *s = avctx->priv_data;
> +GetByteContext gb;
>  HuffEntry he[4096];
>  int i = 0, j = 0, k;
>  
> -while (get_bits_left(gbit) >= 8) {
> -int b = get_bits(gbit, 1);
> -int x = get_bits(gbit, 7);
> +bytestream2_init(, table, table_size);
> +
> +while (bytestream2_get_bytes_left() > 0) {
> +int b = bytestream2_peek_byteu() &  0x80;
> +int x = bytestream2_get_byteu()  & ~0x80;
>  int l = 1;
>  
>  if (b) {
> -if (get_bits_left(gbit) < 8)
> +if (bytestream2_get_bytes_left() <= 0)
>  break;
> -l += get_bits(gbit, 8);
> +l += bytestream2_get_byteu();
>  }
>  k = j + l;
>  if (k > max || x == 0 || x > 32) {
> @@ -440,7 +443,6 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
> *data,
>  ThreadFrame frame = { .f = data };
>  AVFrame *p = data;
>  GetByteContext gbyte;
> -GetBitContext gbit;
>  uint32_t first_offset, offset, next_offset, header_size, slice_width;
>  int width, height, format, version, table_size;
>  int ret, i, j;
> @@ -632,11 +634,8 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
> *data,
>  if (table_size < 2)
>  return AVERROR_INVALIDDATA;
>  
> -ret = init_get_bits8(, avpkt->data + bytestream2_tell(), 
> table_size);
> -if (ret < 0)
> -return ret;
> -
> -ret = build_huffman(avctx, , s->max);
> +ret = build_huffman(avctx, avpkt->data + bytestream2_tell(),
> +table_size, s->max);
>  if (ret < 0)
>  return ret;
>  
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 01/25] avcodec/photocd: Simplify parsing Huffman tables a bit

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:40PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/photocd.c | 13 ++---
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 

should be ok if tested.

> diff --git a/libavcodec/photocd.c b/libavcodec/photocd.c
> index 057c9d33d4..8fd4536a65 100644
> --- a/libavcodec/photocd.c
> +++ b/libavcodec/photocd.c
> @@ -245,21 +245,20 @@ static av_noinline int decode_huff(AVCodecContext 
> *avctx, AVFrame *frame,
>  int x2, idx;
>  
>  for (; get_bits_left() > 0;) {
> -if ((show_bits(, 24) & 0xfff000) == 0xfff000)
> +if (show_bits(, 12) == 0xfff)
>  break;
>  skip_bits(, 8);
>  }
>  
> -shiftreg = show_bits_long(, 32) & 0xff00;
> -while (shiftreg != 0xfe00) {
> +shiftreg = show_bits(, 24);
> +while (shiftreg != 0xfe) {
>  if (get_bits_left() <= 0)
>  return AVERROR_INVALIDDATA;
>  skip_bits(, 1);
> -shiftreg = show_bits_long(, 32) & 0xff00;
> +shiftreg = show_bits(, 24);
>  }
> -skip_bits(, 16);
> -y = show_bits_long(, 23) & 0x1fff;
> -skip_bits(, 8);
> +skip_bits(, 24);
> +y = show_bits(, 15) & 0x1fff;
>  if (y >= height)
>  break;
>  type = get_bits(, 2);
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 02/25] avcodec/bytestream: Add unchecked bytestream2 peek functions

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:41PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/bytestream.h | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

lgtm

> 
> diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h
> index 0516a6e3dc..d0033f14f3 100644
> --- a/libavcodec/bytestream.h
> +++ b/libavcodec/bytestream.h
> @@ -77,11 +77,15 @@ static av_always_inline type bytestream2_get_ ## 
> name(GetByteContext *g)   \
>  }
>   \
>  return bytestream2_get_ ## name ## u(g); 
>   \
>  }
>   \
> +static av_always_inline type bytestream2_peek_ ## name ## u(GetByteContext 
> *g) \
> +{
>   \
> +return read(g->buffer);  
>   \
> +}
>   \
>  static av_always_inline type bytestream2_peek_ ## name(GetByteContext *g)
>   \
>  {
>   \
>  if (g->buffer_end - g->buffer < bytes)   
>   \
>  return 0;
>   \
> -return read(g->buffer);  
>   \
> +return bytestream2_peek_ ## name ## u(g);
>   \
>  }
>  
>  DEF(uint64_t, le64, 8, AV_RL64, AV_WL64)
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/aviobuf: fix checks in ffio_ensure_seekback

2020-09-26 Thread Marton Balint



On Sat, 26 Sep 2020, Michael Niedermayer wrote:


On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:

Signed-off-by: Marton Balint 
---
 libavformat/aviobuf.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)


The commit message is too terse. It is not clear from it what the problem
is that is being fixed and how it is fixed.
Also this feels like it fixes multiple issues so it probably should be spit
unless iam missing some interdependancies


It can be seperated to two patches if that is preferred:

1) existing code does not check if the requested seekback buffer is 
already read entirely. In this case, nothing has to be done. This is the 
newly added if() at the beginning of the patch.


2) the new buf_size is detemined too conservatively, maybe because of the 
issue which was fixed in patch 1/3. So we can safely substract 1 more from 
the new buffer size. Comparing the new buf_size against filled does not 
make a lot of sense to me, that just seems wrong. What makes sense is that 
we want to reallocate the buffer if the new buf_size is bigger than the 
old, therefore the change in the check.




ill also take a look at the "competing" patch, so i dont yet know
how they relate ...




diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 9675425349..aa1d6c0830 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -999,9 +999,12 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t buf_size)
 int filled = s->buf_end - s->buffer;
 ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
s->buffer : -1;

-buf_size += s->buf_ptr - s->buffer + max_buffer_size;
+if (buf_size <= s->buf_end - s->buf_ptr)
+return 0;
+
+buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;




-if (buf_size < filled || s->seekable || !s->read_packet)
+if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
 return 0;


Did you check that this doesnt interfere with the s->buffer_size reduction
we do when it was larger from probing ?
Its maybe ok, just checking that this was considered


I don't see how that can be affected by this change, so I am not sure what 
you mean here. If the new buffer size is bigger than s->orig_buffer_size, 
then eventually it will be reduced, it does not seem more complicated than 
that to me.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 25/25] avcodec/fraps: Use cached bitstream reader

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:28:04PM +0200, Andreas Rheinhardt wrote:
> This proved beneficial for performance: For the sample [1] the number
> of decicycles in one decode call decreased from 155851561 to 108158037
> for Clang 10 and from 168270467 to 128847479 for GCC 9.3. For x86-32
> compiled with GCC 9.3 and run on an x64 Haswell the number increased
> from 158405517 to 202215769, so that the cached bitstream reader is only
> enabled if HAVE_FAST_64BIT is set. These values are the average of 10
> runs each looping five times over the input.
> 
> [1]: 
> samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2593/fraps_flv1_decoding_errors.avi
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/fraps.c | 3 +++
>  1 file changed, 3 insertions(+)

probably ok

> 
> diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
> index 00fd63ffec..8d01b44f11 100644
> --- a/libavcodec/fraps.c
> +++ b/libavcodec/fraps.c
> @@ -31,6 +31,9 @@
>   * Version 2 files support by Konstantin Shishkov
>   */
>  
> +#include "config.h"
> +
> +#define CACHED_BITSTREAM_READER HAVE_FAST_64BIT
>  #define UNCHECKED_BITSTREAM_READER 1
>  #include "avcodec.h"
>  #include "get_bits.h"
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 24/25] avcodec/fraps: Use unchecked bitstream reader

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:28:03PM +0200, Andreas Rheinhardt wrote:
> The fraps decoder already checked for overreads manually (and errored
> out in this scenario), yet it still enabled implicit checks, leading to
> worse performance and more code size.
> 
> This commit disables the implicit bitstream reader checks. For the
> sample [1] this improves performance from 195105896 to 155851561
> decicycles for Clang 10 and from 222801887 to 168270467 decicycles when
> compiled with GCC 9.3. These values are the average of 10 runs each
> looping ten times over the input.
> 
> [1]: 
> samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2593/fraps_flv1_decoding_errors.avi

probably ok

> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/fraps.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
> index 7a7673f73f..00fd63ffec 100644
> --- a/libavcodec/fraps.c
> +++ b/libavcodec/fraps.c
> @@ -31,6 +31,7 @@
>   * Version 2 files support by Konstantin Shishkov
>   */
>  
> +#define UNCHECKED_BITSTREAM_READER 1
>  #include "avcodec.h"
>  #include "get_bits.h"
>  #include "huffman.h"
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 03/25] avcodec/magicyuv: Improve overread check when parsing Huffman tables

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:42PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 

lgtm

> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index b56d3e9d32..d2f6a9b01e 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -394,8 +394,13 @@ static int build_huffman(AVCodecContext *avctx, 
> GetBitContext *gbit, int max)
>  while (get_bits_left(gbit) >= 8) {
>  int b = get_bits(gbit, 1);
>  int x = get_bits(gbit, 7);
> -int l = get_bitsz(gbit, b * 8) + 1;
> +int l = 1;
>  
> +if (b) {
> +if (get_bits_left(gbit) < 8)
> +break;
> +l += get_bits(gbit, 8);
> +}
>  k = j + l;
>  if (k > max || x == 0 || x > 32) {
>  av_log(avctx, AV_LOG_ERROR, "Invalid Huffman codes\n");
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 08/25] avcodec/magicyuv: Replace implicit checks for overread by explicit ones

2020-09-26 Thread Andreas Rheinhardt
Paul B Mahol:
> On Sat, Sep 26, 2020 at 12:27:47PM +0200, Andreas Rheinhardt wrote:
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>>  libavcodec/magicyuv.c | 49 ---
>>  1 file changed, 27 insertions(+), 22 deletions(-)
>>
>> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
>> index 3413d8f298..93ee739093 100644
>> --- a/libavcodec/magicyuv.c
>> +++ b/libavcodec/magicyuv.c
>> @@ -442,23 +442,26 @@ static int magy_decode_frame(AVCodecContext *avctx, 
>> void *data,
>>  MagicYUVContext *s = avctx->priv_data;
>>  ThreadFrame frame = { .f = data };
>>  AVFrame *p = data;
>> -GetByteContext gbyte;
>> +GetByteContext gb;
>>  uint32_t first_offset, offset, next_offset, header_size, slice_width;
>>  int width, height, format, version, table_size;
>>  int ret, i, j;
>>  
>> -bytestream2_init(, avpkt->data, avpkt->size);
>> -if (bytestream2_get_le32() != MKTAG('M', 'A', 'G', 'Y'))
>> +if (avpkt->size < 36)
>> +return AVERROR_INVALIDDATA;
>> +
>> +bytestream2_init(, avpkt->data, avpkt->size);
>> +if (bytestream2_get_le32u() != MKTAG('M', 'A', 'G', 'Y'))
>>  return AVERROR_INVALIDDATA;
>>  
>> -header_size = bytestream2_get_le32();
>> +header_size = bytestream2_get_le32u();
>>  if (header_size < 32 || header_size >= avpkt->size) {
>>  av_log(avctx, AV_LOG_ERROR,
>> "header or packet too small %"PRIu32"\n", header_size);
>>  return AVERROR_INVALIDDATA;
>>  }
>>  
>> -version = bytestream2_get_byte();
>> +version = bytestream2_get_byteu();
>>  if (version != 7) {
>>  avpriv_request_sample(avctx, "Version %d", version);
>>  return AVERROR_PATCHWELCOME;
>> @@ -471,7 +474,7 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
>> *data,
>>  s->decorrelate = 0;
>>  s->bps = 8;
>>  
>> -format = bytestream2_get_byte();
>> +format = bytestream2_get_byteu();
>>  switch (format) {
>>  case 0x65:
>>  avctx->pix_fmt = AV_PIX_FMT_GBRP;
>> @@ -552,34 +555,34 @@ static int magy_decode_frame(AVCodecContext *avctx, 
>> void *data,
>>  s->magy_decode_slice = s->bps == 8 ? magy_decode_slice : 
>> magy_decode_slice10;
>>  s->planes = av_pix_fmt_count_planes(avctx->pix_fmt);
>>  
>> -bytestream2_skip(, 1);
>> -s->color_matrix = bytestream2_get_byte();
>> -s->flags= bytestream2_get_byte();
>> +bytestream2_skipu(, 1);
>> +s->color_matrix = bytestream2_get_byteu();
>> +s->flags= bytestream2_get_byteu();
>>  s->interlaced   = !!(s->flags & 2);
>> -bytestream2_skip(, 3);
>> +bytestream2_skipu(, 3);
>>  
>> -width  = bytestream2_get_le32();
>> -height = bytestream2_get_le32();
>> +width  = bytestream2_get_le32u();
>> +height = bytestream2_get_le32u();
>>  ret = ff_set_dimensions(avctx, width, height);
>>  if (ret < 0)
>>  return ret;
>>  
>> -slice_width = bytestream2_get_le32();
>> +slice_width = bytestream2_get_le32u();
>>  if (slice_width != avctx->coded_width) {
>>  avpriv_request_sample(avctx, "Slice width %"PRIu32, slice_width);
>>  return AVERROR_PATCHWELCOME;
>>  }
>> -s->slice_height = bytestream2_get_le32();
>> +s->slice_height = bytestream2_get_le32u();
>>  if (s->slice_height <= 0 || s->slice_height > INT_MAX - 
>> avctx->coded_height) {
>>  av_log(avctx, AV_LOG_ERROR,
>> "invalid slice height: %d\n", s->slice_height);
>>  return AVERROR_INVALIDDATA;
>>  }
>>  
>> -bytestream2_skip(, 4);
>> +bytestream2_skipu(, 4);
>>  
>>  s->nb_slices = (avctx->coded_height + s->slice_height - 1) / 
>> s->slice_height;
>> -if (s->nb_slices > INT_MAX / sizeof(Slice)) {
>> +if (s->nb_slices > INT_MAX / FFMAX(sizeof(Slice), 4 * 5)) {
>>  av_log(avctx, AV_LOG_ERROR,
>> "invalid number of slices: %d\n", s->nb_slices);
>>  return AVERROR_INVALIDDATA;
>> @@ -596,12 +599,14 @@ static int magy_decode_frame(AVCodecContext *avctx, 
>> void *data,
>>  }
>>  }
>>  
>> +if (bytestream2_get_bytes_left() <= s->nb_slices * s->planes * 5)
>> +return AVERROR_INVALIDDATA;
> 
> From where you picked this 5 number?
> 

5 = 4 + 1. The four corresponds to the four byte read performed in the
loop below; the 1 corresponds to the skip below. <= instead of < because
of the byte containing the number of planes.

>>  for (i = 0; i < s->planes; i++) {
>>  av_fast_malloc(>slices[i], >slices_size[i], s->nb_slices * 
>> sizeof(Slice));
>>  if (!s->slices[i])
>>  return AVERROR(ENOMEM);
>>  
>> -offset = bytestream2_get_le32();
>> +offset = bytestream2_get_le32u();
>>  if (offset >= avpkt->size - header_size)
>>  return AVERROR_INVALIDDATA;
>>  
>> @@ -611,7 +616,7 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
>> *data,
>>

Re: [FFmpeg-devel] [PATCH 04/25] avcodec/diracdsp: Remove unused variable

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:43PM +0200, Andreas Rheinhardt wrote:
> Forgotten in ca3c6c981aa5b0af8a5576020b79fdd3cdf9ae9e.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/diracdsp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

lgtm

> diff --git a/libavcodec/diracdsp.c b/libavcodec/diracdsp.c
> index 4e08d3817e..80dfafd78b 100644
> --- a/libavcodec/diracdsp.c
> +++ b/libavcodec/diracdsp.c
> @@ -195,7 +195,7 @@ static void dequant_subband_ ## PX ## _c(uint8_t *src, 
> uint8_t *dst, ptrdiff_t s
>  {
>   \
>  int i, y;
>   \
>  for (y = 0; y < tot_v; y++) {
>   \
> -PX c, sign, *src_r = (PX *)src, *dst_r = (PX *)dst;  
>   \
> +PX c, *src_r = (PX *)src, *dst_r = (PX *)dst;
>   \
>  for (i = 0; i < tot_h; i++) {
>   \
>  c = *src_r++;
>   \
>  if (c < 0) c = -((-(unsigned)c*qf + qs) >> 2);   
>   \
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 05/25] avcodec/magicyuv: Check early for invalid slices

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:44PM +0200, Andreas Rheinhardt wrote:
> Every plane of each slice has to contain at least two bytes for flags
> and the type of prediction used.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 2 ++
>  1 file changed, 2 insertions(+)

lgtm

> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index d2f6a9b01e..6c29efc9f4 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -614,6 +614,8 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
> *data,
>  return AVERROR_INVALIDDATA;
>  
>  s->slices[i][j].size = next_offset - offset;
> +if (s->slices[i][j].size < 2)
> +return AVERROR_INVALIDDATA;
>  offset = next_offset;
>  }
>  
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 07/25] avcodec/magicyuv: Use const uint8_t* for pointer to immutable data

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:46PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

lgtm

> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index f7dfef0eb8..3413d8f298 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -63,7 +63,7 @@ typedef struct MagicYUVContext {
>  int   color_matrix;   // video color matrix
>  int   flags;
>  int   interlaced; // video is interlaced
> -uint8_t  *buf;// pointer to AVPacket->data
> +const uint8_t*buf;// pointer to AVPacket->data
>  int   hshift[4];
>  int   vshift[4];
>  Slice*slices[4];  // slice bitstream positions for each 
> plane
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 08/25] avcodec/magicyuv: Replace implicit checks for overread by explicit ones

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:47PM +0200, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/magicyuv.c | 49 ---
>  1 file changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
> index 3413d8f298..93ee739093 100644
> --- a/libavcodec/magicyuv.c
> +++ b/libavcodec/magicyuv.c
> @@ -442,23 +442,26 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> void *data,
>  MagicYUVContext *s = avctx->priv_data;
>  ThreadFrame frame = { .f = data };
>  AVFrame *p = data;
> -GetByteContext gbyte;
> +GetByteContext gb;
>  uint32_t first_offset, offset, next_offset, header_size, slice_width;
>  int width, height, format, version, table_size;
>  int ret, i, j;
>  
> -bytestream2_init(, avpkt->data, avpkt->size);
> -if (bytestream2_get_le32() != MKTAG('M', 'A', 'G', 'Y'))
> +if (avpkt->size < 36)
> +return AVERROR_INVALIDDATA;
> +
> +bytestream2_init(, avpkt->data, avpkt->size);
> +if (bytestream2_get_le32u() != MKTAG('M', 'A', 'G', 'Y'))
>  return AVERROR_INVALIDDATA;
>  
> -header_size = bytestream2_get_le32();
> +header_size = bytestream2_get_le32u();
>  if (header_size < 32 || header_size >= avpkt->size) {
>  av_log(avctx, AV_LOG_ERROR,
> "header or packet too small %"PRIu32"\n", header_size);
>  return AVERROR_INVALIDDATA;
>  }
>  
> -version = bytestream2_get_byte();
> +version = bytestream2_get_byteu();
>  if (version != 7) {
>  avpriv_request_sample(avctx, "Version %d", version);
>  return AVERROR_PATCHWELCOME;
> @@ -471,7 +474,7 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
> *data,
>  s->decorrelate = 0;
>  s->bps = 8;
>  
> -format = bytestream2_get_byte();
> +format = bytestream2_get_byteu();
>  switch (format) {
>  case 0x65:
>  avctx->pix_fmt = AV_PIX_FMT_GBRP;
> @@ -552,34 +555,34 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> void *data,
>  s->magy_decode_slice = s->bps == 8 ? magy_decode_slice : 
> magy_decode_slice10;
>  s->planes = av_pix_fmt_count_planes(avctx->pix_fmt);
>  
> -bytestream2_skip(, 1);
> -s->color_matrix = bytestream2_get_byte();
> -s->flags= bytestream2_get_byte();
> +bytestream2_skipu(, 1);
> +s->color_matrix = bytestream2_get_byteu();
> +s->flags= bytestream2_get_byteu();
>  s->interlaced   = !!(s->flags & 2);
> -bytestream2_skip(, 3);
> +bytestream2_skipu(, 3);
>  
> -width  = bytestream2_get_le32();
> -height = bytestream2_get_le32();
> +width  = bytestream2_get_le32u();
> +height = bytestream2_get_le32u();
>  ret = ff_set_dimensions(avctx, width, height);
>  if (ret < 0)
>  return ret;
>  
> -slice_width = bytestream2_get_le32();
> +slice_width = bytestream2_get_le32u();
>  if (slice_width != avctx->coded_width) {
>  avpriv_request_sample(avctx, "Slice width %"PRIu32, slice_width);
>  return AVERROR_PATCHWELCOME;
>  }
> -s->slice_height = bytestream2_get_le32();
> +s->slice_height = bytestream2_get_le32u();
>  if (s->slice_height <= 0 || s->slice_height > INT_MAX - 
> avctx->coded_height) {
>  av_log(avctx, AV_LOG_ERROR,
> "invalid slice height: %d\n", s->slice_height);
>  return AVERROR_INVALIDDATA;
>  }
>  
> -bytestream2_skip(, 4);
> +bytestream2_skipu(, 4);
>  
>  s->nb_slices = (avctx->coded_height + s->slice_height - 1) / 
> s->slice_height;
> -if (s->nb_slices > INT_MAX / sizeof(Slice)) {
> +if (s->nb_slices > INT_MAX / FFMAX(sizeof(Slice), 4 * 5)) {
>  av_log(avctx, AV_LOG_ERROR,
> "invalid number of slices: %d\n", s->nb_slices);
>  return AVERROR_INVALIDDATA;
> @@ -596,12 +599,14 @@ static int magy_decode_frame(AVCodecContext *avctx, 
> void *data,
>  }
>  }
>  
> +if (bytestream2_get_bytes_left() <= s->nb_slices * s->planes * 5)
> +return AVERROR_INVALIDDATA;

From where you picked this 5 number?

>  for (i = 0; i < s->planes; i++) {
>  av_fast_malloc(>slices[i], >slices_size[i], s->nb_slices * 
> sizeof(Slice));
>  if (!s->slices[i])
>  return AVERROR(ENOMEM);
>  
> -offset = bytestream2_get_le32();
> +offset = bytestream2_get_le32u();
>  if (offset >= avpkt->size - header_size)
>  return AVERROR_INVALIDDATA;
>  
> @@ -611,7 +616,7 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
> *data,
>  for (j = 0; j < s->nb_slices - 1; j++) {
>  s->slices[i][j].start = offset + header_size;
>  
> -next_offset = bytestream2_get_le32();
> +next_offset = bytestream2_get_le32u();
>  if (next_offset <= offset || next_offset >= avpkt->size - 
> header_size)
>  return 

[FFmpeg-devel] [PATCH 10/25] avcodec/magicyuv: Fix building Huffman table

2020-09-26 Thread Andreas Rheinhardt
The MagicYUV format stores Huffman tables in its bitstream by coding
the length of a given symbol; it does not code the actual code directly,
instead this is to be inferred by the rule that a symbol is to the left
of every shorter symbol in the Huffman tree and that for symbols of the
same length the symbol is ascending from left to right. With one
exception, this is also what our decoder did.

The exception only matters when there are codes of length 32, because
in this case the first symbol of this length did not get the code 0,
but 1; e.g. if there were exactly two nodes of length 32, then they
would get assigned the codes 1 and 2 and a node of length 31 will get
the 31-bit code 1 which is a prefix of the 32 bit code 2, making the
Huffman table invalid. On the other hand, if there were only one symbol
with the length 32, the earlier code would accept this un-Huffman-tree.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 1b3f4cfc6b..17dea69d76 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -86,7 +86,7 @@ static int huff_build(HuffEntry he[], VLC *vlc, int nb_elems)
 
 AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
 
-code = 1;
+code = 0;
 for (unsigned i = 0; i < nb_elems; i++) {
 he[i].code = code >> (32 - he[i].len);
 code += 0x8000u >> (he[i].len - 1);
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/aviobuf: fix broken logic in ffio_ensure_seekback()

2020-09-26 Thread Michael Niedermayer
On Sun, Sep 20, 2020 at 10:49:12AM +0200, Marton Balint wrote:
[...]

> fundamental problem is that ffio_ensure_seekback simply cannot be
> implemented efficiently with the current design. The guarantees should be
> reduced (e.g. ffio_ensure_seekback can flush the buffer and invalidate
> previous ffio_ensure_seekback request)

this probably makes sense, would this cause a problem to any code using it ?

thx

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 06/25] avcodec/magicyuv: Don't use GetBit API for byte-aligned reads

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 45 +--
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 6c29efc9f4..f7dfef0eb8 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -270,27 +270,26 @@ static int magy_decode_slice(AVCodecContext *avctx, void 
*tdata,
 int sheight = AV_CEIL_RSHIFT(s->slice_height, s->vshift[i]);
 ptrdiff_t fake_stride = p->linesize[i] * (1 + interlaced);
 ptrdiff_t stride = p->linesize[i];
+const uint8_t *slice = s->buf + s->slices[i][j].start;
 int flags, pred;
-int ret = init_get_bits8(, s->buf + s->slices[i][j].start,
- s->slices[i][j].size);
-
-if (ret < 0)
-return ret;
 
-flags = get_bits(, 8);
-pred  = get_bits(, 8);
+flags = bytestream_get_byte();
+pred  = bytestream_get_byte();
 
 dst = p->data[i] + j * sheight * stride;
 if (flags & 1) {
-if (get_bits_left() < 8* width * height)
+if (s->slices[i][j].size - 2 < width * height)
 return AVERROR_INVALIDDATA;
 for (k = 0; k < height; k++) {
-for (x = 0; x < width; x++)
-dst[x] = get_bits(, 8);
-
+bytestream_get_buffer(, dst, width);
 dst += stride;
 }
 } else {
+int ret = init_get_bits8(, slice, s->slices[i][j].size - 2);
+
+if (ret < 0)
+return ret;
+
 for (k = 0; k < height; k++) {
 for (x = 0; x < width; x++) {
 int pix;
@@ -385,21 +384,25 @@ static int magy_decode_slice(AVCodecContext *avctx, void 
*tdata,
 return 0;
 }
 
-static int build_huffman(AVCodecContext *avctx, GetBitContext *gbit, int max)
+static int build_huffman(AVCodecContext *avctx, const uint8_t *table,
+ int table_size, int max)
 {
 MagicYUVContext *s = avctx->priv_data;
+GetByteContext gb;
 HuffEntry he[4096];
 int i = 0, j = 0, k;
 
-while (get_bits_left(gbit) >= 8) {
-int b = get_bits(gbit, 1);
-int x = get_bits(gbit, 7);
+bytestream2_init(, table, table_size);
+
+while (bytestream2_get_bytes_left() > 0) {
+int b = bytestream2_peek_byteu() &  0x80;
+int x = bytestream2_get_byteu()  & ~0x80;
 int l = 1;
 
 if (b) {
-if (get_bits_left(gbit) < 8)
+if (bytestream2_get_bytes_left() <= 0)
 break;
-l += get_bits(gbit, 8);
+l += bytestream2_get_byteu();
 }
 k = j + l;
 if (k > max || x == 0 || x > 32) {
@@ -440,7 +443,6 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 ThreadFrame frame = { .f = data };
 AVFrame *p = data;
 GetByteContext gbyte;
-GetBitContext gbit;
 uint32_t first_offset, offset, next_offset, header_size, slice_width;
 int width, height, format, version, table_size;
 int ret, i, j;
@@ -632,11 +634,8 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 if (table_size < 2)
 return AVERROR_INVALIDDATA;
 
-ret = init_get_bits8(, avpkt->data + bytestream2_tell(), 
table_size);
-if (ret < 0)
-return ret;
-
-ret = build_huffman(avctx, , s->max);
+ret = build_huffman(avctx, avpkt->data + bytestream2_tell(),
+table_size, s->max);
 if (ret < 0)
 return ret;
 
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 02/25] avcodec/bytestream: Add unchecked bytestream2 peek functions

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/bytestream.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h
index 0516a6e3dc..d0033f14f3 100644
--- a/libavcodec/bytestream.h
+++ b/libavcodec/bytestream.h
@@ -77,11 +77,15 @@ static av_always_inline type bytestream2_get_ ## 
name(GetByteContext *g)   \
 }  
\
 return bytestream2_get_ ## name ## u(g);   
\
 }  
\
+static av_always_inline type bytestream2_peek_ ## name ## u(GetByteContext *g) 
\
+{  
\
+return read(g->buffer);
\
+}  
\
 static av_always_inline type bytestream2_peek_ ## name(GetByteContext *g)  
\
 {  
\
 if (g->buffer_end - g->buffer < bytes) 
\
 return 0;  
\
-return read(g->buffer);
\
+return bytestream2_peek_ ## name ## u(g);  
\
 }
 
 DEF(uint64_t, le64, 8, AV_RL64, AV_WL64)
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 01/25] avcodec/photocd: Simplify parsing Huffman tables a bit

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/photocd.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/libavcodec/photocd.c b/libavcodec/photocd.c
index 057c9d33d4..8fd4536a65 100644
--- a/libavcodec/photocd.c
+++ b/libavcodec/photocd.c
@@ -245,21 +245,20 @@ static av_noinline int decode_huff(AVCodecContext *avctx, 
AVFrame *frame,
 int x2, idx;
 
 for (; get_bits_left() > 0;) {
-if ((show_bits(, 24) & 0xfff000) == 0xfff000)
+if (show_bits(, 12) == 0xfff)
 break;
 skip_bits(, 8);
 }
 
-shiftreg = show_bits_long(, 32) & 0xff00;
-while (shiftreg != 0xfe00) {
+shiftreg = show_bits(, 24);
+while (shiftreg != 0xfe) {
 if (get_bits_left() <= 0)
 return AVERROR_INVALIDDATA;
 skip_bits(, 1);
-shiftreg = show_bits_long(, 32) & 0xff00;
+shiftreg = show_bits(, 24);
 }
-skip_bits(, 16);
-y = show_bits_long(, 23) & 0x1fff;
-skip_bits(, 8);
+skip_bits(, 24);
+y = show_bits(, 15) & 0x1fff;
 if (y >= height)
 break;
 type = get_bits(, 2);
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 14/25] avcodec/utvideodec: Remove code duplication when creating Huffman tables

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utvideodec.c | 55 +
 1 file changed, 6 insertions(+), 49 deletions(-)

diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
index b3c4c3519b..f014e90606 100644
--- a/libavcodec/utvideodec.c
+++ b/libavcodec/utvideodec.c
@@ -40,7 +40,7 @@
 #include "thread.h"
 #include "utvideo.h"
 
-static int build_huff10(const uint8_t *src, VLC *vlc, int *fsym)
+static int build_huff(const uint8_t *src, VLC *vlc, int *fsym, unsigned 
nb_elems)
 {
 int i;
 HuffEntry he[1024];
@@ -51,18 +51,18 @@ static int build_huff10(const uint8_t *src, VLC *vlc, int 
*fsym)
 uint32_t code;
 
 *fsym = -1;
-for (i = 0; i < 1024; i++) {
+for (i = 0; i < nb_elems; i++) {
 he[i].sym = i;
 he[i].len = *src++;
 }
-qsort(he, 1024, sizeof(*he), ff_ut10_huff_cmp_len);
+qsort(he, nb_elems, sizeof(*he), ff_ut10_huff_cmp_len);
 
 if (!he[0].len) {
 *fsym = he[0].sym;
 return 0;
 }
 
-last = 1023;
+last = nb_elems - 1;
 while (he[last].len == 255 && last)
 last--;
 
@@ -84,49 +84,6 @@ static int build_huff10(const uint8_t *src, VLC *vlc, int 
*fsym)
   syms,  sizeof(*syms),  sizeof(*syms), 0);
 }
 
-static int build_huff(const uint8_t *src, VLC *vlc, int *fsym)
-{
-int i;
-HuffEntry he[256];
-int last;
-uint32_t codes[256];
-uint8_t bits[256];
-uint8_t syms[256];
-uint32_t code;
-
-*fsym = -1;
-for (i = 0; i < 256; i++) {
-he[i].sym = i;
-he[i].len = *src++;
-}
-qsort(he, 256, sizeof(*he), ff_ut_huff_cmp_len);
-
-if (!he[0].len) {
-*fsym = he[0].sym;
-return 0;
-}
-
-last = 255;
-while (he[last].len == 255 && last)
-last--;
-
-if (he[last].len > 32)
-return -1;
-
-code = 0;
-for (i = last; i >= 0; i--) {
-codes[i] = code >> (32 - he[i].len);
-bits[i]  = he[i].len;
-syms[i]  = he[i].sym;
-code += 0x8000u >> (he[i].len - 1);
-}
-
-return ff_init_vlc_sparse(vlc, VLC_BITS, last + 1,
-  bits,  sizeof(*bits),  sizeof(*bits),
-  codes, sizeof(*codes), sizeof(*codes),
-  syms,  sizeof(*syms),  sizeof(*syms), 0);
-}
-
 static int decode_plane10(UtvideoContext *c, int plane_no,
   uint16_t *dst, ptrdiff_t stride,
   int width, int height,
@@ -139,7 +96,7 @@ static int decode_plane10(UtvideoContext *c, int plane_no,
 GetBitContext gb;
 int prev, fsym;
 
-if ((ret = build_huff10(huff, , )) < 0) {
+if ((ret = build_huff(huff, , , 1024)) < 0) {
 av_log(c->avctx, AV_LOG_ERROR, "Cannot build Huffman codes\n");
 return ret;
 }
@@ -299,7 +256,7 @@ static int decode_plane(UtvideoContext *c, int plane_no,
 return 0;
 }
 
-if (build_huff(src, , )) {
+if (build_huff(src, , , 256)) {
 av_log(c->avctx, AV_LOG_ERROR, "Cannot build Huffman codes\n");
 return AVERROR_INVALIDDATA;
 }
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 12/25] avcodec/magicyuv: Don't waste stack space

2020-09-26 Thread Andreas Rheinhardt
Now that the HuffEntries are no longer sorted by the MagicYUV decoder,
their symbols are trivial: The symbol of the element with index i is i.
They can therefore be removed. Furthermore, despite the length of the
codes being in the range 1..32 bits, the actual value of the codes is
<= 4096 (for 12 bit content). The reason for this is that the longer
codes are on the left side of the tree, so that the higher bits of
these codes are simply zero. By using an uint16_t for the codes and
removing the symbols entry, the size of each HuffEntry is decreased from
eight to four, saving 16KB of stack space.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 7dded9b457..ea1f727e5c 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -46,9 +46,8 @@ typedef enum Prediction {
 } Prediction;
 
 typedef struct HuffEntry {
-uint16_t sym;
 uint8_t  len;
-uint32_t code;
+uint16_t code;
 } HuffEntry;
 
 typedef struct MagicYUVContext {
@@ -90,10 +89,9 @@ static int huff_build(HuffEntry he[], uint16_t 
codes_count[33],
 he[i].code = codes_count[he[i].len];
 codes_count[he[i].len]++;
 }
-return ff_init_vlc_sparse(vlc, FFMIN(max, 12), nb_elems,
-  [0].len,  sizeof(he[0]), sizeof(he[0].len),
-  [0].code, sizeof(he[0]), sizeof(he[0].code),
-  [0].sym,  sizeof(he[0]), sizeof(he[0].sym),  
0);
+return init_vlc(vlc, FFMIN(max, 12), nb_elems,
+[0].len,  sizeof(he[0]), sizeof(he[0].len),
+[0].code, sizeof(he[0]), sizeof(he[0].code), 0);
 }
 
 static void magicyuv_median_pred16(uint16_t *dst, const uint16_t *src1,
@@ -408,10 +406,8 @@ static int build_huffman(AVCodecContext *avctx, const 
uint8_t *table,
 }
 
 length_count[x] += l;
-for (; j < k; j++) {
-he[j].sym = j;
+for (; j < k; j++)
 he[j].len = x;
-}
 
 if (j == max) {
 j = 0;
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 13/25] avcodec/utvideodec/enc: Fix building Huffman table

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:27:52PM +0200, Andreas Rheinhardt wrote:
> The Ut Video format stores Huffman tables in its bitstream by coding
> the length of a given symbol; it does not code the actual code directly,
> instead this is to be inferred by the rule that a symbol is to the left
> of every shorter symbol in the Huffman tree and that for symbols of the
> same length the symbol is descending from left to right. With one
> exception, this is also what our de- and encoder did.
> 
> The exception only matters when there are codes of length 32, because
> in this case the first symbol of this length did not get the code 0,
> but 1; this is tantamount to pretending that there is a (nonexistent)
> leaf of length 32. This is simply false. The reference software agrees
> with this [1].
> 
> [1]: 
> https://github.com/umezawatakeshi/utvideo/blob/2700a471a78402e5c340150b38e8a793ef3676f1/utv_core/HuffmanCode.cpp#L280
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/utvideodec.c | 4 ++--
>  libavcodec/utvideoenc.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

LGTM

> 
> diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
> index c07636d435..b3c4c3519b 100644
> --- a/libavcodec/utvideodec.c
> +++ b/libavcodec/utvideodec.c
> @@ -70,7 +70,7 @@ static int build_huff10(const uint8_t *src, VLC *vlc, int 
> *fsym)
>  return -1;
>  }
>  
> -code = 1;
> +code = 0;
>  for (i = last; i >= 0; i--) {
>  codes[i] = code >> (32 - he[i].len);
>  bits[i]  = he[i].len;
> @@ -113,7 +113,7 @@ static int build_huff(const uint8_t *src, VLC *vlc, int 
> *fsym)
>  if (he[last].len > 32)
>  return -1;
>  
> -code = 1;
> +code = 0;
>  for (i = last; i >= 0; i--) {
>  codes[i] = code >> (32 - he[i].len);
>  bits[i]  = he[i].len;
> diff --git a/libavcodec/utvideoenc.c b/libavcodec/utvideoenc.c
> index f1b9d11c96..05a9614036 100644
> --- a/libavcodec/utvideoenc.c
> +++ b/libavcodec/utvideoenc.c
> @@ -346,7 +346,7 @@ static void calculate_codes(HuffEntry *he)
>  while (he[last].len == 255 && last)
>  last--;
>  
> -code = 1;
> +code = 0;
>  for (i = last; i >= 0; i--) {
>  he[i].code  = code >> (32 - he[i].len);
>  code   += 0x8000u >> (he[i].len - 1);
> -- 
> 2.25.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 24/25] avcodec/fraps: Use unchecked bitstream reader

2020-09-26 Thread Andreas Rheinhardt
The fraps decoder already checked for overreads manually (and errored
out in this scenario), yet it still enabled implicit checks, leading to
worse performance and more code size.

This commit disables the implicit bitstream reader checks. For the
sample [1] this improves performance from 195105896 to 155851561
decicycles for Clang 10 and from 222801887 to 168270467 decicycles when
compiled with GCC 9.3. These values are the average of 10 runs each
looping ten times over the input.

[1]: 
samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2593/fraps_flv1_decoding_errors.avi

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/fraps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
index 7a7673f73f..00fd63ffec 100644
--- a/libavcodec/fraps.c
+++ b/libavcodec/fraps.c
@@ -31,6 +31,7 @@
  * Version 2 files support by Konstantin Shishkov
  */
 
+#define UNCHECKED_BITSTREAM_READER 1
 #include "avcodec.h"
 #include "get_bits.h"
 #include "huffman.h"
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 23/25] avcodec/utils: Reindentation

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utils.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index a976ceb260..af2a4c2a88 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1037,7 +1037,7 @@ free_and_end:
 if (av_codec_is_encoder(avctx->codec)) {
 #if FF_API_CODED_FRAME
 FF_DISABLE_DEPRECATION_WARNINGS
-av_frame_free(>coded_frame);
+av_frame_free(>coded_frame);
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 av_freep(>extradata);
@@ -1048,18 +1048,18 @@ FF_ENABLE_DEPRECATION_WARNINGS
 av_freep(>priv_data);
 av_freep(>subtitle_header);
 
-av_frame_free(>to_free);
-av_frame_free(>compat_decode_frame);
-av_frame_free(>buffer_frame);
-av_packet_free(>compat_encode_packet);
-av_packet_free(>buffer_pkt);
-av_packet_free(>last_pkt_props);
+av_frame_free(>to_free);
+av_frame_free(>compat_decode_frame);
+av_frame_free(>buffer_frame);
+av_packet_free(>compat_encode_packet);
+av_packet_free(>buffer_pkt);
+av_packet_free(>last_pkt_props);
 
-av_packet_free(>ds.in_pkt);
-av_frame_free(>es.in_frame);
-av_bsf_free(>bsf);
+av_packet_free(>ds.in_pkt);
+av_frame_free(>es.in_frame);
+av_bsf_free(>bsf);
 
-av_buffer_unref(>pool);
+av_buffer_unref(>pool);
 av_freep();
 avctx->internal = NULL;
 avctx->codec = NULL;
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 25/25] avcodec/fraps: Use cached bitstream reader

2020-09-26 Thread Andreas Rheinhardt
This proved beneficial for performance: For the sample [1] the number
of decicycles in one decode call decreased from 155851561 to 108158037
for Clang 10 and from 168270467 to 128847479 for GCC 9.3. For x86-32
compiled with GCC 9.3 and run on an x64 Haswell the number increased
from 158405517 to 202215769, so that the cached bitstream reader is only
enabled if HAVE_FAST_64BIT is set. These values are the average of 10
runs each looping five times over the input.

[1]: 
samples.ffmpeg.org/ffmpeg-bugs/trac/ticket2593/fraps_flv1_decoding_errors.avi

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/fraps.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/fraps.c b/libavcodec/fraps.c
index 00fd63ffec..8d01b44f11 100644
--- a/libavcodec/fraps.c
+++ b/libavcodec/fraps.c
@@ -31,6 +31,9 @@
  * Version 2 files support by Konstantin Shishkov
  */
 
+#include "config.h"
+
+#define CACHED_BITSTREAM_READER HAVE_FAST_64BIT
 #define UNCHECKED_BITSTREAM_READER 1
 #include "avcodec.h"
 #include "get_bits.h"
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 22/25] avcodec/utils: Also free encoder extradata on avcodec_open2() error

2020-09-26 Thread Andreas Rheinhardt
It is owned by libavcodec for encoders.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utils.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 05064b560f..a976ceb260 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1034,11 +1034,15 @@ free_and_end:
 av_opt_free(avctx->priv_data);
 av_opt_free(avctx);
 
+if (av_codec_is_encoder(avctx->codec)) {
 #if FF_API_CODED_FRAME
 FF_DISABLE_DEPRECATION_WARNINGS
 av_frame_free(>coded_frame);
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
+av_freep(>extradata);
+avctx->extradata_size = 0;
+}
 
 av_dict_free();
 av_freep(>priv_data);
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 19/25] avcodec/utils: Remove always-true check

2020-09-26 Thread Andreas Rheinhardt
The first thing avcodec_open2() allocates is the AVCodecInternal. If
allocating it fails, a jump to end occurs; but if an error happens after
its allocation, a jump to free_and_end happens which frees all
allocations performed so far and then jumps to end. Yet free_and_end
contained a check for AVCodecInternal (after having already dereferenced
it to check whether ff_thread_free() needs to be called) which is of
course always true. So remove it.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utils.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 94b9e94584..8e7c3125aa 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1043,7 +1043,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 av_dict_free();
 av_freep(>priv_data);
 av_freep(>subtitle_header);
-if (avci) {
+
 av_frame_free(>to_free);
 av_frame_free(>compat_decode_frame);
 av_frame_free(>buffer_frame);
@@ -1056,7 +1056,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
 av_bsf_free(>bsf);
 
 av_buffer_unref(>pool);
-}
 av_freep();
 avctx->internal = NULL;
 avctx->codec = NULL;
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 18/25] avcodec/utils: Only call codec->close if init has been called

2020-09-26 Thread Andreas Rheinhardt
avcodec_open2() also called the AVCodec's close function if an error
happened before init had ever been called if the AVCodec has the
FF_CODEC_CAP_INIT_CLEANUP flag set. This is against the documentation of
said flag: "The codec allows calling the close function for deallocation
even if the init function returned a failure."

E.g. the SVQ3 decoder is not ready to be closed if init has never been
called.

Fixes: NULL dereference
Fixes: 
25762/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SVQ3_fuzzer-5716279070294016

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg

Signed-off-by: Andreas Rheinhardt 
---
If someone were to call avcodec_alloc_context3(NULL) and manually fill
in avctx->codec, then the close function might even be called before
avctx->priv_data has been allocated.

 libavcodec/utils.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index cf0a55f26d..94b9e94584 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -931,6 +931,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 || avci->frame_thread_encoder)) {
 ret = avctx->codec->init(avctx);
 if (ret < 0) {
+codec_init_ok = -1;
 goto free_and_end;
 }
 codec_init_ok = 1;
@@ -1022,8 +1023,8 @@ end:
 return ret;
 free_and_end:
 if (avctx->codec && avctx->codec->close &&
-(codec_init_ok ||
- (avctx->codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP)))
+(codec_init_ok > 0 || (codec_init_ok < 0 &&
+ avctx->codec->caps_internal & FF_CODEC_CAP_INIT_CLEANUP)))
 avctx->codec->close(avctx);
 
 if (HAVE_THREADS && avci->thread_ctx)
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 17/25] avcodec/utvideo: Move stuff only used by Ut encoder to Ut encoder

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/Makefile |  4 ++--
 libavcodec/utvideo.c| 39 ---
 libavcodec/utvideo.h| 12 
 libavcodec/utvideoenc.c | 23 +--
 4 files changed, 23 insertions(+), 55 deletions(-)
 delete mode 100644 libavcodec/utvideo.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 9b120a6f64..bee2335a5a 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -664,8 +664,8 @@ OBJS-$(CONFIG_TTA_ENCODER) += ttaenc.o 
ttaencdsp.o ttadata.o
 OBJS-$(CONFIG_TWINVQ_DECODER)  += twinvqdec.o twinvq.o
 OBJS-$(CONFIG_TXD_DECODER) += txd.o
 OBJS-$(CONFIG_ULTI_DECODER)+= ulti.o
-OBJS-$(CONFIG_UTVIDEO_DECODER) += utvideodec.o utvideo.o utvideodsp.o
-OBJS-$(CONFIG_UTVIDEO_ENCODER) += utvideoenc.o utvideo.o
+OBJS-$(CONFIG_UTVIDEO_DECODER) += utvideodec.o utvideodsp.o
+OBJS-$(CONFIG_UTVIDEO_ENCODER) += utvideoenc.o
 OBJS-$(CONFIG_V210_DECODER)+= v210dec.o
 OBJS-$(CONFIG_V210_ENCODER)+= v210enc.o
 OBJS-$(CONFIG_V210X_DECODER)   += v210x.o
diff --git a/libavcodec/utvideo.c b/libavcodec/utvideo.c
deleted file mode 100644
index 0cf0cbcd8b..00
--- a/libavcodec/utvideo.c
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Common Ut Video code
- * Copyright (c) 2011 Konstantin Shishkov
- *
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-/**
- * @file
- * Common Ut Video code
- */
-
-#include "utvideo.h"
-
-#if FF_API_PRIVATE_OPT
-const int ff_ut_pred_order[5] = {
-PRED_LEFT, PRED_MEDIAN, PRED_MEDIAN, PRED_NONE, PRED_GRADIENT
-};
-#endif
-
-int ff_ut_huff_cmp_len(const void *a, const void *b)
-{
-const HuffEntry *aa = a, *bb = b;
-return (aa->len - bb->len)*256 + aa->sym - bb->sym;
-}
diff --git a/libavcodec/utvideo.h b/libavcodec/utvideo.h
index 2975f287a7..9da9329ff3 100644
--- a/libavcodec/utvideo.h
+++ b/libavcodec/utvideo.h
@@ -61,9 +61,6 @@ enum {
 UTVIDEO_444  = MKTAG('Y', 'V', '2', '4'),
 };
 
-/* Mapping of libavcodec prediction modes to Ut Video's */
-extern const int ff_ut_pred_order[5];
-
 typedef struct UtvideoContext {
 const AVClass *class;
 AVCodecContext *avctx;
@@ -91,13 +88,4 @@ typedef struct UtvideoContext {
 size_t control_stream_size[4][256];
 } UtvideoContext;
 
-typedef struct HuffEntry {
-uint16_t sym;
-uint8_t  len;
-uint32_t code;
-} HuffEntry;
-
-/* Compare huffman tree nodes */
-int ff_ut_huff_cmp_len(const void *a, const void *b);
-
 #endif /* AVCODEC_UTVIDEO_H */
diff --git a/libavcodec/utvideoenc.c b/libavcodec/utvideoenc.c
index 05a9614036..5c87eb50ac 100644
--- a/libavcodec/utvideoenc.c
+++ b/libavcodec/utvideoenc.c
@@ -37,6 +37,25 @@
 #include "utvideo.h"
 #include "huffman.h"
 
+typedef struct HuffEntry {
+uint16_t sym;
+uint8_t  len;
+uint32_t code;
+} HuffEntry;
+
+#if FF_API_PRIVATE_OPT
+static const int ut_pred_order[5] = {
+PRED_LEFT, PRED_MEDIAN, PRED_MEDIAN, PRED_NONE, PRED_GRADIENT
+};
+#endif
+
+/* Compare huffman tree nodes */
+static int ut_huff_cmp_len(const void *a, const void *b)
+{
+const HuffEntry *aa = a, *bb = b;
+return (aa->len - bb->len)*256 + aa->sym - bb->sym;
+}
+
 /* Compare huffentry symbols */
 static int huff_cmp_sym(const void *a, const void *b)
 {
@@ -139,7 +158,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
 
 /* Convert from libavcodec prediction type to Ut Video's */
 if (avctx->prediction_method)
-c->frame_pred = ff_ut_pred_order[avctx->prediction_method];
+c->frame_pred = ut_pred_order[avctx->prediction_method];
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
@@ -340,7 +359,7 @@ static void calculate_codes(HuffEntry *he)
 int last, i;
 uint32_t code;
 
-qsort(he, 256, sizeof(*he), ff_ut_huff_cmp_len);
+qsort(he, 256, sizeof(*he), ut_huff_cmp_len);
 
 last = 255;
 while (he[last].len == 255 && last)
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 21/25] avcodec/utils: Don't forget cleaning up when allocating priv_data fails

2020-09-26 Thread Andreas Rheinhardt
Allocating an AVCodecContext's priv_data used to be the first object
allocated in avcodec_open2(), so it was unnecessary to goto free_and_end
(which does the cleanup) upon error here. But this is no longer so since
f3a29b750a5979ae6847879fba758faf1fae88d0.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index a8af45d0c3..05064b560f 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -608,7 +608,7 @@ int attribute_align_arg avcodec_open2(AVCodecContext 
*avctx, const AVCodec *code
 avctx->priv_data = av_mallocz(codec->priv_data_size);
 if (!avctx->priv_data) {
 ret = AVERROR(ENOMEM);
-goto end;
+goto free_and_end;
 }
 if (codec->priv_class) {
 *(const AVClass **)avctx->priv_data = codec->priv_class;
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 20/25] avcodec/utils: Improve check for freeing codec private options

2020-09-26 Thread Andreas Rheinhardt
Don't check for AVCodec.priv_data_size (which is always true if
AVCodec.priv_class is set). Instead check for AVCodecContext.priv_data
to actually exist.

(Note: av_opt_free(NULL) is a no-op.)

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 8e7c3125aa..a8af45d0c3 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -1030,7 +1030,7 @@ free_and_end:
 if (HAVE_THREADS && avci->thread_ctx)
 ff_thread_free(avctx);
 
-if (codec->priv_class && codec->priv_data_size)
+if (codec->priv_class && avctx->priv_data)
 av_opt_free(avctx->priv_data);
 av_opt_free(avctx);
 
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 15/25] avcodec/utvideodec: Avoid qsort when creating Huffman tables

2020-09-26 Thread Andreas Rheinhardt
The Ut video format uses Huffman trees which are only implicitly coded
in the bitstream: Only the lengths of the codes are coded, the rest has
to be inferred by the decoder according to the rule that the longer
codes are to the left of shorter codes in the tree and on each level the
symbols are descending from left to right.

Because longer codes are to the left of shorter codes, one needs to know
how many non-leaf nodes there are on each level in order to know the
code of the next left-most leaf (which belongs to the highest symbol on
that level). The current code does this by sorting the entries to be
ascending according to length and (for entries with the same length)
ascending according to their symbols. This array is then traversed in
reverse order, so that the lowest level is dealt with first, so that the
number of non-leaf nodes of the next higher level is known when
processing said level.

But this can also be calculated without sorting: Simply count how many
leaf nodes there are on each level. Then one can calculate the number of
non-leaf nodes on each level iteratively from the lowest level upwards:
It is just half the number of nodes of the level below.

This improves performance: For the sample from ticket #4044 the amount
of decicycles for one call to build_huff() decreased from 1055489 to
446310 for Clang 10 and from 1080306 to 535155 for GCC 9.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utvideo.c|  6 -
 libavcodec/utvideo.h|  1 -
 libavcodec/utvideodec.c | 53 -
 3 files changed, 26 insertions(+), 34 deletions(-)

diff --git a/libavcodec/utvideo.c b/libavcodec/utvideo.c
index 5828d5ec0d..b14e56e0d8 100644
--- a/libavcodec/utvideo.c
+++ b/libavcodec/utvideo.c
@@ -39,9 +39,3 @@ int ff_ut_huff_cmp_len(const void *a, const void *b)
 const HuffEntry *aa = a, *bb = b;
 return (aa->len - bb->len)*256 + aa->sym - bb->sym;
 }
-
-int ff_ut10_huff_cmp_len(const void *a, const void *b)
-{
-const HuffEntry *aa = a, *bb = b;
-return (aa->len - bb->len)*1024 + aa->sym - bb->sym;
-}
diff --git a/libavcodec/utvideo.h b/libavcodec/utvideo.h
index cf0bb28c44..2975f287a7 100644
--- a/libavcodec/utvideo.h
+++ b/libavcodec/utvideo.h
@@ -99,6 +99,5 @@ typedef struct HuffEntry {
 
 /* Compare huffman tree nodes */
 int ff_ut_huff_cmp_len(const void *a, const void *b);
-int ff_ut10_huff_cmp_len(const void *a, const void *b);
 
 #endif /* AVCODEC_UTVIDEO_H */
diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
index f014e90606..8b47c14d98 100644
--- a/libavcodec/utvideodec.c
+++ b/libavcodec/utvideodec.c
@@ -43,45 +43,44 @@
 static int build_huff(const uint8_t *src, VLC *vlc, int *fsym, unsigned 
nb_elems)
 {
 int i;
-HuffEntry he[1024];
-int last;
 uint32_t codes[1024];
 uint8_t bits[1024];
-uint16_t syms[1024];
-uint32_t code;
+uint16_t codes_count[33] = { 0 };
 
 *fsym = -1;
 for (i = 0; i < nb_elems; i++) {
-he[i].sym = i;
-he[i].len = *src++;
-}
-qsort(he, nb_elems, sizeof(*he), ff_ut10_huff_cmp_len);
+if (src[i] == 0) {
+*fsym = i;
+return 0;
+} else if (src[i] == 255) {
+bits[i] = 0;
+} else if (src[i] <= 32) {
+bits[i] = src[i];
+} else
+return AVERROR_INVALIDDATA;
 
-if (!he[0].len) {
-*fsym = he[0].sym;
-return 0;
+codes_count[bits[i]]++;
 }
+if (codes_count[0] == nb_elems)
+return AVERROR_INVALIDDATA;
 
-last = nb_elems - 1;
-while (he[last].len == 255 && last)
-last--;
-
-if (he[last].len > 32) {
-return -1;
+for (unsigned i = 32, nb_codes = 0; i > 0; i--) {
+uint16_t curr = codes_count[i];   // # of leafs of length i
+codes_count[i] = nb_codes / 2;// # of non-leaf nodes on level i
+nb_codes = codes_count[i] + curr; // # of nodes on level i
 }
 
-code = 0;
-for (i = last; i >= 0; i--) {
-codes[i] = code >> (32 - he[i].len);
-bits[i]  = he[i].len;
-syms[i]  = he[i].sym;
-code += 0x8000u >> (he[i].len - 1);
+for (unsigned i = nb_elems; i-- > 0;) {
+if (!bits[i]) {
+codes[i] = 0;
+continue;
+}
+codes[i] = codes_count[bits[i]]++;
 }
 #define VLC_BITS 11
-return ff_init_vlc_sparse(vlc, VLC_BITS, last + 1,
-  bits,  sizeof(*bits),  sizeof(*bits),
-  codes, sizeof(*codes), sizeof(*codes),
-  syms,  sizeof(*syms),  sizeof(*syms), 0);
+return init_vlc(vlc, VLC_BITS, nb_elems,
+bits,  sizeof(*bits),  sizeof(*bits),
+codes, sizeof(*codes), sizeof(*codes), 0);
 }
 
 static int decode_plane10(UtvideoContext *c, int plane_no,
-- 
2.25.1

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

[FFmpeg-devel] [PATCH 09/25] avcodec/magicyuv: Don't invert order unnecessarily

2020-09-26 Thread Andreas Rheinhardt
The MagicYUV decoder currently sets both the length and the symbol field
of an array of HuffEntries; hereby the symbol of the ith entry (0-based)
is just i. Then said array gets sorted so that entries with greater
length are at the end and entries with the same length are ordered so
that those with smaller symbols are at the end. Afterwards the newly
sorted array is traversed in reverse order. This commit instead inverts
the ordering and traverses the array in its ordinary order in order to
simplify understanding.

Signed-off-by: Andreas Rheinhardt 
---
This commit actually only exists to simplify understanding of the next
two commits; apart from that, it is unnecessary.

 libavcodec/magicyuv.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 93ee739093..1b3f4cfc6b 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -77,24 +77,23 @@ typedef struct MagicYUVContext {
 static int huff_cmp_len(const void *a, const void *b)
 {
 const HuffEntry *aa = a, *bb = b;
-return (aa->len - bb->len) * 4096 + bb->sym - aa->sym;
+return (bb->len - aa->len) * 4096 + aa->sym - bb->sym;
 }
 
 static int huff_build(HuffEntry he[], VLC *vlc, int nb_elems)
 {
 uint32_t code;
-int i;
 
 AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
 
 code = 1;
-for (i = nb_elems - 1; i >= 0; i--) {
+for (unsigned i = 0; i < nb_elems; i++) {
 he[i].code = code >> (32 - he[i].len);
 code += 0x8000u >> (he[i].len - 1);
 }
 
 ff_free_vlc(vlc);
-return ff_init_vlc_sparse(vlc, FFMIN(he[nb_elems - 1].len, 12), nb_elems,
+return ff_init_vlc_sparse(vlc, FFMIN(he[0].len, 12), nb_elems,
   [0].len,  sizeof(he[0]), sizeof(he[0].len),
   [0].code, sizeof(he[0]), sizeof(he[0].code),
   [0].sym,  sizeof(he[0]), sizeof(he[0].sym),  
0);
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 08/25] avcodec/magicyuv: Replace implicit checks for overread by explicit ones

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 49 ---
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 3413d8f298..93ee739093 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -442,23 +442,26 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 MagicYUVContext *s = avctx->priv_data;
 ThreadFrame frame = { .f = data };
 AVFrame *p = data;
-GetByteContext gbyte;
+GetByteContext gb;
 uint32_t first_offset, offset, next_offset, header_size, slice_width;
 int width, height, format, version, table_size;
 int ret, i, j;
 
-bytestream2_init(, avpkt->data, avpkt->size);
-if (bytestream2_get_le32() != MKTAG('M', 'A', 'G', 'Y'))
+if (avpkt->size < 36)
+return AVERROR_INVALIDDATA;
+
+bytestream2_init(, avpkt->data, avpkt->size);
+if (bytestream2_get_le32u() != MKTAG('M', 'A', 'G', 'Y'))
 return AVERROR_INVALIDDATA;
 
-header_size = bytestream2_get_le32();
+header_size = bytestream2_get_le32u();
 if (header_size < 32 || header_size >= avpkt->size) {
 av_log(avctx, AV_LOG_ERROR,
"header or packet too small %"PRIu32"\n", header_size);
 return AVERROR_INVALIDDATA;
 }
 
-version = bytestream2_get_byte();
+version = bytestream2_get_byteu();
 if (version != 7) {
 avpriv_request_sample(avctx, "Version %d", version);
 return AVERROR_PATCHWELCOME;
@@ -471,7 +474,7 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 s->decorrelate = 0;
 s->bps = 8;
 
-format = bytestream2_get_byte();
+format = bytestream2_get_byteu();
 switch (format) {
 case 0x65:
 avctx->pix_fmt = AV_PIX_FMT_GBRP;
@@ -552,34 +555,34 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 s->magy_decode_slice = s->bps == 8 ? magy_decode_slice : 
magy_decode_slice10;
 s->planes = av_pix_fmt_count_planes(avctx->pix_fmt);
 
-bytestream2_skip(, 1);
-s->color_matrix = bytestream2_get_byte();
-s->flags= bytestream2_get_byte();
+bytestream2_skipu(, 1);
+s->color_matrix = bytestream2_get_byteu();
+s->flags= bytestream2_get_byteu();
 s->interlaced   = !!(s->flags & 2);
-bytestream2_skip(, 3);
+bytestream2_skipu(, 3);
 
-width  = bytestream2_get_le32();
-height = bytestream2_get_le32();
+width  = bytestream2_get_le32u();
+height = bytestream2_get_le32u();
 ret = ff_set_dimensions(avctx, width, height);
 if (ret < 0)
 return ret;
 
-slice_width = bytestream2_get_le32();
+slice_width = bytestream2_get_le32u();
 if (slice_width != avctx->coded_width) {
 avpriv_request_sample(avctx, "Slice width %"PRIu32, slice_width);
 return AVERROR_PATCHWELCOME;
 }
-s->slice_height = bytestream2_get_le32();
+s->slice_height = bytestream2_get_le32u();
 if (s->slice_height <= 0 || s->slice_height > INT_MAX - 
avctx->coded_height) {
 av_log(avctx, AV_LOG_ERROR,
"invalid slice height: %d\n", s->slice_height);
 return AVERROR_INVALIDDATA;
 }
 
-bytestream2_skip(, 4);
+bytestream2_skipu(, 4);
 
 s->nb_slices = (avctx->coded_height + s->slice_height - 1) / 
s->slice_height;
-if (s->nb_slices > INT_MAX / sizeof(Slice)) {
+if (s->nb_slices > INT_MAX / FFMAX(sizeof(Slice), 4 * 5)) {
 av_log(avctx, AV_LOG_ERROR,
"invalid number of slices: %d\n", s->nb_slices);
 return AVERROR_INVALIDDATA;
@@ -596,12 +599,14 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 }
 }
 
+if (bytestream2_get_bytes_left() <= s->nb_slices * s->planes * 5)
+return AVERROR_INVALIDDATA;
 for (i = 0; i < s->planes; i++) {
 av_fast_malloc(>slices[i], >slices_size[i], s->nb_slices * 
sizeof(Slice));
 if (!s->slices[i])
 return AVERROR(ENOMEM);
 
-offset = bytestream2_get_le32();
+offset = bytestream2_get_le32u();
 if (offset >= avpkt->size - header_size)
 return AVERROR_INVALIDDATA;
 
@@ -611,7 +616,7 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 for (j = 0; j < s->nb_slices - 1; j++) {
 s->slices[i][j].start = offset + header_size;
 
-next_offset = bytestream2_get_le32();
+next_offset = bytestream2_get_le32u();
 if (next_offset <= offset || next_offset >= avpkt->size - 
header_size)
 return AVERROR_INVALIDDATA;
 
@@ -625,16 +630,16 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 s->slices[i][j].size  = avpkt->size - s->slices[i][j].start;
 }
 
-if (bytestream2_get_byte() != s->planes)
+if (bytestream2_get_byteu() != s->planes)
 return AVERROR_INVALIDDATA;
 
-bytestream2_skip(, s->nb_slices * 

[FFmpeg-devel] [PATCH 16/25] avcodec/utvideo: Remove unused array

2020-09-26 Thread Andreas Rheinhardt
Unused since 3594788b713e76449eda0bc9d64b38258c86a594.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utvideo.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavcodec/utvideo.c b/libavcodec/utvideo.c
index b14e56e0d8..0cf0cbcd8b 100644
--- a/libavcodec/utvideo.c
+++ b/libavcodec/utvideo.c
@@ -32,8 +32,6 @@ const int ff_ut_pred_order[5] = {
 };
 #endif
 
-const int ff_ut_rgb_order[4]  = { 1, 2, 0, 3 }; // G, B, R, A
-
 int ff_ut_huff_cmp_len(const void *a, const void *b)
 {
 const HuffEntry *aa = a, *bb = b;
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 07/25] avcodec/magicyuv: Use const uint8_t* for pointer to immutable data

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index f7dfef0eb8..3413d8f298 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -63,7 +63,7 @@ typedef struct MagicYUVContext {
 int   color_matrix;   // video color matrix
 int   flags;
 int   interlaced; // video is interlaced
-uint8_t  *buf;// pointer to AVPacket->data
+const uint8_t*buf;// pointer to AVPacket->data
 int   hshift[4];
 int   vshift[4];
 Slice*slices[4];  // slice bitstream positions for each 
plane
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 11/25] avcodec/magicyuv: Avoid AV_QSORT when creating Huffman table

2020-09-26 Thread Andreas Rheinhardt
The MagicYUV format stores Huffman tables in its bitstream by coding
the length of a given symbol; it does not code the actual code directly,
instead this is to be inferred by the rule that a symbol is to the left
of every shorter symbol in the Huffman tree and that for symbols of the
same length the symbol is ascending from left to right.

Our decoder implemented this by first sorting the array containing
length and symbol of each element according to descending length and
for equal length, according to ascending symbol. Afterwards, the current
state in the tree got encoded in a variable code; if the next array entry
had length len, then the len most significant bits of code contained
the code of this entry. Whenever an entry of the array of length
len was processed, code was incremented by 1U << (32 - len). So two
entries of length len have the same effect as incrementing code by
1U << (32 - (len - 1)), which corresponds to the parent node of length
len - 1 of the two nodes of length len etc.

This commit modifies this to avoid sorting the entries before
calculating the codes. This is done by calculating how many non-leaf
nodes there are on each level of the tree before calculating the codes.
Afterwards every leaf node on this level gets assigned the number of
nodes already on this level as code. This of course works only because
the entries are already sorted by their symbol initially, so that this
algorithm indeed gives ascending symbols from left to right on every
level.

This offers both speed- as well as (obvious) codesize advantages. With
Clang 10 the number of decicycles for build_huffman decreased from
1561987 to 1228405; for GCC 9 it went from 1825096 decicyles to 1429921.
These tests were carried out with a sample with 150 frames that was
looped 13 times; and this was iterated 10 times. The earlier reference
point here is from the point when the loop generating the codes was
traversed in reverse order (as the patch reversing the order led to
performance penalties).

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index 17dea69d76..7dded9b457 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -25,7 +25,6 @@
 #define CACHED_BITSTREAM_READER !ARCH_X86_32
 
 #include "libavutil/pixdesc.h"
-#include "libavutil/qsort.h"
 
 #include "avcodec.h"
 #include "bytestream.h"
@@ -74,26 +73,24 @@ typedef struct MagicYUVContext {
 LLVidDSPContext   llviddsp;
 } MagicYUVContext;
 
-static int huff_cmp_len(const void *a, const void *b)
+static int huff_build(HuffEntry he[], uint16_t codes_count[33],
+  VLC *vlc, int nb_elems)
 {
-const HuffEntry *aa = a, *bb = b;
-return (bb->len - aa->len) * 4096 + aa->sym - bb->sym;
-}
-
-static int huff_build(HuffEntry he[], VLC *vlc, int nb_elems)
-{
-uint32_t code;
-
-AV_QSORT(he, nb_elems, HuffEntry, huff_cmp_len);
+unsigned nb_codes = 0, max = 0;
+
+for (int i = 32; i > 0; i--) {
+uint16_t curr = codes_count[i];   // # of leafs of length i
+codes_count[i] = nb_codes / 2;// # of non-leaf nodes on level i
+nb_codes = codes_count[i] + curr; // # of nodes on level i
+if (curr && !max)
+max = i;
+}
 
-code = 0;
 for (unsigned i = 0; i < nb_elems; i++) {
-he[i].code = code >> (32 - he[i].len);
-code += 0x8000u >> (he[i].len - 1);
+he[i].code = codes_count[he[i].len];
+codes_count[he[i].len]++;
 }
-
-ff_free_vlc(vlc);
-return ff_init_vlc_sparse(vlc, FFMIN(he[0].len, 12), nb_elems,
+return ff_init_vlc_sparse(vlc, FFMIN(max, 12), nb_elems,
   [0].len,  sizeof(he[0]), sizeof(he[0].len),
   [0].code, sizeof(he[0]), sizeof(he[0].code),
   [0].sym,  sizeof(he[0]), sizeof(he[0].sym),  
0);
@@ -389,6 +386,7 @@ static int build_huffman(AVCodecContext *avctx, const 
uint8_t *table,
 MagicYUVContext *s = avctx->priv_data;
 GetByteContext gb;
 HuffEntry he[4096];
+uint16_t length_count[33] = { 0 };
 int i = 0, j = 0, k;
 
 bytestream2_init(, table, table_size);
@@ -409,6 +407,7 @@ static int build_huffman(AVCodecContext *avctx, const 
uint8_t *table,
 return AVERROR_INVALIDDATA;
 }
 
+length_count[x] += l;
 for (; j < k; j++) {
 he[j].sym = j;
 he[j].len = x;
@@ -416,7 +415,7 @@ static int build_huffman(AVCodecContext *avctx, const 
uint8_t *table,
 
 if (j == max) {
 j = 0;
-if (huff_build(he, >vlc[i], max)) {
+if (huff_build(he, length_count, >vlc[i], max)) {
 av_log(avctx, AV_LOG_ERROR, "Cannot build Huffman codes\n");
 return AVERROR_INVALIDDATA;
 }
@@ -424,6 +423,7 @@ static int 

[FFmpeg-devel] [PATCH 13/25] avcodec/utvideodec/enc: Fix building Huffman table

2020-09-26 Thread Andreas Rheinhardt
The Ut Video format stores Huffman tables in its bitstream by coding
the length of a given symbol; it does not code the actual code directly,
instead this is to be inferred by the rule that a symbol is to the left
of every shorter symbol in the Huffman tree and that for symbols of the
same length the symbol is descending from left to right. With one
exception, this is also what our de- and encoder did.

The exception only matters when there are codes of length 32, because
in this case the first symbol of this length did not get the code 0,
but 1; this is tantamount to pretending that there is a (nonexistent)
leaf of length 32. This is simply false. The reference software agrees
with this [1].

[1]: 
https://github.com/umezawatakeshi/utvideo/blob/2700a471a78402e5c340150b38e8a793ef3676f1/utv_core/HuffmanCode.cpp#L280

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/utvideodec.c | 4 ++--
 libavcodec/utvideoenc.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
index c07636d435..b3c4c3519b 100644
--- a/libavcodec/utvideodec.c
+++ b/libavcodec/utvideodec.c
@@ -70,7 +70,7 @@ static int build_huff10(const uint8_t *src, VLC *vlc, int 
*fsym)
 return -1;
 }
 
-code = 1;
+code = 0;
 for (i = last; i >= 0; i--) {
 codes[i] = code >> (32 - he[i].len);
 bits[i]  = he[i].len;
@@ -113,7 +113,7 @@ static int build_huff(const uint8_t *src, VLC *vlc, int 
*fsym)
 if (he[last].len > 32)
 return -1;
 
-code = 1;
+code = 0;
 for (i = last; i >= 0; i--) {
 codes[i] = code >> (32 - he[i].len);
 bits[i]  = he[i].len;
diff --git a/libavcodec/utvideoenc.c b/libavcodec/utvideoenc.c
index f1b9d11c96..05a9614036 100644
--- a/libavcodec/utvideoenc.c
+++ b/libavcodec/utvideoenc.c
@@ -346,7 +346,7 @@ static void calculate_codes(HuffEntry *he)
 while (he[last].len == 255 && last)
 last--;
 
-code = 1;
+code = 0;
 for (i = last; i >= 0; i--) {
 he[i].code  = code >> (32 - he[i].len);
 code   += 0x8000u >> (he[i].len - 1);
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 05/25] avcodec/magicyuv: Check early for invalid slices

2020-09-26 Thread Andreas Rheinhardt
Every plane of each slice has to contain at least two bytes for flags
and the type of prediction used.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index d2f6a9b01e..6c29efc9f4 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -614,6 +614,8 @@ static int magy_decode_frame(AVCodecContext *avctx, void 
*data,
 return AVERROR_INVALIDDATA;
 
 s->slices[i][j].size = next_offset - offset;
+if (s->slices[i][j].size < 2)
+return AVERROR_INVALIDDATA;
 offset = next_offset;
 }
 
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 03/25] avcodec/magicyuv: Improve overread check when parsing Huffman tables

2020-09-26 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/magicyuv.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavcodec/magicyuv.c b/libavcodec/magicyuv.c
index b56d3e9d32..d2f6a9b01e 100644
--- a/libavcodec/magicyuv.c
+++ b/libavcodec/magicyuv.c
@@ -394,8 +394,13 @@ static int build_huffman(AVCodecContext *avctx, 
GetBitContext *gbit, int max)
 while (get_bits_left(gbit) >= 8) {
 int b = get_bits(gbit, 1);
 int x = get_bits(gbit, 7);
-int l = get_bitsz(gbit, b * 8) + 1;
+int l = 1;
 
+if (b) {
+if (get_bits_left(gbit) < 8)
+break;
+l += get_bits(gbit, 8);
+}
 k = j + l;
 if (k > max || x == 0 || x > 32) {
 av_log(avctx, AV_LOG_ERROR, "Invalid Huffman codes\n");
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 04/25] avcodec/diracdsp: Remove unused variable

2020-09-26 Thread Andreas Rheinhardt
Forgotten in ca3c6c981aa5b0af8a5576020b79fdd3cdf9ae9e.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/diracdsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/diracdsp.c b/libavcodec/diracdsp.c
index 4e08d3817e..80dfafd78b 100644
--- a/libavcodec/diracdsp.c
+++ b/libavcodec/diracdsp.c
@@ -195,7 +195,7 @@ static void dequant_subband_ ## PX ## _c(uint8_t *src, 
uint8_t *dst, ptrdiff_t s
 {  
\
 int i, y;  
\
 for (y = 0; y < tot_v; y++) {  
\
-PX c, sign, *src_r = (PX *)src, *dst_r = (PX *)dst;
\
+PX c, *src_r = (PX *)src, *dst_r = (PX *)dst;  
\
 for (i = 0; i < tot_h; i++) {  
\
 c = *src_r++;  
\
 if (c < 0) c = -((-(unsigned)c*qf + qs) >> 2); 
\
-- 
2.25.1

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/aviobuf: fix checks in ffio_ensure_seekback

2020-09-26 Thread Michael Niedermayer
On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:
> Signed-off-by: Marton Balint 
> ---
>  libavformat/aviobuf.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

The commit message is too terse. It is not clear from it what the problem
is that is being fixed and how it is fixed.
Also this feels like it fixes multiple issues so it probably should be spit
unless iam missing some interdependancies

ill also take a look at the "competing" patch, so i dont yet know
how they relate ...


> 
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 9675425349..aa1d6c0830 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -999,9 +999,12 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t 
> buf_size)
>  int filled = s->buf_end - s->buffer;
>  ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr - 
> s->buffer : -1;
>  
> -buf_size += s->buf_ptr - s->buffer + max_buffer_size;
> +if (buf_size <= s->buf_end - s->buf_ptr)
> +return 0;
> +
> +buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;
>  

> -if (buf_size < filled || s->seekable || !s->read_packet)
> +if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
>  return 0;

Did you check that this doesnt interfere with the s->buffer_size reduction
we do when it was larger from probing ?
Its maybe ok, just checking that this was considered

thx


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

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


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/aviobuf: fix checks in ffio_ensure_seekback

2020-09-26 Thread Marton Balint



On Sun, 20 Sep 2020, Marton Balint wrote:




On Sun, 20 Sep 2020, Paul B Mahol wrote:


On Sun, Sep 20, 2020 at 03:16:15PM +0200, Marton Balint wrote:



On Sun, 20 Sep 2020, Paul B Mahol wrote:

> On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:
> > Signed-off-by: Marton Balint 
> > ---
> >  libavformat/aviobuf.c | 7 +--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c

> > index 9675425349..aa1d6c0830 100644
> > --- a/libavformat/aviobuf.c
> > +++ b/libavformat/aviobuf.c
> > @@ -999,9 +999,12 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t 

buf_size)

> >  int filled = s->buf_end - s->buffer;
> >  ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr 

- s->buffer : -1;
> > 
> > -buf_size += s->buf_ptr - s->buffer + max_buffer_size;

> > +if (buf_size <= s->buf_end - s->buf_ptr)
> > +return 0;
> > +
> > +buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;
> > 
> > -if (buf_size < filled || s->seekable || !s->read_packet)

> > +if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
> >  return 0;
> >  av_assert0(!s->write_flag);
> 
> 
> Not acceptable change.
> 
> Your code does this to chained ogg files:
> 
> XXX 10

> XXX 65307
> XXX 65307
> 102031
> 106287
> 110527
> 114745
> 119319
> [...]

This was also the case before the patch, no? So this alone is no reason
to reject the patch.


Exactly the reson for patch rejection is that it does not improve code at 

all.

It might not fix your issue, but it certainly improves the code.


Ping for this as well.

Thanks,
Marton








> 
> It continues allocating excessive small extra chunks of bytes for no 

apparent reason in each and every call
> which slowly and gradually increases memory usage, but every call causes 

unnecessary memcpy calls thus causing

> almost exponential slowdown of file processing.

And when I say ffio_ensure_seekback() has a design issue, this is exactly
what I mean. It is not that suprising, consider this:

I have 32k buffer, and I read at most 32k at once.
I want seekback of 64k. Buffer got increased to 96k
I read 64k.
I want seekback of 64k. Buffer got increased to 160k.
I read 64k.
... and so on.


My patch exactly does that and it proves it works.



a read call cannot flush the buffer because I am always whitin my 

requested
boundary. ffio_ensure_seekback() cannot flush the buffer either, because 

it

is not allowed to do that. Therefore I consume infinite memory.


This explanation is flawed.


Please point out where the flaw is.

Thanks,
Marton





> 
> Lines with XXX, means that allocation and memcpy was not needed.


Are you sure about that? Feel free to give an example with buffer sizes 

and

buffer positions, and prove that reallocation is uneeded. But please be
aware how fill_buffer() works and make sure that when reading sequentially
up to buf_size, seeking within the current pos and pos+buf_size is 

possible.


Seeking should be possible anywhere between buffer start and buffer end.
current buffer ptr is not important as it just points within buffer.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/aviobuf: read till the very end of the IO buffer

2020-09-26 Thread Marton Balint



On Sun, 20 Sep 2020, Marton Balint wrote:




On Sun, 20 Sep 2020, Paul B Mahol wrote:


On Sun, Sep 20, 2020 at 10:52:51AM +0200, Marton Balint wrote:
There was an off-by-one error when checking if the IO buffer still has 

enough

space till the end.


How to reproduce such error(s)?


It is not something you will notice, only the buffer is flushed when it
does not necessarily need to be.

But I guess you can reproduce it by using an buffer size of 66852 and try
receiving an MPEGTS UDP stream. Without the patch fill_buffer always
flushes, with the patch fill_buffer only flushes in every second
call, as it needs to, because a single UDP TS packet (1316 bytes) and
the potential maximum sized UDP packet (65536 bytes) simultaneously fit in
the buffer.


Ping.

Thanks,
Marton



Regards,
Marton






Signed-off-by: Marton Balint 
---
 libavformat/aviobuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index a77517d712..9675425349 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -540,7 +540,7 @@ static void fill_buffer(AVIOContext *s)
 {
 int max_buffer_size = s->max_packet_size ?
   s->max_packet_size : IO_BUFFER_SIZE;
-uint8_t *dst= s->buf_end - s->buffer + max_buffer_size < 

s->buffer_size ?
+uint8_t *dst= s->buf_end - s->buffer + max_buffer_size <= 

s->buffer_size ?

   s->buf_end : s->buffer;
 int len = s->buffer_size - (dst - s->buffer);

--
2.26.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] lavf/hls: add AC-3/EAC-3 to allowed extensions file list

2020-09-26 Thread Steven Liu
Jun Zhao  于2020年9月25日周五 下午8:25写道:
>
> From: Jun Zhao 
>
> Add AC-3/EAC-3 to allowed extensions file list.
>
> From HTTP Live Streaming 2nd Edition draft-pantos-hls-rfc8216bis-07
> section 3.1.3.Packed Audio, HLS demuxer need to support MP3/AC-3/EAC-3.
>
> Signed-off-by: Jun Zhao 
> ---
>  libavformat/hls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index f33ff3f..72e28ab 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2371,7 +2371,7 @@ static const AVOption hls_options[] = {
>  OFFSET(live_start_index), AV_OPT_TYPE_INT, {.i64 = -3}, INT_MIN, 
> INT_MAX, FLAGS},
>  {"allowed_extensions", "List of file extensions that hls is allowed to 
> access",
>  OFFSET(allowed_extensions), AV_OPT_TYPE_STRING,
> -{.str = 
> "3gp,aac,avi,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,wav"},
> +{.str = 
> "3gp,aac,avi,ac3,eac3,flac,mkv,m3u8,m4a,m4s,m4v,mpg,mov,mp2,mp3,mp4,mpeg,mpegts,ogg,ogv,oga,ts,vob,wav"},
>  INT_MIN, INT_MAX, FLAGS},
>  {"max_reload", "Maximum number of times a insufficient list is attempted 
> to be reloaded",
>  OFFSET(max_reload), AV_OPT_TYPE_INT, {.i64 = 1000}, 0, INT_MAX, 
> FLAGS},
> --
> 2.7.4
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

LGTM


Thanks

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] examples/muxing: misc style fixes

2020-09-26 Thread Steven Liu
Jun Zhao  于2020年9月25日周五 下午8:24写道:
>
> From: Jun Zhao 
>
> misc style fixes.
>
> Signed-off-by: Jun Zhao 
> ---
>  doc/examples/muxing.c | 47 +++
>  1 file changed, 23 insertions(+), 24 deletions(-)
>
> diff --git a/doc/examples/muxing.c b/doc/examples/muxing.c
> index bd16486..42f704c 100644
> --- a/doc/examples/muxing.c
> +++ b/doc/examples/muxing.c
> @@ -200,7 +200,7 @@ static void add_stream(OutputStream *ost, AVFormatContext 
> *oc,
>   * the motion of the chroma plane does not match the luma plane. 
> */
>  c->mb_decision = 2;
>  }
> -break;
> +break;
>
>  default:
>  break;
> @@ -284,25 +284,25 @@ static void open_audio(AVFormatContext *oc, AVCodec 
> *codec, OutputStream *ost, A
>  }
>
>  /* create resampler context */
> -ost->swr_ctx = swr_alloc();
> -if (!ost->swr_ctx) {
> -fprintf(stderr, "Could not allocate resampler context\n");
> -exit(1);
> -}
> +ost->swr_ctx = swr_alloc();
> +if (!ost->swr_ctx) {
> +fprintf(stderr, "Could not allocate resampler context\n");
> +exit(1);
> +}
>
> -/* set options */
> -av_opt_set_int   (ost->swr_ctx, "in_channel_count",   
> c->channels,   0);
> -av_opt_set_int   (ost->swr_ctx, "in_sample_rate", 
> c->sample_rate,0);
> -av_opt_set_sample_fmt(ost->swr_ctx, "in_sample_fmt",  
> AV_SAMPLE_FMT_S16, 0);
> -av_opt_set_int   (ost->swr_ctx, "out_channel_count",  
> c->channels,   0);
> -av_opt_set_int   (ost->swr_ctx, "out_sample_rate",
> c->sample_rate,0);
> -av_opt_set_sample_fmt(ost->swr_ctx, "out_sample_fmt", 
> c->sample_fmt, 0);
> -
> -/* initialize the resampling context */
> -if ((ret = swr_init(ost->swr_ctx)) < 0) {
> -fprintf(stderr, "Failed to initialize the resampling context\n");
> -exit(1);
> -}
> +/* set options */
> +av_opt_set_int   (ost->swr_ctx, "in_channel_count",   c->channels,   
> 0);
> +av_opt_set_int   (ost->swr_ctx, "in_sample_rate", 
> c->sample_rate,0);
> +av_opt_set_sample_fmt(ost->swr_ctx, "in_sample_fmt",  
> AV_SAMPLE_FMT_S16, 0);
> +av_opt_set_int   (ost->swr_ctx, "out_channel_count",  c->channels,   
> 0);
> +av_opt_set_int   (ost->swr_ctx, "out_sample_rate",
> c->sample_rate,0);
> +av_opt_set_sample_fmt(ost->swr_ctx, "out_sample_fmt", c->sample_fmt, 
> 0);
> +
> +/* initialize the resampling context */
> +if ((ret = swr_init(ost->swr_ctx)) < 0) {
> +fprintf(stderr, "Failed to initialize the resampling context\n");
> +exit(1);
> +}
>  }
>
>  /* Prepare a 16 bit dummy audio frame of 'frame_size' samples and
> @@ -349,10 +349,10 @@ static int write_audio_frame(AVFormatContext *oc, 
> OutputStream *ost)
>
>  if (frame) {
>  /* convert samples from native format to destination codec format, 
> using the resampler */
> -/* compute destination number of samples */
> -dst_nb_samples = av_rescale_rnd(swr_get_delay(ost->swr_ctx, 
> c->sample_rate) + frame->nb_samples,
> -c->sample_rate, c->sample_rate, 
> AV_ROUND_UP);
> -av_assert0(dst_nb_samples == frame->nb_samples);
> +/* compute destination number of samples */
> +dst_nb_samples = av_rescale_rnd(swr_get_delay(ost->swr_ctx, 
> c->sample_rate) + frame->nb_samples,
> +c->sample_rate, c->sample_rate, 
> AV_ROUND_UP);
> +av_assert0(dst_nb_samples == frame->nb_samples);
>
>  /* when we pass a frame to the encoder, it may keep a reference to it
>   * internally;
> @@ -519,7 +519,6 @@ static AVFrame *get_video_frame(OutputStream *ost)
>  static int write_video_frame(AVFormatContext *oc, OutputStream *ost)
>  {
>  return write_frame(oc, ost->enc, ost->st, get_video_frame(ost));
> -
>  }
>
>  static void close_stream(AVFormatContext *oc, OutputStream *ost)
> --
> 2.7.4
>
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
lgtm
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/6] avcodec/dxtory: Fix negative shift in dx2_decode_slice_410()

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:26:35AM +0200, Michael Niedermayer wrote:
> Fixes: left shift of negative value -768
> Fixes: 
> 25574/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXTORY_fuzzer-6012596027916288
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/dxtory.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

lgtm

> 
> diff --git a/libavcodec/dxtory.c b/libavcodec/dxtory.c
> index a82532c467..bc95e0e7e1 100644
> --- a/libavcodec/dxtory.c
> +++ b/libavcodec/dxtory.c
> @@ -637,7 +637,7 @@ static int dx2_decode_slice_410(GetBitContext *gb, 
> AVFrame *frame,
>  V[huvborder] = decode_sym(gb, lru[2]) ^ 0x80;
>  }
>  
> -Y += ystride << 2;
> +Y += ystride * 4;
>  U += ustride;
>  V += vstride;
>  }
> -- 
> 2.17.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] avcodec/dxtory: Fix get_raw_size() for YUV

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:26:33AM +0200, Michael Niedermayer wrote:
> Fixes: out of array read
> Fixes: 
> 25455/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXTORY_fuzzer-6327985731534848
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/dxtory.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

lgtm

> 
> diff --git a/libavcodec/dxtory.c b/libavcodec/dxtory.c
> index 3f3c23ff2a..157e4b3ed2 100644
> --- a/libavcodec/dxtory.c
> +++ b/libavcodec/dxtory.c
> @@ -44,9 +44,9 @@ static int64_t get_raw_size(enum AVPixelFormat fmt, int 
> width, int height)
>  case AV_PIX_FMT_YUV444P:
>  return width * height * 3LL;
>  case AV_PIX_FMT_YUV420P:
> -return (int64_t)(width * height) + AV_CEIL_RSHIFT(width, 1) * 
> AV_CEIL_RSHIFT(height, 1);
> +return (int64_t)(width * height) + 2 * AV_CEIL_RSHIFT(width, 1) * 
> AV_CEIL_RSHIFT(height, 1);
>  case AV_PIX_FMT_YUV410P:
> -return (int64_t)(width * height) + AV_CEIL_RSHIFT(width, 2) * 
> AV_CEIL_RSHIFT(height, 2);
> +return (int64_t)(width * height) + 2 * AV_CEIL_RSHIFT(width, 2) * 
> AV_CEIL_RSHIFT(height, 2);
>  }
>  
>  return 0;
> -- 
> 2.17.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/6] avformat/argo_{asf, brp}: fix potential segfault in ff_argo_asf_fill_stream()

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 02:38:45AM +, Zane van Iperen wrote:
> Signed-off-by: Zane van Iperen 
> ---
>  libavformat/argo_asf.c | 10 +-
>  libavformat/argo_asf.h |  2 +-
>  libavformat/argo_brp.c |  2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/6] avcodec/dxtory: Fix negative shift in dxtory_decode_v1_410()

2020-09-26 Thread Paul B Mahol
On Sat, Sep 26, 2020 at 12:26:34AM +0200, Michael Niedermayer wrote:
> Fixes: left shift of negative value -256
> Fixes: 
> 25460/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DXTORY_fuzzer-5073252341514240
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/dxtory.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

lgtm

> 
> diff --git a/libavcodec/dxtory.c b/libavcodec/dxtory.c
> index 157e4b3ed2..a82532c467 100644
> --- a/libavcodec/dxtory.c
> +++ b/libavcodec/dxtory.c
> @@ -177,10 +177,10 @@ static int dxtory_decode_v1_410(AVCodecContext *avctx, 
> AVFrame *pic,
>  V[huvborder] = src[1] + 0x80;
>  src += 2;
>  }
> -Y1 += pic->linesize[0] << 2;
> -Y2 += pic->linesize[0] << 2;
> -Y3 += pic->linesize[0] << 2;
> -Y4 += pic->linesize[0] << 2;
> +Y1 += pic->linesize[0] * 4;
> +Y2 += pic->linesize[0] * 4;
> +Y3 += pic->linesize[0] * 4;
> +Y4 += pic->linesize[0] * 4;
>  U  += pic->linesize[1];
>  V  += pic->linesize[2];
>  }
> -- 
> 2.17.1
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".