Re: [libav-devel] [PATCH V2] lavc/qsvdec: expose frame pic_type and key_frame

2018-01-31 Thread Li, Zhong 
> On 31/01/18 08:16, Li, Zhong wrote:
> > As your comment, maybe we need to find a HEVC clip with CRA frames to
> test.
> 
> Try the conformance sample RAP_A_docomo_4.bit (you can find it in
> fate/hevc-conformance).  That contains CRA and RASL frames (and no IDR
> frames at all).
> 
> - Mark

Tested it. Yup, it is an issue just like your inference, the CRA frames aren't 
set as key frame for QSV. But software codec set as key frames.
I haven't find where MSDK expose this, maybe need file an issue to MSDK. 


___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] Help with patch identification

2018-01-31 Thread Luca Barbato 

On 31/01/2018 16:04, Andrey Voropaev wrote:

Hello all!

I have rather unusual request. There's one video file that causes
segfault when played with libavcodec54 under Ubuntu (full version
6:9.20-0ubuntu0.14.04.1). The same file plays OK with newer versions
of libavcodec, in particular with libavcodec57 (full version
7:3.4.1-1).

I can't upgrade version of libavcodec in my version of Ubuntu, but I
thought I could try to apply patch that fixes the problem. There is
only one small obstacle, I don't know which patch is that :) So, I'm
coming here in hope that someone can identify the problem that happens
and then point me to some patch(es) in git that fix this.

The file that causes segfault can be found at
https://drive.google.com/open?id=16w3AwVX7a1JEebuw2LAksED_0Oyvdtb5



You can automate it by using git bisect with a test script. In general 
it would take a bit of time but it should give you a quite precise result.


___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] Help with patch identification

2018-01-31 Thread Andrey Voropaev 
Hello all!

I have rather unusual request. There's one video file that causes
segfault when played with libavcodec54 under Ubuntu (full version
6:9.20-0ubuntu0.14.04.1). The same file plays OK with newer versions
of libavcodec, in particular with libavcodec57 (full version
7:3.4.1-1).

I can't upgrade version of libavcodec in my version of Ubuntu, but I
thought I could try to apply patch that fixes the problem. There is
only one small obstacle, I don't know which patch is that :) So, I'm
coming here in hope that someone can identify the problem that happens
and then point me to some patch(es) in git that fix this.

The file that causes segfault can be found at
https://drive.google.com/open?id=16w3AwVX7a1JEebuw2LAksED_0Oyvdtb5

Thank you for your help

Andrey Voropaev
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH V2] lavc/qsvdec: expose frame pic_type and key_frame

