[FFmpeg-devel] [PATCH] H264 parser fix

Michael Niedermayer michaelni
Wed May 26 00:26:27 CEST 2010


On Tue, May 25, 2010 at 09:31:51AM -0700, Howard Chu wrote:
> Michael Niedermayer wrote:
>> On Tue, May 25, 2010 at 07:29:11AM -0700, Howard Chu wrote:
>>> Michael Niedermayer wrote:
>>>> On Mon, May 24, 2010 at 11:25:05AM -0700, Howard Chu wrote:
>>>>> Michael Niedermayer wrote:
>>>>>> On Wed, May 19, 2010 at 01:15:17AM -0700, Howard Chu wrote:
>>>>>>> When decoding an H264 stream that uses global headers, you get an
>>>>>>> endless
>>>>>>> stream of "non-existing PPS referenced" messages. This message comes
>>>>>>> from
>>>>>>> h264_parser.c, and it happens because it's using an H264Context 
>>>>>>> hanging
>>>>>>> off
>>>>>>> the AVCodecParserContext, which is separate from the H264Context
>>>>>>> hanging
>>>>>>> off the AVCodecContext, and only the AVCodecContext contains the
>>>>>>> extradata
>>>>>>> that provides the PPS and SPS.
>>>>>>>
>>>>>>> This patch is admittedly a hack, but it stops the error. It just 
>>>>>>> checks
>>>>>>> in
>>>>>>> the AVCodecContext if the PPS wasn't found in the 
>>>>>>> AVCodecParserContext.
>>>>>>
>>>>>> ugly ugly ugly
>>>>>> if iam not mistaken this would be fixed by making the parser work with
>>>>>> mov/mkv ("avc") style h264
>>>>>
>>>>> I read thru mov.c and matroskadec.c and couldn't find anything 
>>>>> relevant,
>>>>> so
>>>>> I don't understand what your suggestion means.
>>>>
>>>> i thought that the problem is that the parser is feeded a format it does
>>>> not
>>>> support. I did not confirm this but i see no reason that this is a
>>>> different
>>>> issue.
>>>
>>> That is not the problem I was reporting. The parser has no trouble with 
>>> the
>>> data format. It is simply missing the PPS and SPS because they were not
>>> available in the AVCodecParserContext. The extradata is only available in
>>> the AVCodecContext.
>>>
>>>> The format difference should be easiest vissible by looking at code
>>>> surrounding is_avc in h264.c
>>>>
>>>> if this is the issue the parser should be improved to suport the slghtly
>>>> differnt way to pack things and also to look in extradata.
>>>
>>> Before we can tell the parser to look in the extradata, we have to give 
>>> the
>>> parser a way to access it, ideally when the parser is initialized. Right
>>> now there's no reliable way to do that.
>>
>> The parse function has the AVCodecContext extradata
>
>>> It seems odd to me that the parser
>>> entry points all require an AVCodecContext but av_parser_init() only 
>>> takes
>>> a codec_id. If av_parser_init() took an AVCodecContext instead, all of 
>>> the
>>> necessary information would be available.
>>
>> well, yes, its not given during init but just later, i dont see how this
>> would be a problem
>
> This patch works. It just means adding an if (extradata_not_parsed_yet) 
> test in h264_parse(), instead of handling it unconditionally at init time. 
> Probably a trivial overhead. I've just taken the extradata parsing code 
> from ff_h264_decode_init() and broken it out into its own function so that 
> h264_parse can use it too.
>
> -- 
>   -- Howard Chu
>   CTO, Symas Corp.           http://www.symas.com
>   Director, Highland Sun     http://highlandsun.com/hyc/
>   Chief Architect, OpenLDAP  http://www.openldap.org/project/

> Index: libavcodec/h264_parser.c
> ===================================================================
> --- libavcodec/h264_parser.c	(revision 23295)
> +++ libavcodec/h264_parser.c	(working copy)
> @@ -245,6 +245,12 @@
>      ParseContext *pc = &h->s.parse_context;
>      int next;
>  

> +    /* Should a failure here cause the entire parse to fail? */

no


> +    if (!h->sps_buffers[0] && avctx->extradata_size) {

that looks wrong, i dont think anything gurantees that there is a sps
with sps_id=0



> +        h->s.avctx = avctx;
> +        ff_h264_decode_extradata(avctx, h);
> +    }
> +
>      if(s->flags & PARSER_FLAG_COMPLETE_FRAMES){
>          next= buf_size;
>      }else{

> Index: libavcodec/h264.c
> ===================================================================
> --- libavcodec/h264.c	(revision 23295)
> +++ libavcodec/h264.c	(working copy)
> @@ -844,41 +844,9 @@
>      memset(h->pps.scaling_matrix8, 16, 2*64*sizeof(uint8_t));
>  }
>  
> -av_cold int ff_h264_decode_init(AVCodecContext *avctx){
> -    H264Context *h= avctx->priv_data;
> -    MpegEncContext * const s = &h->s;
> -
> -    MPV_decode_defaults(s);
> -
> -    s->avctx = avctx;
> -    common_init(h);
> -
> -    s->out_format = FMT_H264;
> -    s->workaround_bugs= avctx->workaround_bugs;
> -
> -    // set defaults
> -//    s->decode_mb= ff_h263_decode_mb;
> -    s->quarter_sample = 1;
> -    if(!avctx->has_b_frames)
> -    s->low_delay= 1;
> -
> -    avctx->chroma_sample_location = AVCHROMA_LOC_LEFT;
> -
> -    ff_h264_decode_init_vlc();
> -
> -    h->thread_context[0] = h;
> -    h->outputed_poc = INT_MIN;
> -    h->prev_poc_msb= 1<<16;
> -    h->x264_build = -1;
> -    ff_h264_reset_sei(h);
> -    if(avctx->codec_id == CODEC_ID_H264){
> -        if(avctx->ticks_per_frame == 1){
> -            s->avctx->time_base.den *=2;
> -        }
> -        avctx->ticks_per_frame = 2;
> -    }
> -
> -    if(avctx->extradata_size > 0 && avctx->extradata && *(char *)avctx->extradata == 1){
> +int ff_h264_decode_extradata(AVCodecContext *avctx, H264Context *h)

no need to pass AVCodecContext if its in h->s.avctx

also the factorization should be a seperate patch

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100526/ab04b93a/attachment.pgp>



More information about the ffmpeg-devel mailing list