[FFmpeg-devel] [PATCH 0/3] avcodec/h264dec: Fix dropped frames when draining

[FFmpeg-devel] [PATCH 0/3] avcodec/h264dec: Fix dropped frames when draining

[arch1t3cht]

Fix one or more frames being dropped when starting decoding at a SEI
recovery point that is very close to the end of the stream
(specifically, which is less than 2 * has_b_frames frames before the
end of the stream in decoding order). One case of this was reported
in ticket #10936.

Tested for regressions using FATE. If necessary I can also try to add
a test for the previously broken behavior.

There's now a bit of code duplication between
h264_select_output_frame and send_next_delayed_frame (or, rather,
there was already some code duplication and this patch
adds a bit more). But this probably cannot be avoided without a larger
refactor to, say, also call h264_select_output_frame in
send_next_delayed_frame instead of manually selecting a frame, which
I don't feel confident enough to do myself.

arch1t3cht (3):
  avcodec/h264dec: Properly mark frames as recovered when draining
  avcodec/h264dec: Handle non-recovered frames when draining
  avcodec/h264dec: Reindent after the previous commit

 libavcodec/h264dec.c | 44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

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

[FFmpeg-devel] [PATCH 1/3] avcodec/h264dec: Properly mark frames as recovered when draining

[arch1t3cht]

When decoding starts at a SEI recovery point very shortly before the
end of the video stream, there can be frames which are decoded before
the recovery point's frame is output and which will only be output once
the draining has started. Previously, these frames would never be set
as recovered. This commit copies the logic from h264_select_output_frame
to send_next_delayed_frame to properly mark such frames as recovered.

Fixes ticket #10936.

