Skip to content
This repository has been archived by the owner on May 17, 2023. It is now read-only.

MSDK decoder error handing for wrong level_idc clips. #582

Closed
lizhong1008 opened this issue Aug 29, 2018 · 15 comments
Closed

MSDK decoder error handing for wrong level_idc clips. #582

lizhong1008 opened this issue Aug 29, 2018 · 15 comments

Comments

@lizhong1008
Copy link
Contributor

Some clips declare a wrong level_idc, smaller than it should be. Then will trigger an error MFX_ERR_INCOMPATIBLE_VIDEO_PARAM and make decoding process interrupted. If using MSDK parser MFXVideoDECODE_DecodeHeader, this error won't happen. However, using MFXVideoDECODE_DecodeHeader is an optional, not must. So the error handling should be move to somewhere more common.
More details please refer Intel-FFmpeg-Plugin/Intel_FFmpeg_plugins#37

@dmitryermilov
Copy link
Contributor

DecodeHeader() is optional but in this case application takes responsibility of filling correct mfxVideoParam . If the issue is not seen using DecodeHeader it means that FFMPEG passed wrong mfxVideoParam to Init() function. The question: where are these param from ?
Decoder detects that initial params are different from params detected at run-time and returns MFX_ERR_INCOMPATIBLE_VIDEO_PARAM which is not a critical error. According to MediaSDK manual in case of MFX_ERR_INCOMPATIBLE_VIDEO_PARAM, application should retrieve buffered frames, obtain new mfxVideoParam by DecodeHeader() and call Reset.

@dvrogozh
Copy link
Contributor

@dmitry-gurulev : please, participate in the discussion.

Can someone, please:

  1. Remind me whether DecodeHeader() moves mfxBitstream::DataOffset pointer?
  2. If answer on 1) is "yes" and hence DecodeHeader skips the header, then, please, clarify whether DecodeFrameAsync() will tolerate if user will ignore DataOffset adjustment by DecodeHeader and feed DecodeFrameAsync with the header?

My thoughts around this issue are the following:

  1. If application parsed the header on its own, then initialized decoder and did not pass the header in DecodeFrameAsync then the application may actually be fully responsible for the decoding failure since decoder may not be able to detect correct parameters due to missed header
  2. If application parsed the header, initialized decoder and honestly fed decoder with the bitstream containing the header, but received an error, then this looks to be mediasdk issue since mediasdk either failed to accept/recognize the header or failed to detect correct settings.

@lizhong1008
Copy link
Contributor Author

@dmitryermilov ,there is nothing wrong that ffmpeg passing to init(), the parameters are parsed by ffmpeg software decoder. The root cause is the input clip hasn't strictly following H264 spec: the level_idc is conflict with other parameters: conflict with resolution in Intel-FFmpeg-Plugin/Intel_FFmpeg_plugins#37 and conflict with reference number in Intel-FFmpeg-Plugin/Intel_FFmpeg_plugins#32.
Looks like MFXVideoDECODE_DecodeHeader can detect such conflict and do something error handling, so my suggestion is to move such error handling to another common place to make sure even MFXVideoDECODE_DecodeHeader hasn't been called, these clips still can be decode-able, just like ffmpeg software decoder.

@dgurulev
Copy link

@dvrogozh, DecodeHeader sets mfxBitstream::DataOffset to first valid SPS it found in the bitstream. DecodeFrameAsync will work in both cases - if user bitstream starts form header or doesn't.

If application parsed the header, initialized decoder and honestly fed decoder...
In that certain case decoder is initialized w/ level 1.0, but accord. to SPS level should be at least 2.1. That's why decoder claims re-initialization. In the same time, MFX decoder header adjusts the level accord. to SPS actual data and when decoder is initialized it is capable to decode given stream. Such level adjustment was implemented as WA because of many streams have the same issue. If app. doesn't use MFX decoder header it is responsible to WA such corner cases itself.

@dmitryermilov
Copy link
Contributor

I fully agree with Dmitry G. reply

@dvrogozh
Copy link
Contributor

@dmitry-gurulev : I don't fully understand your answer. This in particular: "In that certain case decoder is initialized w/ level 1.0, but accord. to SPS level should be at least 2.1." Did you intent to answer that SPS contains incorrect level 1.0, but there are other parameters in the same SPS which signify that level should be at least 2.1?

@dgurulev
Copy link

@dvrogozh,

SPS contains incorrect level 1.0, but there are other parameters in the same SPS which signify that level should be at least 2.1?

Yes, that's exactly what I mean.

@dvrogozh
Copy link
Contributor

@lizhong1008 : please, take a look into the following description and provide your comments:

  • ffmpeg-qsv initialized the mediadk decoder with some parameters obtained from the self-parsing of the bitstream. Initialization was successful, mediasdk allocated internal resources to process the bitstream according with the initialized parameters.
  • ffmpeg-qsv started to feed mediasdk decoder with the data. mediasdk detected that parameters in the bitstream are actually different from those used during initialization and mediasdk can't process this bitstream without re-initialization of internal resources.
  • Per mediasdk architecture and design resources re-initialization at runtime are prohibited. As a result mediasdk signaled to the application that reset or re-initialization is required. This is done via error -14=MFX_ERR_INCOMPATIBLE_VIDEO_PARAM.
  • ffmpeg-qsv did not handle -14 and return error to the upper layer