2018-01-31 Thread Mark Thompson 
On 31/01/18 08:11, Li, Zhong wrote:
>>> @@ -416,6 +421,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
> 
>>>  outsurf->Info.PicStruct & MFX_PICSTRUCT_FIELD_TFF;
> 
>>>  frame->interlaced_frame =
> 
>>>  !(outsurf->Info.PicStruct &
> 
>> MFX_PICSTRUCT_PROGRESSIVE);
> 
>>> +frame->pict_type =
> 
>> ff_qsv_map_pictype(out_frame->dec_info.FrameType);
> 
>>> +frame->key_frame = !!(out_frame->dec_info.FrameType &
> 
>>> + MFX_FRAMETYPE_IDR);
> 
>>>
> 
>>>  /* update the surface properties */
> 
>>>  if (avctx->pix_fmt == AV_PIX_FMT_QSV)
> 
>>>
> 
>>
> 
>> Does this do the right thing for H.265?  (Can't test right now.)
> 
> 
> 
> Testing should use a HEVC clips
> 
> 
> 
>> I'm asking that because I know that frame types are still a problem for H.265
> 
>> encoding, because not all IRAPs are IDR - when CRA frames are generated
> 
>> they aren't marked as key.  The FrameType field (as used here) is not
> 
>> sufficient in that case, but I never sorted out exactly what does contain the
> 
>> necessary information.
> 
>>
> 
>> - Mark
> 
> 
> 
> Exactly, only IDR type is exposed from FrameType here, and it is set by 
> parsing slice_header 
> (https://github.com/Intel-Media-SDK/MediaSDK/blob/master/_studio/shared/umc/codec/h265_dec/src/umc_h265_bitstream_headers.cpp#L1428
>  )
> 
> But the problem is (I am not familiar with HEVC as well as H264, so please 
> correct me if I am wrong), should CRA frames be treated as key frames?

Yes, they have to be.  IDR, CRA and BLA frames are all IRAP pictures, so they 
are key frames because seeking can always move to that point in the stream.

> See the description as blew (source 
> http://ieeexplore.ieee.org/document/6324417/?part=undefined%7Cfig4#fig4 )
> 
> [cid:image003.jpg@01D39AAE.23C1D950]
> 
> 
> 
> Here some leading pictures such as B30 can’t be decoded when random access 
> from I28.
> 
> As my understanding, all frames behand (in display order) key frame should be 
> decodable.

Yes, all frames after the IRAP in display order should be decodable.  However, 
B30 is not after I28 in display order - it be marked as RASL to indicate that 
it should be skipped when it is found in decode order.

> IDR it can make sure this, but CRA can’t. So at least not all of the CRA 
> frames can be treated as key frames.
> 
> Looks some people agree with me (refer Jeanlf’s comment on 
> https://github.com/gpac/gpac/issues/144)
I'm not familiar with the precise meaning of "sync sample" in this case, so I'm 
not sure.  If it means the same thing as IRAP (a point at which decoding of a 
stream can always start correctly without any pictures which precede it in 
decode order), then the commenter is wrong.


On 31/01/18 08:16, Li, Zhong wrote:
> As your comment, maybe we need to find a HEVC clip with CRA frames to test.

Try the conformance sample RAP_A_docomo_4.bit (you can find it in 
fate/hevc-conformance).  That contains CRA and RASL frames (and no IDR frames 
at all).

- Mark
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure issue

2018-01-31 Thread Li, Zhong 
> From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of Mark
> Thompson
> Sent: Wednesday, January 31, 2018 6:17 AM
> To: libav-devel@libav.org
> Subject: Re: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload failure
> issue
> 
> On 30/01/18 03:35, Song, Ruiling wrote:
> >> -Original Message-
> >> From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf Of
> >> wm4
> >> Sent: Friday, January 26, 2018 5:15 PM
> >> To: libav-devel@libav.org
> >> Subject: Re: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload
> >> failure issue
> >>
> >> On Fri, 26 Jan 2018 05:56:46 +
> >> "Li, Zhong"  wrote:
> >>
>  From: libav-devel [mailto:libav-devel-boun...@libav.org] On Behalf
>  Of Ruiling Song
>  Sent: Friday, January 26, 2018 9:17 AM
>  To: libav-devel@libav.org
>  Subject: [libav-devel] [PATCH] hwcontext_qsv: Fix qsv hwupload
>  failure issue
> 
>  From: "Ruiling, Song" 
> 
>  1.) the initial_pool_size need to be set instead of zero.
>  2.) the PicStruct is required by MediaSDK, so give a default value.
> 
>  A simple command to reproduce the issue:
>  avconv -i INPUT -init_hw_device qsv=foo -filter_hw_device foo -vf
>  format=nv12,hwupload -c:v h264_qsv ... OUTPUT
> 
>  Signed-off-by: Ruiling Song 
>  ---
>   libavutil/hwcontext_qsv.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
>  diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c
>  index
>  9270b22..14f26c6 100644
>  --- a/libavutil/hwcontext_qsv.c
>  +++ b/libavutil/hwcontext_qsv.c
>  @@ -313,6 +313,7 @@ static int
> qsv_init_surface(AVHWFramesContext
>  *ctx,
>  mfxFrameSurface1 *surf)
>   surf->Info.CropH  = ctx->height;
>   surf->Info.FrameRateExtN  = 25;
>   surf->Info.FrameRateExtD  = 1;
>  +surf->Info.PicStruct  = MFX_PICSTRUCT_PROGRESSIVE;
> 
>   return 0;
>   }
>  @@ -325,8 +326,7 @@ static int qsv_init_pool(AVHWFramesContext
>  *ctx, uint32_t fourcc)
>   int i, ret = 0;
> 
>   if (ctx->initial_pool_size <= 0) {
>  -av_log(ctx, AV_LOG_ERROR, "QSV requires a fixed frame
> pool
>  size\n");
> >>>
> >>> Should keep this log message as a warning be better?
> >>>
>  -return AVERROR(EINVAL);
>  +ctx->initial_pool_size = 64;
> >>
> >> That doesn't really feel appropriate. If a fixed size pool is
> >> required, the user should simply be forced to specify a size. Making
> >> it use a random value doesn't make too much sense to me, and the
> >> required size is going to depend on the caller's use cases. In
> >> addition 64 by default sounds like a huge waste of memory.
> >
> > Thanks for your comment!
> > But I think it is better if we don't force the user to explicitly setup a 
> > value to
> get it work.
> > Less parameters means less burden for end-users. If this rule is not
> applicable here, please correct me.
> > I am not sure why a default workable value is not as good here. Could you
> share me more thought?
> > And there are more places that set default values to initial_pool_size:
> > Inside libavfilter/qsvvpp.c it also sets initial_pool_size to 64.
> > Inside avtools/avcov_qsv.c, it sets initial_pool_size to 32.
> > I am not sure do you have any comment on this? Will be 32 looks a little
> better?
> 
> IMO any fixed number is a problem.  The user can always break it by
> holding on to more frames - the lookahead option in the libmfx encoder is
> the easiest way to eat lots of frames, but there is nothing stopping the user
> just not giving the frames back for a long time.  The advantage of the
> extra_hw_frames option is that it actually codifies how many frames the
> user is allowed to hold, so that if they do hold more it is clear where the
> error is rather than the answer being "um, that doesn't work, sorry".
> 
> It is unclear what the default should be before that number is applied, but
> probably not something particularly large because it is very
> memory-wasteful if you have 64 surfaces and only ever use 2 (currently a
> common problem when chaining filters together - it's only the encoder
> which wants many).  Obviously that can all solved by some sort of magic
> autonegotiation, but noone has yet offered a plan of how that could work.
> 
> - Mark

I think it is a good idea to add the extra_hw_frames option to make it more 
flexible.
But I see default value is -1. IMHO it is equal to forcing user to set this 
option else it will fail. It becomes a burden for user and they have to know 
what's the exact number of extra_hw_frames should be set.
So I think it is a good idea to set a default value (though I also don't know 
why it is 64) when user hasn't set it.
Yes it will waste memory but IMHO it is not a big problem because it only takes 
effect when

Re: [libav-devel] [PATCH V2] lavc/qsvdec: expose frame pic_type and key_frame

2018-01-31 Thread Li, Zhong 
Sorry it is interrupted, I mean could you specify which hevc clip should be 
tested?

As your comment, maybe we need to find a HEVC clip with CRA frames to test.

From: Li, Zhong
Sent: Wednesday, January 31, 2018 4:11 PM
To: libav-devel@libav.org
Subject: RE: [libav-devel] [PATCH V2] lavc/qsvdec: expose frame pic_type and 
key_frame


> > @@ -416,6 +421,8 @@ FF_ENABLE_DEPRECATION_WARNINGS

> >  outsurf->Info.PicStruct & MFX_PICSTRUCT_FIELD_TFF;

> >  frame->interlaced_frame =

> >  !(outsurf->Info.PicStruct &

> MFX_PICSTRUCT_PROGRESSIVE);

> > +frame->pict_type =

> ff_qsv_map_pictype(out_frame->dec_info.FrameType);

> > +frame->key_frame = !!(out_frame->dec_info.FrameType &

> > + MFX_FRAMETYPE_IDR);

> >

> >  /* update the surface properties */

> >  if (avctx->pix_fmt == AV_PIX_FMT_QSV)

> >

>

> Does this do the right thing for H.265?  (Can't test right now.)



Testing should use a HEVC clips


___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH V2] lavc/qsvdec: expose frame pic_type and key_frame

2018-01-31 Thread Li, Zhong 
> > @@ -416,6 +421,8 @@ FF_ENABLE_DEPRECATION_WARNINGS

> >  outsurf->Info.PicStruct & MFX_PICSTRUCT_FIELD_TFF;

> >  frame->interlaced_frame =

> >  !(outsurf->Info.PicStruct &

> MFX_PICSTRUCT_PROGRESSIVE);

> > +frame->pict_type =

> ff_qsv_map_pictype(out_frame->dec_info.FrameType);

> > +frame->key_frame = !!(out_frame->dec_info.FrameType &

> > + MFX_FRAMETYPE_IDR);

> >

> >  /* update the surface properties */

> >  if (avctx->pix_fmt == AV_PIX_FMT_QSV)

> >

>

> Does this do the right thing for H.265?  (Can't test right now.)



Testing should use a HEVC clips



> I'm asking that because I know that frame types are still a problem for H.265

> encoding, because not all IRAPs are IDR - when CRA frames are generated

> they aren't marked as key.  The FrameType field (as used here) is not

> sufficient in that case, but I never sorted out exactly what does contain the

> necessary information.

>

> - Mark



Exactly, only IDR type is exposed from FrameType here, and it is set by parsing 
slice_header 
(https://github.com/Intel-Media-SDK/MediaSDK/blob/master/_studio/shared/umc/codec/h265_dec/src/umc_h265_bitstream_headers.cpp#L1428
 )

But the problem is (I am not familiar with HEVC as well as H264, so please 
correct me if I am wrong), should CRA frames be treated as key frames?

See the description as blew (source 
http://ieeexplore.ieee.org/document/6324417/?part=undefined%7Cfig4#fig4 )

[cid:image003.jpg@01D39AAE.23C1D950]



Here some leading pictures such as B30 can’t be decoded when random access from 
I28.

As my understanding, all frames behand (in display order) key frame should be 
decodable.

IDR it can make sure this, but CRA can’t. So at least not all of the CRA frames 
can be treated as key frames.

Looks some people agree with me (refer Jeanlf’s comment on 
https://github.com/gpac/gpac/issues/144)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel