diff mbox

[FFmpeg-devel] lavc/mpeg2dec: fix MPEG2 VA-API interlace decoding issue.

Message ID 1537183291-992-1-git-send-email-mypopydev@gmail.com
State New
Headers show

Commit Message

Jun Zhao Sept. 17, 2018, 11:21 a.m. UTC 
From: Jun Zhao <jun.zhao@intel.com>

For interlaced frame, it has to call slice_end() for both fields. And
VASliceParameterBufferMPEG2::slice_vertical_position is supposed to be
the position in the picture but not field in this case.

Signed-off-by: Dong, Jerry <jerry.dong@intel.com>
Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 libavcodec/mpeg12dec.c   |    9 +++++++++
 libavcodec/vaapi_mpeg2.c |    2 +-
 2 files changed, 10 insertions(+), 1 deletions(-)

Comments

Hendrik Leppkes Sept. 17, 2018, noon UTC | #1 
On Mon, Sep 17, 2018 at 1:22 PM Jun Zhao <mypopydev@gmail.com> wrote:
>
> From: Jun Zhao <jun.zhao@intel.com>
>
> For interlaced frame, it has to call slice_end() for both fields. And
> VASliceParameterBufferMPEG2::slice_vertical_position is supposed to be
> the position in the picture but not field in this case.
>

How does this impact other hwaccels or software decoding? Because
those cases seem to be working just fine the way it is now, from what
I can tell.

Also, in the VA-API calling code, are you setting avctx->slice_flags
to SLICE_FLAG_ALLOW_FIELD? Because that forces an end_frame call for
every field, which may be all that is needed here.

- Hendrik
mypopy@gmail.com Sept. 20, 2018, 12:43 a.m. UTC | #2 
On Mon, Sep 17, 2018 at 8:00 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote:
>
> On Mon, Sep 17, 2018 at 1:22 PM Jun Zhao <mypopydev@gmail.com> wrote:
> >
> > From: Jun Zhao <jun.zhao@intel.com>
> >
> > For interlaced frame, it has to call slice_end() for both fields. And
> > VASliceParameterBufferMPEG2::slice_vertical_position is supposed to be
> > the position in the picture but not field in this case.
> >
>
> How does this impact other hwaccels or software decoding? Because
> those cases seem to be working just fine the way it is now, from what
> I can tell.
After double-check the coding logic, I think we don't need to change
mpeg12dec.c, will update V2 patch.
>
> Also, in the VA-API calling code, are you setting avctx->slice_flags
> to SLICE_FLAG_ALLOW_FIELD? Because that forces an end_frame call for
> every field, which may be all that is needed here.
>
> - Hendrik
And we are not setting SLICE_FLAG_ALLOW_FIELD in this case, this is a good
suggestion for me, will try this way.

Thanks.
diff mbox

Patch

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 83e5378..c21f133 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -2546,6 +2546,15 @@  static int decode_chunks(AVCodecContext *avctx, AVFrame *picture,
                     s2->er.error_count += s2->thread_context[i]->er.error_count;
                 s->slice_count = 0;
             }
+
+            if (s2->first_field) {
+                /* slice ends for the first field */
+                ret = slice_end(avctx, picture);
+                av_assert1(ret == 0);
+                if (ret < 0)
+                    return ret;
+            }
+
             if (last_code == 0 || last_code == SLICE_MIN_START_CODE) {
                 ret = mpeg_decode_postinit(avctx);
                 if (ret < 0) {
diff --git a/libavcodec/vaapi_mpeg2.c b/libavcodec/vaapi_mpeg2.c
index aaed434..b159f49 100644
--- a/libavcodec/vaapi_mpeg2.c
+++ b/libavcodec/vaapi_mpeg2.c
@@ -156,7 +156,7 @@  static int vaapi_mpeg2_decode_slice(AVCodecContext *avctx, const uint8_t *buffer
         .slice_data_flag            = VA_SLICE_DATA_FLAG_ALL,
         .macroblock_offset          = macroblock_offset,
         .slice_horizontal_position  = s->mb_x,
-        .slice_vertical_position    = s->mb_y >> (s->picture_structure != PICT_FRAME),
+        .slice_vertical_position    = s->mb_y,
         .quantiser_scale_code       = quantiser_scale_code,
         .intra_slice_flag           = intra_slice_flag,
     };