[FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[Paul B Mahol]

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(-)

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]) {
-- 
2.33.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 736 bytes --]

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

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

Opposition brings concord. Out of discord comes the fairest harmony.
-- Heraclitus

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[Paul B Mahol]

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
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1545 bytes --]

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     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[Andreas Rheinhardt]

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
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] avcodec/mpegvideo_enc: remove direct=1 support

[Paul B Mahol]

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
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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