error_resilience: Provide approximate MVs when not available

Message ID 1426005902-9482-1-git-send-email-vittorio.giovara@gmail.com
State Old
Headers show

Commit Message

Vittorio Giovara March 10, 2015, 4:45 p.m.
From: Michael Niedermayer <michaelni@gmx.at>

Originally removed in d66e305, however mpeg1 videos encoded with avconv
do not play correctly without it.

CC: libav-stable@libav.org
Signed-off-by: Vittorio Giovara <vittorio.giovara@gmail.com>
---
 libavcodec/error_resilience.c | 35 +++++++++++++++++++++++++++++++----
 libavcodec/error_resilience.h |  3 +++
 2 files changed, 34 insertions(+), 4 deletions(-)

Comments

Luca Barbato March 10, 2015, 5:05 p.m. | #1
On 10/03/15 17:45, Vittorio Giovara wrote:
> From: Michael Niedermayer <michaelni@gmx.at>
>
> Originally removed in d66e305, however mpeg1 videos encoded with avconv
> do not play correctly without it.

Probably it is a good idea, but that doesn't mean that the decoder is 
leveraging something it shouldn't for a non-corrupted bitstream?

And why we did not catch it on fate?

lu
Vittorio Giovara March 10, 2015, 5:13 p.m. | #2
On Tue, Mar 10, 2015 at 5:05 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
> On 10/03/15 17:45, Vittorio Giovara wrote:
>>
>> From: Michael Niedermayer <michaelni@gmx.at>
>>
>> Originally removed in d66e305, however mpeg1 videos encoded with avconv
>> do not play correctly without it.
>
>
> Probably it is a good idea, but that doesn't mean that the decoder is
> leveraging something it shouldn't for a non-corrupted bitstream?

Well it's either that or the mpeg1 encoder is producing broken
streams: also quicktime showed the same artefacts that avplay showed
without this patch.

> And why we did not catch it on fate?

Maybe it has something to deal with the video size: fate tests use a
small pgm image, I tried with a 1920x1080 video.
Luca Barbato March 10, 2015, 5:18 p.m. | #3
On 10/03/15 18:13, Vittorio Giovara wrote:
> On Tue, Mar 10, 2015 at 5:05 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
>> On 10/03/15 17:45, Vittorio Giovara wrote:
>>>
>>> From: Michael Niedermayer <michaelni@gmx.at>
>>>
>>> Originally removed in d66e305, however mpeg1 videos encoded with avconv
>>> do not play correctly without it.
>>
>>
>> Probably it is a good idea, but that doesn't mean that the decoder is
>> leveraging something it shouldn't for a non-corrupted bitstream?
>
> Well it's either that or the mpeg1 encoder is producing broken
> streams: also quicktime showed the same artefacts that avplay showed
> without this patch.

Wait a second, this patch fixes the decoding or the encoding or both?

Ok that mpeg1 is not exactly used but would be nice to not create 
broken-by-default files.

>> And why we did not catch it on fate?
>
> Maybe it has something to deal with the video size: fate tests use a
> small pgm image, I tried with a 1920x1080 video.

I see, I used testsrc and it showing the problem easily.

lu
Vittorio Giovara March 10, 2015, 5:28 p.m. | #4
On Tue, Mar 10, 2015 at 5:18 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
> On 10/03/15 18:13, Vittorio Giovara wrote:
>>
>> On Tue, Mar 10, 2015 at 5:05 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
>>>
>>> On 10/03/15 17:45, Vittorio Giovara wrote:
>>>>
>>>>
>>>> From: Michael Niedermayer <michaelni@gmx.at>
>>>>
>>>> Originally removed in d66e305, however mpeg1 videos encoded with avconv
>>>> do not play correctly without it.
>>>
>>>
>>>
>>> Probably it is a good idea, but that doesn't mean that the decoder is
>>> leveraging something it shouldn't for a non-corrupted bitstream?
>>
>>
>> Well it's either that or the mpeg1 encoder is producing broken
>> streams: also quicktime showed the same artefacts that avplay showed
>> without this patch.
>
>
> Wait a second, this patch fixes the decoding or the encoding or both?

Only for decoding.
Luca Barbato March 10, 2015, 7:02 p.m. | #5
On 10/03/15 18:28, Vittorio Giovara wrote:
> On Tue, Mar 10, 2015 at 5:18 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
>> On 10/03/15 18:13, Vittorio Giovara wrote:
>>>
>>> On Tue, Mar 10, 2015 at 5:05 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
>>>>
>>>> On 10/03/15 17:45, Vittorio Giovara wrote:
>>>>>
>>>>>
>>>>> From: Michael Niedermayer <michaelni@gmx.at>
>>>>>
>>>>> Originally removed in d66e305, however mpeg1 videos encoded with avconv
>>>>> do not play correctly without it.
>>>>
>>>>
>>>>
>>>> Probably it is a good idea, but that doesn't mean that the decoder is
>>>> leveraging something it shouldn't for a non-corrupted bitstream?
>>>
>>>
>>> Well it's either that or the mpeg1 encoder is producing broken
>>> streams: also quicktime showed the same artefacts that avplay showed
>>> without this patch.
>>
>>
>> Wait a second, this patch fixes the decoding or the encoding or both?
>
> Only for decoding.
>

The encoder must be fixed before even thinking about "fixing" the decoder.

lu
Vittorio Giovara June 13, 2015, 10:18 p.m. | #6
On Tue, Mar 10, 2015 at 7:02 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
> On 10/03/15 18:28, Vittorio Giovara wrote:
>>
>> On Tue, Mar 10, 2015 at 5:18 PM, Luca Barbato <lu_zero@gentoo.org> wrote:
>>>
>>> On 10/03/15 18:13, Vittorio Giovara wrote:
>>>>
>>>>
>>>> On Tue, Mar 10, 2015 at 5:05 PM, Luca Barbato <lu_zero@gentoo.org>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 10/03/15 17:45, Vittorio Giovara wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> From: Michael Niedermayer <michaelni@gmx.at>
>>>>>>
>>>>>> Originally removed in d66e305, however mpeg1 videos encoded with
>>>>>> avconv
>>>>>> do not play correctly without it.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Probably it is a good idea, but that doesn't mean that the decoder is
>>>>> leveraging something it shouldn't for a non-corrupted bitstream?
>>>>
>>>>
>>>>
>>>> Well it's either that or the mpeg1 encoder is producing broken
>>>> streams: also quicktime showed the same artefacts that avplay showed
>>>> without this patch.
>>>
>>>
>>>
>>> Wait a second, this patch fixes the decoding or the encoding or both?
>>
>>
>> Only for decoding.
>>
>
> The encoder must be fixed before even thinking about "fixing" the decoder.

Anyone willing to pick up this old issue?

Patch

diff --git a/libavcodec/error_resilience.c b/libavcodec/error_resilience.c
index 0120109..53579ba 100644
--- a/libavcodec/error_resilience.c
+++ b/libavcodec/error_resilience.c
@@ -821,6 +821,27 @@  void ff_er_add_slice(ERContext *s, int startx, int starty,
     }
 }
 
+static int estimate_mv(ERContext *s)
+{
+    int size = s->b8_stride * 2 * s->mb_height;
+    int i;
+
+    av_log(s->avctx, AV_LOG_WARNING, "MVs not available, estimating...\n");
+
+    for (i = 0; i < 2; i++) {
+        s->ref_index_buf[i]  = av_buffer_allocz(s->mb_stride * s->mb_height *
+                                                4 * sizeof(uint8_t));
+        s->motion_val_buf[i] = av_buffer_allocz((size + 4) *
+                                                2 * sizeof(uint16_t));
+        if (!s->ref_index_buf[i] || !s->motion_val_buf[i])
+            return AVERROR(ENOMEM);
+
+        s->cur_pic.ref_index[i]  = s->ref_index_buf[i]->data;
+        s->cur_pic.motion_val[i] = (int16_t (*)[2])s->motion_val_buf[i]->data + 4;
+    }
+    return 0;
+}
+
 void ff_er_frame_end(ERContext *s)
 {
     int *linesize = s->cur_pic.f->linesize;
@@ -841,10 +862,9 @@  void ff_er_frame_end(ERContext *s)
         return;
     };
 
-    if (!s->cur_pic.motion_val[0] || !s->cur_pic.ref_index[0]) {
-        av_log(s->avctx, AV_LOG_ERROR, "MVs not available, ER not possible.\n");
-        return;
-    }
+    if (!s->cur_pic.motion_val[0] || !s->cur_pic.ref_index[0])
+        if (estimate_mv(s) < 0)
+            goto ec_clean;
 
     if (s->avctx->debug & FF_DEBUG_ER) {
         for (mb_y = 0; mb_y < s->mb_height; mb_y++) {
@@ -1212,6 +1232,13 @@  ec_clean:
         s->mbintra_table[mb_xy] = 1;
     }
 
+    for (i = 0; i < 2; i++) {
+        av_buffer_unref(&s->ref_index_buf[i]);
+        av_buffer_unref(&s->motion_val_buf[i]);
+        s->cur_pic.ref_index[i]  = NULL;
+        s->cur_pic.motion_val[i] = NULL;
+    }
+
     memset(&s->cur_pic, 0, sizeof(ERPicture));
     memset(&s->last_pic, 0, sizeof(ERPicture));
     memset(&s->next_pic, 0, sizeof(ERPicture));
diff --git a/libavcodec/error_resilience.h b/libavcodec/error_resilience.h
index 611b529..1a1fa6b 100644
--- a/libavcodec/error_resilience.h
+++ b/libavcodec/error_resilience.h
@@ -73,6 +73,9 @@  typedef struct ERContext {
     ERPicture last_pic;
     ERPicture next_pic;
 
+    AVBufferRef *ref_index_buf[2];
+    AVBufferRef *motion_val_buf[2];
+
     uint16_t pp_time;
     uint16_t pb_time;
     int quarter_sample;