Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr
@ 2022-08-15 11:49 Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 2/7] avcodec/h263dec: Don't set frame parameters redundantly Andreas Rheinhardt
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:49 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

It is done later in ff_mpv_frame_start() (and nobody uses
current_picture_ptr between setting it in ff_mpv_frame_start()).

(The reason the vsynth*-h263-obmc code changes is because
the call to ff_find_unused_picture() now happens after the older
pictures have been unreferenced in ff_mpv_frame_start(),
so that their slots in the picture array can be immediately
reused; the obmc code is somehow buggy and changes its output
depending on the earlier contents of the motion_val buffer.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
I'd like to take this opportunity to once again ask anyone familiar
with H.263 to take a look at this OBMC issue.

 libavcodec/h263dec.c                   | 7 -------
 tests/ref/vsynth/vsynth1-h263-obmc     | 4 ++--
 tests/ref/vsynth/vsynth2-h263-obmc     | 4 ++--
 tests/ref/vsynth/vsynth_lena-h263-obmc | 4 ++--
 4 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index 8db0eccd89..a65f16caea 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -543,13 +543,6 @@ retry:
             return ret;
     }
 
-    if (!s->current_picture_ptr || s->current_picture_ptr->f->data[0]) {
-        int i = ff_find_unused_picture(s->avctx, s->picture, 0);
-        if (i < 0)
-            return i;
-        s->current_picture_ptr = &s->picture[i];
-    }
-
     avctx->has_b_frames = !s->low_delay;
 
     if (CONFIG_MPEG4_DECODER && avctx->codec_id == AV_CODEC_ID_MPEG4) {
diff --git a/tests/ref/vsynth/vsynth1-h263-obmc b/tests/ref/vsynth/vsynth1-h263-obmc
index b7a267a8cb..a9d0b166cf 100644
--- a/tests/ref/vsynth/vsynth1-h263-obmc
+++ b/tests/ref/vsynth/vsynth1-h263-obmc
@@ -1,4 +1,4 @@
 7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi
 657320 tests/data/fate/vsynth1-h263-obmc.avi
-844f7ee27fa122e199fe20987b41a15c *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
-stddev:    8.16 PSNR: 29.89 MAXDIFF:  113 bytes:  7603200/  7603200
+f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
+stddev:    8.12 PSNR: 29.93 MAXDIFF:  113 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth2-h263-obmc b/tests/ref/vsynth/vsynth2-h263-obmc
index 2cef7f551b..2275b6e446 100644
--- a/tests/ref/vsynth/vsynth2-h263-obmc
+++ b/tests/ref/vsynth/vsynth2-h263-obmc
@@ -1,4 +1,4 @@
 2d8a58b295e03f94e6a41468b2d3909e *tests/data/fate/vsynth2-h263-obmc.avi
 208522 tests/data/fate/vsynth2-h263-obmc.avi
-4a939ef99fc759293f2e609bfcacd2a4 *tests/data/fate/vsynth2-h263-obmc.out.rawvideo
-stddev:    6.10 PSNR: 32.41 MAXDIFF:   90 bytes:  7603200/  7603200
+20c4dda7bc5b4da28611a8c731cfa1c5 *tests/data/fate/vsynth2-h263-obmc.out.rawvideo
+stddev:    6.08 PSNR: 32.44 MAXDIFF:   81 bytes:  7603200/  7603200
diff --git a/tests/ref/vsynth/vsynth_lena-h263-obmc b/tests/ref/vsynth/vsynth_lena-h263-obmc
index 5b963107f6..a18ef8e9e3 100644
--- a/tests/ref/vsynth/vsynth_lena-h263-obmc
+++ b/tests/ref/vsynth/vsynth_lena-h263-obmc
@@ -1,4 +1,4 @@
 3c6946f808412ac320be9e0c36051ea2 *tests/data/fate/vsynth_lena-h263-obmc.avi
 154730 tests/data/fate/vsynth_lena-h263-obmc.avi
-588d992d9d8096da8bdc5027268da914 *tests/data/fate/vsynth_lena-h263-obmc.out.rawvideo
-stddev:    5.39 PSNR: 33.49 MAXDIFF:   82 bytes:  7603200/  7603200
+acc9705f4c9a019c2032a875a6a715ae *tests/data/fate/vsynth_lena-h263-obmc.out.rawvideo
+stddev:    5.39 PSNR: 33.50 MAXDIFF:   77 bytes:  7603200/  7603200
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH 2/7] avcodec/h263dec: Don't set frame parameters redundantly
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
@ 2022-08-15 11:52 ` Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 3/7] avcodec/mpegpicture: Reset fields explicitly instead of memsetting them Andreas Rheinhardt
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

This frame will be reset later in ff_mpv_frame_start()
anyway.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/h263dec.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavcodec/h263dec.c b/libavcodec/h263dec.c
index a65f16caea..a14d7811f5 100644
--- a/libavcodec/h263dec.c
+++ b/libavcodec/h263dec.c
@@ -583,10 +583,6 @@ retry:
         s->codec_id == AV_CODEC_ID_H263I)
         s->gob_index = H263_GOB_HEIGHT(s->height);
 
-    // for skipping the frame
-    s->current_picture.f->pict_type = s->pict_type;
-    s->current_picture.f->key_frame = s->pict_type == AV_PICTURE_TYPE_I;
-
     /* skip B-frames if we don't have reference frames */
     if (!s->last_picture_ptr &&
         (s->pict_type == AV_PICTURE_TYPE_B || s->droppable))
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH 3/7] avcodec/mpegpicture: Reset fields explicitly instead of memsetting them
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 2/7] avcodec/h263dec: Don't set frame parameters redundantly Andreas Rheinhardt
@ 2022-08-15 11:52 ` Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 4/7] avcodec/mpegvideo_dec: Remove commented-out cruft Andreas Rheinhardt
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Improves the grepability of the code.
(Furthermore, I hope that no compiler will really call memset
for 28 bytes.)

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mpegpicture.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libavcodec/mpegpicture.c b/libavcodec/mpegpicture.c
index c57f149752..884af1731b 100644
--- a/libavcodec/mpegpicture.c
+++ b/libavcodec/mpegpicture.c
@@ -297,8 +297,6 @@ fail:
  */
 void ff_mpeg_unref_picture(AVCodecContext *avctx, Picture *pic)
 {
-    int off = offsetof(Picture, hwaccel_priv_buf) + sizeof(pic->hwaccel_priv_buf);
-
     pic->tf.f = pic->f;
     /* WM Image / Screen codecs allocate internal buffers with different
      * dimensions / colorspaces; ignore user-defined callbacks for these. */
@@ -314,7 +312,12 @@ void ff_mpeg_unref_picture(AVCodecContext *avctx, Picture *pic)
     if (pic->needs_realloc)
         free_picture_tables(pic);
 
-    memset((uint8_t*)pic + off, 0, sizeof(*pic) - off);
+    pic->hwaccel_picture_private = NULL;
+    pic->field_picture = 0;
+    pic->b_frame_score = 0;
+    pic->needs_realloc = 0;
+    pic->reference     = 0;
+    pic->shared        = 0;
 }
 
 int ff_update_picture_tables(Picture *dst, const Picture *src)
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH 4/7] avcodec/mpegvideo_dec: Remove commented-out cruft
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 2/7] avcodec/h263dec: Don't set frame parameters redundantly Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 3/7] avcodec/mpegpicture: Reset fields explicitly instead of memsetting them Andreas Rheinhardt
@ 2022-08-15 11:52 ` Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 5/7] avcodec/get_buffer: Don't get AVPixFmtDescriptor unnecessarily Andreas Rheinhardt
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

The fields in question were removed in
759001c534287a96dc96d1e274665feb7059145d.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/mpegvideo_dec.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavcodec/mpegvideo_dec.c b/libavcodec/mpegvideo_dec.c
index 93ba4e31b3..17d0c73926 100644
--- a/libavcodec/mpegvideo_dec.c
+++ b/libavcodec/mpegvideo_dec.c
@@ -73,8 +73,6 @@ int ff_mpeg_update_thread_context(AVCodecContext *dst,
         s->bitstream_buffer_size = s->allocated_bitstream_buffer_size = 0;
 
         if (s1->context_initialized) {
-//             s->picture_range_start  += MAX_PICTURE_COUNT;
-//             s->picture_range_end    += MAX_PICTURE_COUNT;
             ff_mpv_idct_init(s);
             if ((err = ff_mpv_common_init(s)) < 0) {
                 memset(s, 0, sizeof(*s));
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH 5/7] avcodec/get_buffer: Don't get AVPixFmtDescriptor unnecessarily
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
                   ` (2 preceding siblings ...)
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 4/7] avcodec/mpegvideo_dec: Remove commented-out cruft Andreas Rheinhardt
@ 2022-08-15 11:52 ` Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 6/7] avcodec/ffv1enc: Don't create and keep unnecessary reference Andreas Rheinhardt
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

It is unused since 3575a495f6dcc395656343380e13c57d48b9f976
(and the error message is dangerous: av_get_pix_fmt_name(format)
returns NULL iff av_pix_fmt_desc_get(format) returns NULL
and using a NULL string for %s would be UB).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/get_buffer.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/libavcodec/get_buffer.c b/libavcodec/get_buffer.c
index 3e45a0479f..a04fd878de 100644
--- a/libavcodec/get_buffer.c
+++ b/libavcodec/get_buffer.c
@@ -246,7 +246,6 @@ fail:
 static int video_get_buffer(AVCodecContext *s, AVFrame *pic)
 {
     FramePool *pool = (FramePool*)s->internal->pool->data;
-    const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(pic->format);
     int i;
 
     if (pic->data[0] || pic->data[1] || pic->data[2] || pic->data[3]) {
@@ -254,13 +253,6 @@ static int video_get_buffer(AVCodecContext *s, AVFrame *pic)
         return -1;
     }
 
-    if (!desc) {
-        av_log(s, AV_LOG_ERROR,
-            "Unable to get pixel format descriptor for format %s\n",
-            av_get_pix_fmt_name(pic->format));
-        return AVERROR(EINVAL);
-    }
-
     memset(pic->data, 0, sizeof(pic->data));
     pic->extended_data = pic->data;
 
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH 6/7] avcodec/ffv1enc: Don't create and keep unnecessary reference
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
                   ` (3 preceding siblings ...)
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 5/7] avcodec/get_buffer: Don't get AVPixFmtDescriptor unnecessarily Andreas Rheinhardt
@ 2022-08-15 11:52 ` Andreas Rheinhardt
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 7/7] avcodec/ffv1enc: Remove redundant wrapper Andreas Rheinhardt
  2022-08-15 22:21 ` [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Michael Niedermayer
  6 siblings, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/ffv1.h    |  1 +
 libavcodec/ffv1enc.c | 15 ++++++---------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/libavcodec/ffv1.h b/libavcodec/ffv1.h
index ac80fa85ce..3532815501 100644
--- a/libavcodec/ffv1.h
+++ b/libavcodec/ffv1.h
@@ -91,6 +91,7 @@ typedef struct FFV1Context {
     struct FFV1Context *fsrc;
 
     AVFrame *cur;
+    const AVFrame *cur_enc_frame;
     int plane_count;
     int ac;                              ///< 1=range coder <-> 0=golomb rice
     int ac_byte_count;                   ///< number of bytes used for AC coding
diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index ec06636db5..6f8b8275b5 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -916,12 +916,12 @@ static void encode_slice_header(FFV1Context *f, FFV1Context *fs)
         put_symbol(c, state, f->plane[j].quant_table_index, 0);
         av_assert0(f->plane[j].quant_table_index == f->context_model);
     }
-    if (!f->picture.f->interlaced_frame)
+    if (!f->cur_enc_frame->interlaced_frame)
         put_symbol(c, state, 3, 0);
     else
-        put_symbol(c, state, 1 + !f->picture.f->top_field_first, 0);
-    put_symbol(c, state, f->picture.f->sample_aspect_ratio.num, 0);
-    put_symbol(c, state, f->picture.f->sample_aspect_ratio.den, 0);
+        put_symbol(c, state, 1 + !f->cur_enc_frame->top_field_first, 0);
+    put_symbol(c, state, f->cur_enc_frame->sample_aspect_ratio.num, 0);
+    put_symbol(c, state, f->cur_enc_frame->sample_aspect_ratio.den, 0);
     if (f->version > 3) {
         put_rac(c, state, fs->slice_coding_mode == 1);
         if (fs->slice_coding_mode == 1)
@@ -1024,7 +1024,7 @@ static int encode_slice(AVCodecContext *c, void *arg)
     int height       = fs->slice_height;
     int x            = fs->slice_x;
     int y            = fs->slice_y;
-    const AVFrame *const p = f->picture.f;
+    const AVFrame *const p = f->cur_enc_frame;
     const int ps     = av_pix_fmt_desc_get(c->pix_fmt)->comp[0].step;
     int ret;
     RangeCoder c_bak = fs->c;
@@ -1098,7 +1098,6 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 {
     FFV1Context *f      = avctx->priv_data;
     RangeCoder *const c = &f->slice_context[0]->c;
-    AVFrame *const p    = f->picture.f;
     uint8_t keystate    = 128;
     uint8_t *buf_p;
     int i, ret;
@@ -1165,9 +1164,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     ff_init_range_encoder(c, pkt->data, pkt->size);
     ff_build_rac_states(c, 0.05 * (1LL << 32), 256 - 8);
 
-    av_frame_unref(p);
-    if ((ret = av_frame_ref(p, pict)) < 0)
-        return ret;
+    f->cur_enc_frame = pict;
 
     if (avctx->gop_size == 0 || f->picture_number % avctx->gop_size == 0) {
         put_rac(c, &keystate, 1);
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [FFmpeg-devel] [PATCH 7/7] avcodec/ffv1enc: Remove redundant wrapper
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
                   ` (4 preceding siblings ...)
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 6/7] avcodec/ffv1enc: Don't create and keep unnecessary reference Andreas Rheinhardt
@ 2022-08-15 11:52 ` Andreas Rheinhardt
  2022-08-15 22:21 ` [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Michael Niedermayer
  6 siblings, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-15 11:52 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Andreas Rheinhardt

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/ffv1enc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/libavcodec/ffv1enc.c b/libavcodec/ffv1enc.c
index 6f8b8275b5..90593fbaf1 100644
--- a/libavcodec/ffv1enc.c
+++ b/libavcodec/ffv1enc.c
@@ -1240,12 +1240,6 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
     return 0;
 }
 
-static av_cold int encode_close(AVCodecContext *avctx)
-{
-    ff_ffv1_close(avctx);
-    return 0;
-}
-
 #define OFFSET(x) offsetof(FFV1Context, x)
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
@@ -1281,7 +1275,7 @@ const FFCodec ff_ffv1_encoder = {
     .priv_data_size = sizeof(FFV1Context),
     .init           = encode_init,
     FF_CODEC_ENCODE_CB(encode_frame),
-    .close          = encode_close,
+    .close          = ff_ffv1_close,
     .p.capabilities = AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_DELAY,
     .p.pix_fmts     = (const enum AVPixelFormat[]) {
         AV_PIX_FMT_YUV420P,   AV_PIX_FMT_YUVA420P,  AV_PIX_FMT_YUVA422P,  AV_PIX_FMT_YUV444P,
-- 
2.34.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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr
  2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
                   ` (5 preceding siblings ...)
  2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 7/7] avcodec/ffv1enc: Remove redundant wrapper Andreas Rheinhardt
@ 2022-08-15 22:21 ` Michael Niedermayer
  2022-08-16  7:08   ` Andreas Rheinhardt
  2022-08-17 22:05   ` Andreas Rheinhardt
  6 siblings, 2 replies; 10+ messages in thread
From: Michael Niedermayer @ 2022-08-15 22:21 UTC (permalink / raw)
  To: FFmpeg development discussions and patches


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

On Mon, Aug 15, 2022 at 01:49:24PM +0200, Andreas Rheinhardt wrote:
> It is done later in ff_mpv_frame_start() (and nobody uses
> current_picture_ptr between setting it in ff_mpv_frame_start()).
> 
> (The reason the vsynth*-h263-obmc code changes is because
> the call to ff_find_unused_picture() now happens after the older
> pictures have been unreferenced in ff_mpv_frame_start(),
> so that their slots in the picture array can be immediately
> reused; the obmc code is somehow buggy and changes its output
> depending on the earlier contents of the motion_val buffer.)
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---

> I'd like to take this opportunity to once again ask anyone familiar
> with H.263 to take a look at this OBMC issue.

Iam too busy ATM :( i might look at some point but its not very high
on my todo as it works :) security & release is more important

but i can say this breaks fate as it is:

--- ./tests/ref/vsynth/vsynth1-h263-obmc	2022-08-16 00:19:00.345967181 +0200
+++ tests/data/fate/vsynth1-h263-obmc	2022-08-16 00:19:05.262017999 +0200
@@ -1,4 +1,4 @@
 7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi
 657320 tests/data/fate/vsynth1-h263-obmc.avi
-f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
-stddev:    8.12 PSNR: 29.93 MAXDIFF:  113 bytes:  7603200/  7603200
+2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
+stddev:    8.38 PSNR: 29.66 MAXDIFF:  116 bytes:  7603200/  7603200
Test vsynth1-h263-obmc failed. Look at tests/data/fate/vsynth1-h263-obmc.err for details.
tests/Makefile:304: recipe for target 'fate-vsynth1-h263-obmc' failed
make: *** [fate-vsynth1-h263-obmc] Error 1

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

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

[-- 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".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr
  2022-08-15 22:21 ` [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Michael Niedermayer
@ 2022-08-16  7:08   ` Andreas Rheinhardt
  2022-08-17 22:05   ` Andreas Rheinhardt
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-16  7:08 UTC (permalink / raw)
  To: ffmpeg-devel

Michael Niedermayer:
> On Mon, Aug 15, 2022 at 01:49:24PM +0200, Andreas Rheinhardt wrote:
>> It is done later in ff_mpv_frame_start() (and nobody uses
>> current_picture_ptr between setting it in ff_mpv_frame_start()).
>>
>> (The reason the vsynth*-h263-obmc code changes is because
>> the call to ff_find_unused_picture() now happens after the older
>> pictures have been unreferenced in ff_mpv_frame_start(),
>> so that their slots in the picture array can be immediately
>> reused; the obmc code is somehow buggy and changes its output
>> depending on the earlier contents of the motion_val buffer.)
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
> 
>> I'd like to take this opportunity to once again ask anyone familiar
>> with H.263 to take a look at this OBMC issue.
> 
> Iam too busy ATM :( i might look at some point but its not very high
> on my todo as it works :) security & release is more important
> 

Good that you became aware of this issue. (You were the "anyone familiar
with H.263" person I thought of.)

> but i can say this breaks fate as it is:
> 

I am aware of this; this is due to a slight conflict with
b645138a34321fb1d1b7988cd0d78b897e4d65ca. The ref file changes were
appropriate for git master at the time I sent this. I already updated
mine locally.

> --- ./tests/ref/vsynth/vsynth1-h263-obmc	2022-08-16 00:19:00.345967181 +0200
> +++ tests/data/fate/vsynth1-h263-obmc	2022-08-16 00:19:05.262017999 +0200
> @@ -1,4 +1,4 @@
>  7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi
>  657320 tests/data/fate/vsynth1-h263-obmc.avi
> -f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
> -stddev:    8.12 PSNR: 29.93 MAXDIFF:  113 bytes:  7603200/  7603200
> +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
> +stddev:    8.38 PSNR: 29.66 MAXDIFF:  116 bytes:  7603200/  7603200
> Test vsynth1-h263-obmc failed. Look at tests/data/fate/vsynth1-h263-obmc.err for details.
> tests/Makefile:304: recipe for target 'fate-vsynth1-h263-obmc' failed
> make: *** [fate-vsynth1-h263-obmc] Error 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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr
  2022-08-15 22:21 ` [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Michael Niedermayer
  2022-08-16  7:08   ` Andreas Rheinhardt
@ 2022-08-17 22:05   ` Andreas Rheinhardt
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Rheinhardt @ 2022-08-17 22:05 UTC (permalink / raw)
  To: ffmpeg-devel

Michael Niedermayer:
> On Mon, Aug 15, 2022 at 01:49:24PM +0200, Andreas Rheinhardt wrote:
>> It is done later in ff_mpv_frame_start() (and nobody uses
>> current_picture_ptr between setting it in ff_mpv_frame_start()).
>>
>> (The reason the vsynth*-h263-obmc code changes is because
>> the call to ff_find_unused_picture() now happens after the older
>> pictures have been unreferenced in ff_mpv_frame_start(),
>> so that their slots in the picture array can be immediately
>> reused; the obmc code is somehow buggy and changes its output
>> depending on the earlier contents of the motion_val buffer.)
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
> 
>> I'd like to take this opportunity to once again ask anyone familiar
>> with H.263 to take a look at this OBMC issue.
> 
> Iam too busy ATM :( i might look at some point but its not very high
> on my todo as it works :) security & release is more important
> 

If you (and no one else) don't object, I'll apply this tomorrow.

> but i can say this breaks fate as it is:
> 
> --- ./tests/ref/vsynth/vsynth1-h263-obmc	2022-08-16 00:19:00.345967181 +0200
> +++ tests/data/fate/vsynth1-h263-obmc	2022-08-16 00:19:05.262017999 +0200
> @@ -1,4 +1,4 @@
>  7dec64380f375e5118b66f3baaaa1e24 *tests/data/fate/vsynth1-h263-obmc.avi
>  657320 tests/data/fate/vsynth1-h263-obmc.avi
> -f5048b5f0c98833a1d11f8034fb1827f *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
> -stddev:    8.12 PSNR: 29.93 MAXDIFF:  113 bytes:  7603200/  7603200
> +2a69f6b37378aa34418dfd04ec98c1c8 *tests/data/fate/vsynth1-h263-obmc.out.rawvideo
> +stddev:    8.38 PSNR: 29.66 MAXDIFF:  116 bytes:  7603200/  7603200
> Test vsynth1-h263-obmc failed. Look at tests/data/fate/vsynth1-h263-obmc.err for details.
> tests/Makefile:304: recipe for target 'fate-vsynth1-h263-obmc' failed
> make: *** [fate-vsynth1-h263-obmc] Error 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-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-08-17 22:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-15 11:49 [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Andreas Rheinhardt
2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 2/7] avcodec/h263dec: Don't set frame parameters redundantly Andreas Rheinhardt
2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 3/7] avcodec/mpegpicture: Reset fields explicitly instead of memsetting them Andreas Rheinhardt
2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 4/7] avcodec/mpegvideo_dec: Remove commented-out cruft Andreas Rheinhardt
2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 5/7] avcodec/get_buffer: Don't get AVPixFmtDescriptor unnecessarily Andreas Rheinhardt
2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 6/7] avcodec/ffv1enc: Don't create and keep unnecessary reference Andreas Rheinhardt
2022-08-15 11:52 ` [FFmpeg-devel] [PATCH 7/7] avcodec/ffv1enc: Remove redundant wrapper Andreas Rheinhardt
2022-08-15 22:21 ` [FFmpeg-devel] [PATCH 1/7] avcodec/h263dec: Remove redundant code to set cur_pic_ptr Michael Niedermayer
2022-08-16  7:08   ` Andreas Rheinhardt
2022-08-17 22:05   ` Andreas Rheinhardt

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git