[FFmpeg-soc] [soc]: r2021 - in eac3:?ac3dec.h?ac3dec_data.c?ac3dec_data.h eac3dec.c

[Michael Niedermayer]

On Wed, Mar 26, 2008 at 01:33:28PM +0100, Bartlomiej Wolowiec wrote:
> This is last part of patch written earlier:
> 
> > Here is my idea of changing ac3 parser, so that it will send in one package 
> > to decoder all frames concerning one part of time.
> > It works like that: it buffers all frames until next independent frame.
> > The problem is that the last frame never leaves the buffer. Any ideas how to
> > solve it?

first, after reviewing our ac3 parser i must say i do not like it as it
always copies the data even if completely unneeded. Our mp3 parser while
maybe being a mess does not. This is not strictly related to the patch.

But i will not tolerate any further unneeded copying being added, even
less so if it adds a 1 packet delay (for normal AC3/AAC).


[...]
> Index: libavcodec/aac_ac3_parser.c
> ===================================================================
> --- libavcodec/aac_ac3_parser.c	(wersja 12599)
> +++ libavcodec/aac_ac3_parser.c	(kopia robocza)
> @@ -23,6 +23,7 @@
>  #include "parser.h"
>  #include "aac_ac3_parser.h"
>  
> +// TODO check s->inbuf_tab size
>  int ff_aac_ac3_parse(AVCodecParserContext *s1,
>                       AVCodecContext *avctx,
>                       const uint8_t **poutbuf, int *poutbuf_size,
> @@ -36,6 +37,13 @@
>      *poutbuf_size = 0;
>  
>      buf_ptr = buf;
> +
> +    if(s->inbuf_ptr < s->inbuf) {
> +        //after sending package of data in the end there was one new header
> +        memcpy(s->inbuf_tab, s->inbuf, s->header_size);
> +        s->inbuf = s->inbuf_tab;
> +        s->inbuf_ptr = s->inbuf_tab + s->header_size;
> +    }

The if condition just looks invalid. I know what you are doing here but
at least with the current variable names this makes no sense to a reader at
all.
Also i doubt renaming inbuf is the cleanest solution ...


>      while (buf_size > 0) {
>          int size_needed= s->frame_size ? s->frame_size : s->header_size;
>          len = s->inbuf_ptr - s->inbuf;

> @@ -56,7 +64,14 @@
>                      memmove(s->inbuf, s->inbuf + 1, s->header_size - 1);
>                      s->inbuf_ptr--;
>                  } else {
> -                    s->frame_size = len;
> +                    if(!s->stream_type) {
> +                        if(s->inbuf != s->inbuf_tab) {
> +                            *poutbuf = s->inbuf_tab;
> +                            *poutbuf_size = s->inbuf - s->inbuf_tab;
> +                            s->inbuf_ptr = s->inbuf_tab;
> +                            s->frame_size = 0;
> +                            break;
> +                        }

this is VERY hackish, stream_type has from the point of view of a
generic (AAC+AC3+EAC3) parser no relation to the number of frames over
which channels are split.


>                      /* update codec info */
>                      avctx->sample_rate = s->sample_rate;
>                      /* allow downmixing to stereo (or mono for AC3) */
> @@ -71,15 +86,18 @@
>                      }
>                      avctx->bit_rate = s->bit_rate;
>                      avctx->frame_size = s->samples;
> +                    } else {
> +                        //TODO update bit_rate
> +                        avctx->frame_size += s->samples;
> +                    }
> +                    s->frame_size = len;
>                  }
>              }
>          } else {

>              if(s->inbuf_ptr - s->inbuf == s->frame_size){
> -                *poutbuf = s->inbuf;
> -                *poutbuf_size = s->frame_size;
> +                s->inbuf += s->frame_size;
>                  s->inbuf_ptr = s->inbuf;
>                  s->frame_size = 0;
> -                break;
>              }

this causes additional copying and delay i think for normal AC3 & AAC

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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-soc/attachments/20080326/38d744ec/attachment.pgp>