Signed-off-by: arch1t3cht <arch1t3cht@gmail.com>
---
 libavcodec/h264dec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index b4973fce29..8503ea018a 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -1013,6 +1013,9 @@ static int send_next_delayed_frame(H264Context *h, AVFrame *dst_frame,
         h->delayed_pic[i] = h->delayed_pic[i + 1];
 
     if (out) {
+        h->frame_recovered |= out->recovered;
+        out->recovered |= h->frame_recovered & FRAME_RECOVERED_SEI;
+
         out->reference &= ~DELAYED_PIC_REF;
         ret = finalize_frame(h, dst_frame, out, got_frame);
         if (ret < 0)
-- 
2.44.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".

[FFmpeg-devel] [PATCH 2/3] avcodec/h264dec: Handle non-recovered frames when draining

[arch1t3cht]

When starting on a SEI recovery point close enough to the end of
the stream that draining starts before the recovery point's frame
is output, there can be non-recovered frames in the delayed picture
buffer that would currently cause the decoder to fail to output a
frame. This commit skips such frames and outputs the first recovered
frame, if there exists one.

Signed-off-by: arch1t3cht <arch1t3cht@gmail.com>
---
 libavcodec/h264dec.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 8503ea018a..68f47912e2 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -993,11 +993,13 @@ static int send_next_delayed_frame(H264Context *h, AVFrame *dst_frame,
                                    int *got_frame, int buf_index)
 {
     int ret, i, out_idx;
-    H264Picture *out = h->delayed_pic[0];
+    H264Picture *out;
 
     h->cur_pic_ptr = NULL;
     h->first_field = 0;
 
+    while (h->delayed_pic[0]) {
+    out = h->delayed_pic[0];
     out_idx = 0;
     for (i = 1;
          h->delayed_pic[i] &&
@@ -1020,6 +1022,9 @@ static int send_next_delayed_frame(H264Context *h, AVFrame *dst_frame,
         ret = finalize_frame(h, dst_frame, out, got_frame);
         if (ret < 0)
             return ret;
+        if (*got_frame)
+            break;
+    }
     }
 
     return buf_index;
-- 
2.44.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".

[FFmpeg-devel] [PATCH 3/3] avcodec/h264dec: Reindent after the previous commit

[arch1t3cht]

Signed-off-by: arch1t3cht <arch1t3cht@gmail.com>
---
 libavcodec/h264dec.c | 46 ++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/libavcodec/h264dec.c b/libavcodec/h264dec.c
index 68f47912e2..6f62fadb11 100644
--- a/libavcodec/h264dec.c
+++ b/libavcodec/h264dec.c
@@ -999,32 +999,32 @@ static int send_next_delayed_frame(H264Context *h, AVFrame *dst_frame,
     h->first_field = 0;
 
     while (h->delayed_pic[0]) {
-    out = h->delayed_pic[0];
-    out_idx = 0;
-    for (i = 1;
-         h->delayed_pic[i] &&
-         !(h->delayed_pic[i]->f->flags & AV_FRAME_FLAG_KEY) &&
-         !h->delayed_pic[i]->mmco_reset;
-         i++)
-        if (h->delayed_pic[i]->poc < out->poc) {
-            out     = h->delayed_pic[i];
-            out_idx = i;
-        }
+        out = h->delayed_pic[0];
+        out_idx = 0;
+        for (i = 1;
+             h->delayed_pic[i] &&
+             !(h->delayed_pic[i]->f->flags & AV_FRAME_FLAG_KEY) &&
+             !h->delayed_pic[i]->mmco_reset;
+             i++)
+            if (h->delayed_pic[i]->poc < out->poc) {
+                out     = h->delayed_pic[i];
+                out_idx = i;
+            }
 
-    for (i = out_idx; h->delayed_pic[i]; i++)
-        h->delayed_pic[i] = h->delayed_pic[i + 1];
+        for (i = out_idx; h->delayed_pic[i]; i++)
+            h->delayed_pic[i] = h->delayed_pic[i + 1];
 
-    if (out) {
-        h->frame_recovered |= out->recovered;
-        out->recovered |= h->frame_recovered & FRAME_RECOVERED_SEI;
+        if (out) {
+            h->frame_recovered |= out->recovered;
+            out->recovered |= h->frame_recovered & FRAME_RECOVERED_SEI;
 
-        out->reference &= ~DELAYED_PIC_REF;
-        ret = finalize_frame(h, dst_frame, out, got_frame);
-        if (ret < 0)
-            return ret;
-        if (*got_frame)
-            break;
-    }
+            out->reference &= ~DELAYED_PIC_REF;
+            ret = finalize_frame(h, dst_frame, out, got_frame);
+            if (ret < 0)
+                return ret;
+            if (*got_frame)
+                break;
+        }
     }
 
     return buf_index;
-- 
2.44.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 0/3] avcodec/h264dec: Fix dropped frames when draining

[arch1t3cht]

On 28/03/2024 02:56, arch1t3cht wrote:
> Fix one or more frames being dropped when starting decoding at a SEI
> recovery point that is very close to the end of the stream
> (specifically, which is less than 2 * has_b_frames frames before the
> end of the stream in decoding order). One case of this was reported
> in ticket #10936.
> 
> Tested for regressions using FATE. If necessary I can also try to add
> a test for the previously broken behavior.
> 
> There's now a bit of code duplication between
> h264_select_output_frame and send_next_delayed_frame (or, rather,
> there was already some code duplication and this patch
> adds a bit more). But this probably cannot be avoided without a larger
> refactor to, say, also call h264_select_output_frame in
> send_next_delayed_frame instead of manually selecting a frame, which
> I don't feel confident enough to do myself.
> 
> arch1t3cht (3):
>    avcodec/h264dec: Properly mark frames as recovered when draining
>    avcodec/h264dec: Handle non-recovered frames when draining
>    avcodec/h264dec: Reindent after the previous commit
> 
>   libavcodec/h264dec.c | 44 ++++++++++++++++++++++++++------------------
>   1 file changed, 26 insertions(+), 18 deletions(-)
> 
Can I bump this? It's my first time sending a patch here so let me know
if I did something wrong.

-arch1t3cht
_______________________________________________
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 0/3] avcodec/h264dec: Fix dropped frames when draining

[Derek Buitenhuis]

On 4/3/2024 10:45 PM, arch1t3cht wrote:
> Can I bump this? It's my first time sending a patch here so let me know
> if I did something wrong.

To me, it LGTM, but I would like someone more experience in H.264 internals
to OK it, too - possibly Michael?

- Derek
_______________________________________________
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 0/3] avcodec/h264dec: Fix dropped frames when draining

[Derek Buitenhuis]

On 4/9/2024 4:15 PM, Derek Buitenhuis wrote:
> To me, it LGTM, but I would like someone more experience in H.264 internals
> to OK it, too - possibly Michael?

If there are no objections raised, I will merge this tomorrow.

- Derek
_______________________________________________
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".