| Message ID | 20220302113805.272888-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/mpegvideo_enc: remove direct=1 support | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_aarch64_jetson | success | Make finished |
| andriy/make_fate_aarch64_jetson | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On Wed, Mar 02, 2022 at 12:38:05PM +0100, Paul B Mahol wrote: > It seems it does not work properly. > > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > libavcodec/mpegvideo_enc.c | 23 +++-------------------- > 1 file changed, 3 insertions(+), 20 deletions(-) if iam not mistaken, that requires the whole image to be copied one extra time i think before doing that it should be understood where the problem is and why that is the better solution to fixing it and not doing that extra copy of course i may be missing something thx [...]
On 3/2/22, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 02, 2022 at 12:38:05PM +0100, Paul B Mahol wrote: >> It seems it does not work properly. >> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> libavcodec/mpegvideo_enc.c | 23 +++-------------------- >> 1 file changed, 3 insertions(+), 20 deletions(-) > > if iam not mistaken, that requires the whole image to be copied > one extra time > i think before doing that it should be understood > where the problem is > and why that is the better solution to fixing it and not doing > that extra copy > > of course i may be missing something > Yea, If you manage to trigger that path, I'm all ears. > thx > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Opposition brings concord. Out of discord comes the fairest harmony. > -- Heraclitus >
On Wed, Mar 02, 2022 at 08:11:18PM +0100, Paul B Mahol wrote: > On 3/2/22, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Wed, Mar 02, 2022 at 12:38:05PM +0100, Paul B Mahol wrote: > >> It seems it does not work properly. > >> > >> Signed-off-by: Paul B Mahol <onemda@gmail.com> > >> --- > >> libavcodec/mpegvideo_enc.c | 23 +++-------------------- > >> 1 file changed, 3 insertions(+), 20 deletions(-) > > > > if iam not mistaken, that requires the whole image to be copied > > one extra time > > i think before doing that it should be understood > > where the problem is > > and why that is the better solution to fixing it and not doing > > that extra copy > > > > of course i may be missing something > > > > Yea, If you manage to trigger that path, I'm all ears. thats a fair request i tried --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -1065,6 +1065,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg) pic->reference = 3; if (direct) { + av_log(0,0, "direct mode\n"); if ((ret = av_frame_ref(pic->f, pic_arg)) < 0) return ret; } with ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -t 1 test.avi and that gives me: direct mode Last message repeated 23 times so it seems the code is used [...]
Michael Niedermayer: > On Wed, Mar 02, 2022 at 08:11:18PM +0100, Paul B Mahol wrote: >> On 3/2/22, Michael Niedermayer <michael@niedermayer.cc> wrote: >>> On Wed, Mar 02, 2022 at 12:38:05PM +0100, Paul B Mahol wrote: >>>> It seems it does not work properly. >>>> >>>> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>>> --- >>>> libavcodec/mpegvideo_enc.c | 23 +++-------------------- >>>> 1 file changed, 3 insertions(+), 20 deletions(-) >>> >>> if iam not mistaken, that requires the whole image to be copied >>> one extra time >>> i think before doing that it should be understood >>> where the problem is >>> and why that is the better solution to fixing it and not doing >>> that extra copy >>> >>> of course i may be missing something >>> >> >> Yea, If you manage to trigger that path, I'm all ears. > > thats a fair request > > i tried > --- a/libavcodec/mpegvideo_enc.c > +++ b/libavcodec/mpegvideo_enc.c > @@ -1065,6 +1065,7 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg) > pic->reference = 3; > > if (direct) { > + av_log(0,0, "direct mode\n"); > if ((ret = av_frame_ref(pic->f, pic_arg)) < 0) > return ret; > } > > with > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -t 1 test.avi > > and that gives me: > direct mode > Last message repeated 23 times > > so it seems the code is used > > [...] > Furthermore, http://coverage.ffmpeg.org/index.mpegvideo_enc.c.68609919c7303277fae79be622d1b44d.html shows that the direct branch is taken 649 times (out of 8563 instances). - Andreas
On 3/2/22, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Mar 02, 2022 at 08:11:18PM +0100, Paul B Mahol wrote: >> On 3/2/22, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > On Wed, Mar 02, 2022 at 12:38:05PM +0100, Paul B Mahol wrote: >> >> It seems it does not work properly. >> >> >> >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> >> --- >> >> libavcodec/mpegvideo_enc.c | 23 +++-------------------- >> >> 1 file changed, 3 insertions(+), 20 deletions(-) >> > >> > if iam not mistaken, that requires the whole image to be copied >> > one extra time >> > i think before doing that it should be understood >> > where the problem is >> > and why that is the better solution to fixing it and not doing >> > that extra copy >> > >> > of course i may be missing something >> > >> >> Yea, If you manage to trigger that path, I'm all ears. > > thats a fair request > > i tried > --- a/libavcodec/mpegvideo_enc.c > +++ b/libavcodec/mpegvideo_enc.c > @@ -1065,6 +1065,7 @@ static int load_input_picture(MpegEncContext *s, const > AVFrame *pic_arg) > pic->reference = 3; > > if (direct) { > + av_log(0,0, "direct mode\n"); > if ((ret = av_frame_ref(pic->f, pic_arg)) < 0) > return ret; > } > > with > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -t 1 test.avi > > and that gives me: > direct mode > Last message repeated 23 times > > so it seems the code is used Same happens if filtering is used? > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Observe your enemies, for they first find out your faults. -- Antisthenes >
diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c index c69114ea15..fb36c8e2d8 100644 --- a/libavcodec/mpegvideo_enc.c +++ b/libavcodec/mpegvideo_enc.c @@ -1009,7 +1009,6 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg) int encoding_delay = s->max_b_frames ? s->max_b_frames : (s->low_delay ? 0 : 1); int flush_offset = 1; - int direct = 1; if (pic_arg) { pts = pic_arg->pts; @@ -1042,37 +1041,21 @@ static int load_input_picture(MpegEncContext *s, const AVFrame *pic_arg) } } - if (!pic_arg->buf[0] || - pic_arg->linesize[0] != s->linesize || - pic_arg->linesize[1] != s->uvlinesize || - pic_arg->linesize[2] != s->uvlinesize) - direct = 0; - if ((s->width & 15) || (s->height & 15)) - direct = 0; - if (((intptr_t)(pic_arg->data[0])) & (STRIDE_ALIGN-1)) - direct = 0; - if (s->linesize & (STRIDE_ALIGN-1)) - direct = 0; - ff_dlog(s->avctx, "%d %d %"PTRDIFF_SPECIFIER" %"PTRDIFF_SPECIFIER"\n", pic_arg->linesize[0], pic_arg->linesize[1], s->linesize, s->uvlinesize); - i = ff_find_unused_picture(s->avctx, s->picture, direct); + i = ff_find_unused_picture(s->avctx, s->picture, 0); if (i < 0) return i; pic = &s->picture[i]; pic->reference = 3; - if (direct) { - if ((ret = av_frame_ref(pic->f, pic_arg)) < 0) - return ret; - } - ret = alloc_picture(s, pic, direct); + ret = alloc_picture(s, pic, 0); if (ret < 0) return ret; - if (!direct) { + if (1) { if (pic->f->data[0] + INPLACE_OFFSET == pic_arg->data[0] && pic->f->data[1] + INPLACE_OFFSET == pic_arg->data[1] && pic->f->data[2] + INPLACE_OFFSET == pic_arg->data[2]) {
It seems it does not work properly. Signed-off-by: Paul B Mahol <onemda@gmail.com> --- libavcodec/mpegvideo_enc.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)