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