[FFmpeg-cvslog] r12759 - in trunk/libavcodec: aac_ac3_parser.c?aac_ac3_parser.h aac_parser.c ac3_parser.c

[Michael Niedermayer]

On Sat, Apr 12, 2008 at 12:27:49AM +0200, Bartlomiej Wolowiec wrote:
> On ?roda, 9 kwietnia 2008, Michael Niedermayer wrote:
> > I suspect that the best course of action is to revert the changes up to the
> > point where the regression tests pass again.
> > Next the rm demuxer must be fixed which will likely change the regression
> > checksums, which need to be updated after confirming that the rm demuxer
> > works equally well or better.
> > And last the parser should be commited again. (but of course only if
> > regressions pass, if they dont the bugs causing the failure must be found
> > and fixed similar to the rm demxuer _before_ commiting the parser)
> >
> > [...]
> 
> This is patch version with all corrections. But if after ac3 frames are eac3 
> frames, I don't know what to do to have seektest passed?
> 
> tests/data/a-ac3.rm
> [...]
> -ret: 0 st: 0 dts:0.000000 pts:0.000000 pos:271 size:556 flags:1
> +ret: 0 st: 0 dts:0.000000 pts:0.000000 pos:839 size:556 flags:1
>  ret: 0 st:-1 ts:2.576668 flags:0
> -ret: 0 st: 0 dts:2.960000 pts:2.960000 pos:48659 size:558 flags:1
> +ret: 0 st: 0 dts:2.960000 pts:2.960000 pos:49229 size:558 flags:1
> [...]
> 
> tests/data/b-libav.rm:
> [...]
> -ret: 0 st: 1 dts:0.975000 pts:0.975000 pos:355101 size:278 flags:1
> +ret: 0 st: 1 dts:0.975000 pts:0.975000 pos:-1 size:278 flags:1
>  ret: 0 st: 1 ts:1.471000 flags:1
> -ret: 0 st: 1 dts:0.975000 pts:0.975000 pos:355101 size:278 flags:1
> +ret: 0 st: 1 dts:0.975000 pts:0.975000 pos:-1 size:278 flags:1

As you can see the pos value is wrong after your patch, i suspect it
might need to be reordered similar to pts/dts in av_parser_parse()


> [...]
> 
> -- 
> Bartlomiej Wolowiec

> Index: libavcodec/aac_ac3_parser.c
> ===================================================================
> --- libavcodec/aac_ac3_parser.c	(wersja 12790)
> +++ libavcodec/aac_ac3_parser.c	(kopia robocza)
> @@ -29,60 +29,46 @@
>                       const uint8_t *buf, int buf_size)
>  {
>      AACAC3ParseContext *s = s1->priv_data;
> -    AACAC3FrameFlag frame_flag;
> -    const uint8_t *buf_ptr;
> -    int len;
> +    ParseContext *pc = &s->pc;
> +    int len, i;
>  
> -    *poutbuf = NULL;
> -    *poutbuf_size = 0;
> +    while(s->remaining_size <= buf_size){

> +        if(s->remaining_size && !s->need_next_header || !buf_size){
> +            i= s->remaining_size;
> +            /* If EOF set remaining_size>0, to finish correctly. */

> +            s->remaining_size = !buf_size;

this is VERY ugly

you still ignore the return of ff_combine_frame() you just skip the
correct call, and luckily even with wrong arguments ff_combine_frame() works
halfway. But then your hack requires another hack, namely setting 
remaining_size to 1 so the previous hack doesnt trigger again
this is completely unacceptable.


[...]
> +static int aac_parse_header(const uint8_t *buf, int buf_size,
> +        AACAC3ParseContext *hdr_info, AVCodecContext *avctx){
> +
> +    /* allow downmixing to stereo */
> +    if(avctx->request_channels > 0 &&
> +            avctx->request_channels < hdr_info->channels &&
> +            avctx->request_channels == 2){
> +        avctx->channels = avctx->request_channels;
> +    } else {
> +        avctx->channels = hdr_info->channels;
> +    }
> +    avctx->sample_rate = hdr_info->sample_rate;
> +    avctx->bit_rate = hdr_info->bit_rate;
> +    avctx->frame_size = hdr_info->samples;
> +    return 0;
> +}
[...]

> +static int ac3_parse_header(const uint8_t *buf, int buf_size,
> +        AACAC3ParseContext *hdr_info, AVCodecContext *avctx){
> +
> +    /* allow downmixing to mono or stereo */
> +    if(avctx->request_channels > 0 &&
> +            avctx->request_channels < hdr_info->channels &&
> +            avctx->request_channels <= 2){
> +        avctx->channels = avctx->request_channels;
> +    } else {
> +        avctx->channels = hdr_info->channels;
> +    }
> +    avctx->sample_rate = hdr_info->sample_rate;
> +    avctx->bit_rate = hdr_info->bit_rate;
> +    avctx->frame_size = hdr_info->samples;
> +    return 0;
> +}

duplicate
and this belongs in a seperate patch

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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-cvslog/attachments/20080412/57cfa274/attachment.pgp>