So, the key point is that mediadk:

  1. Signals via -14 error that decoding can't be continued without decoder reset or reinitialization with correct parameters. (This is a typical situation, for example, that's the case when you have 2 streams concatenated and resolution change should be handled.)
  2. To get correct valid parameters mediasdk provides an uAPI: DecodeHeader

So, in a way ffmpeg-qsv bites its own tail: library provides you a way to detect correct settings, but you neither don't use this api nor implement an alternative.

Question: what is motivation behind avoiding to use DecodeHeader? Could you, please, provide explanation and/or implement this support in the ffmpeg-qsv?

@lizhong1008
Copy link
Contributor Author

lizhong1008 commented Sep 4, 2018

@dvrogozh , resolution change has been handled in ffmpeg: https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/qsvdec.c#L523 . For other cases need to be reinit, let us create a new discussion thread.
Here let's just talking the level_idc issue: "SPS contains incorrect level but there are other parameters in the same SPS which signify that level should be larger". For these cases, MSDK just give an error of MFX_ERR_INCOMPATIBLE_VIDEO_PARAM, but haven't indicate application which parameters are wrong. Is there anything I can get from MSDK than the correct level is 2.1 without calling DecodeHeader? If no, FFmpeg can't know how to re-init it.

Then, Let's talk MFX DecodeHeader:

  1. MFX DecoderHeader is not must as MSDK documentation, it is an option. so MSDK should make sure same quality with or without using DecodeHeader. As @dmitry-gurulev said, many streams have the same issue. But this is not something WA, it is decoding error handling thus all decoders should do. Almost all decoder can decode such clips successfully as I know. If you just do this in DecoderHeader and declare this API is not must just optional, even FFmpeg can WA it, you still can't make sure other application can treat this issue well. This is not a good new for MSDK developers.
  2. FFmpeg parser: av_parser_parse2() is the standard/recommended interface of ffmpeg. See doc/examples/decode_video.c in FFmpeg. And It is much simpler than DecodeHeader, without decoding all SPS/PPS/Slice headers but provide enough information to init MSDK decoder.
  3. This error handling can't be done in FFmpeg parser, since it won't decoding all sps/pps information and then can't detect such level conflict.
  4. Though DecodeHeader can avoid this level_idc issue, but using av_parser_parse2 is benefit to improve some AVC/HEVC conformance passrate IMHO. Yes, these conformance issues maybe fixed in DecodeHeader too, but at least FFmpeg parser is robust enough IMHO.

### My question: Is there any technology difficulty to move such error handing implementation from DecodeHeader to somewhere else?

@dvrogozh
Copy link
Contributor

dvrogozh commented Sep 4, 2018

@lizhong1008 : the mediasdk uAPI to get correct parameters to continue decoding is DecodeHeader. And I honestly do not see any reason to define another uAPI for the same purpose if there is the one already. As of now, if application don't want to use this uAPI, it just restricts supported use cases to those which has completely correct bitstreams.

Is there any technology difficulty to move such error handing implementation from DecodeHeader to somewhere else?

I don't see this as a technology question of what can and can not be done. We talk about a software, a lot of things can be done. To me that's a question of how uAPI is defined and what is and is not permitted within uAPI calls. For example, if decoder was initialized to handle X (i.e. it allocated required resources, etc.), it can not dynamically reinitialize DecodeFrameAsync to handle Y (i.e. reallocate resources). That's why -14 was returned from DecodeFrameAsync and this is correct. If you meet such situation, you have one of the 2 choices:
1, Either you declare such case as not supported
2. Or you support DecodeHeader to get correct parameters
While the above describes situation in general, in our particular case with level_idc my question @dmitry-gurulev is: why decoder Init() did not fix parameters if it saw (or could see) them being wrong?

Yes, these conformance issues maybe fixed in DecodeHeader too, but at least FFmpeg parser is robust enough IMHO.

You do not have a strict separation in ffmpeg on the areas of responsibility. You can use ffmpeg parser, but you still pass same bitstream to mediasdk. If the situation was that ffmpeg parser worked, processed part of the bitstream and passed remained part to mediasdk that would be the case of your concerns that mediasdk don't support something, but you have other situation... As a result selective approach of what you want mediasdk to do and what you don't is quite strange.

@AlekseiBaygulov
Copy link

### My question: Is there any technology difficulty to move such error handing implementation from DecodeHeader to somewhere else?

Common issue with incorrect level_idc is num_ref_frames more than calculated DPB size (DPB depends on level_idc). DecodeHeader can increase level_idc after parsing sps header. Other functions (Init/Query/QueryIOSurf) know nothing about sps and num_ref_frames and can't do error handling.
And it is too late to do something at DecodeFrameAsync because number of allocated surfaces is small for new DPB size.

@lizhong1008
Copy link
Contributor Author

Finally I agree to replace current parser with MFXVideoDECODE_DecodeHeader() for ffmpeg-qsv. And patch has been sent: https://patchwork.ffmpeg.org/patch/11809/.
Thanks for all your comments. I will close this issue once patch merged.

@dmitryermilov
Copy link
Contributor

Hi @lizhong1008 ,

Was the ffmpeg change merged?

@lizhong1008
Copy link
Contributor Author

@dmitryermilov not yet, community still would like to use ffmpeg software parser.

@lizhong1008
Copy link
Contributor Author

@dmitryermilov , ffmpeg change merged, so it can be closed now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants