Re: [FFmpeg-devel] [PATCH 2/2] libswcale/input: fix incorrect rgbf32 yuv conversions
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
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
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
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
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()
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()
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
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
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
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
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
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
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
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()
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
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
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
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)
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
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()
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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()
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()
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".