[MPlayer-dev-eng] [PATCH] AVParser for audio support

[Uoti Urpala]

On Wed, 2009-08-26 at 22:09 +0200, Reimar Döffinger wrote:
> On Wed, Aug 26, 2009 at 10:52:48PM +0300, Uoti Urpala wrote:
> > How about doing the
> > parsing in ad_ffmpeg? Is there a reason to expect a practical benefit
> > from doing it elsewhere?
> 
> Well, what is the benefit of doing it in ad_ffmpeg? My incomplete attempt
> looked at least as bad and won't even work with mencoder an -oac copy.

At least it should avoid the "if (ds == ds->demuxer->audio)" testing and
creation of the parser in the middle of a ds_add_packet() call, plus
allow sane(r) handling of any possible error cases. The
av_parser_parse2() documentation also mentions some flush-at-EOF
functionality and that cannot really be supported at the packet add
side.

I'm not sure whether always forcing modification of the audio track by
parsing when using mencoder -oac copy is an improvement or not.


> > Even if it's needed for multiple uses either
> > adding a separate parsing function or doing parsing in demuxer.c
> > functions when taking packets _out of_ the queue (so the information
> > from the demuxer is kept intact in the queue) could be preferable.
> 
> On the other hand the information before parsing is unlikely to be really
> too correct (particularly pts/dts). In case of AC3 in AVI there is also the
> annoyance that packets seem to vary greatly in size (and thus duration)
> before parsing.

Why is that an annoyance? BTW in that case it's probably better to only
set the pts for the first parsed packed corresponding to a container
packet and leave it unset for the rest (instead of creating multiple
packets with the same pts like your current patch does).


> > The new version you just posted fixes the pos/pts field loss that
> > happened due to creating new packets, but doing special-case copies of
> > those fields is still ugly.
> 
> There is a clone_packet function or something like that already, but it
> seems it can't use a different size. Maybe there is a function that
> does the right thing, if not it should probably be added.
> The patch has not really reached the "make the code look nice" stage,
> though I'd of course appreciate if someone else does that part.

Doing the parsing in ad_ffmpeg would avoid the need to attempt
implementing a general-case one-to-many packet duplication function.

Your current patch also creates bogus 0-size packets when the parser
reads the end of a container packet without outputting anything.