[FFmpeg-devel] [PATCH] close parser in av_find_stream_info is parsing is not needed

[Michael Niedermayer]

On Tue, May 18, 2010 at 06:51:05AM -0700, Baptiste Coudurier wrote:
> On 5/18/10 5:58 AM, Michael Niedermayer wrote:
>> On Mon, May 17, 2010 at 01:17:42PM -0700, Baptiste Coudurier wrote:
>>> Hey guys,
>>>
>>> $subject.
>>>
>>> The parser is opened here:
>>>
>>>          //only for the split stuff
>>>          if (!st->parser&&  !(ic->flags&  AVFMT_FLAG_NOPARSE)) {
>>>              st->parser = av_parser_init(st->codec->codec_id);
>>>              if(st->need_parsing == AVSTREAM_PARSE_HEADERS&&  
>>> st->parser){
>>>                  st->parser->flags |= PARSER_FLAG_COMPLETE_FRAMES;
>>>              }
>>>          }
>>>
>>> So either, it must be closed after, or it shouldn't be opened if parsing 
>>> is
>>> not needed. I'm not sure of the side effects.
>>
>> Parsers that have a AVCodecParser.split() must be opened and must stay 
>> open
>> until they have extracted the global headers into extradata
>> after that they could be closed if they arent needed for any other reasons
>>
>> There are several problems with this though, for example streams that 
>> start
>> later (after av_find_stream_info()) might not have a parser enabled even 
>> if
>> they need one for split().
>>
>> also if we close them before they have set extradata that similarly would
>> be a problem
>
> IMHO this is an ugly hack anyway, extradata should be extracted with a 
> bitstream filter. This has proven to produce side effects.

The sideeffects where due to av_parser_change() and the muxer API namely
AVFMT_GLOBALHEADER being not expressive enough.
mpeg2 and mpeg4 in mov would have needed different AVFMT_GLOBALHEADER but
as there is just one flag per muxer one had to blow up.

We could implement the extraction with bitstream filters or anything else
but it would not affect the side effect we had unless i misremember.
Also whichever way does the extraction, must do so before actual decoding
that is we need to search the stream for the headers and then go back and
only once we have them start decoding from the begin.
This would implicate that it has to stay in av_find_stream_info() or another
thing run prior to actual decoding/demuxing.
The demuxing case results from us needing extradata at the begin for writing
the header for the -codec copy case.


>
> Anyway, in ffmpeg.c:
>                     int ticks= ist->st->parser ? 
> ist->st->parser->repeat_pict+1 : ist->st->codec->ticks_per_frame;
>
> This is no good for H264 in mkv, since parser is set but does not set 
> repeat_pict. What do suggest ?

definitly, fix the parser
even if we later decide to change or remove the code from ffmpeg.c we
still need the parser to handle the mkv/mov case, i remember some bugreport
related to the parser not supporting that, but doent remember what exactly
it was about


> Using repeat_pict like this outside libavformat looks a bit weird to me 
> anyway.

true
for the case with decoding we could use AVFrame.repeat_pict probably
a patch changing this would be welcome if it passes fate

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20100518/7174f625/attachment.pgp>