Hi!
Wolfram Gloger wrote:
> I still get "false hits" from streamdata.cpp:mpaframe() for audio
> streams where MPEG audio frames aren't aligned to packet boundaries.
>
> The reason is that mpaframe() simply looks for a possible MPA header,
> and then looks for another MPA header at an offset of the framesize
> _or anywhere later_. So the following can happen:
>
> Buffer Offset Content
> ===========================================================================
> x MPA frame start
> ...
> x+120 Packet boundary ("Item" points here)
> ...
> x+129 Invalid MPA (is_mpa() returns true but it's
> not really the start of a frame)
> ...
> x+576 MPA frame start
> ...
> x+2*576 MPA frame start
> ...
>
> mpaframe() in this case will happily detect an MPA frame between x+129
> and x+2*576. The correct frame, however, is between x+576 and
> x+2*576. With the current code, this means that the frame starting at
> x+576 is missed.
Yep. That's why there is the "sync" code. After a while, the program
should "lock" to the correct position automatically.
> The attached patch fixes this by only considering MPA headers which
> are _exactly_ one frame size apart. (All other MPEG parsers that I
> know do this also.) It also fixes a potential buffer overflow in
> ac3frame().
That will work fine in a perfect world where no transmission errors occur.
I noticed that you drop out of the loop in audio_addpts() (line 182 in
your version) when framesize == 0. Doesn't that mean that a single
missing frame header will stop audio pts processing prematurely?
> I changed the interface for mpaframe() and ac3frame() so they return
> the start _and_ size of valid audio frames. This will be needed in
> another patch which I will submit soon.
>
> BTW the "needsync" code in audio_addpts() IMHO does absolutely nothing
> and should be removed. All "items" which have no pts are removed at
> the start of the function, and only items which do have pts are added.
> So, the following loop always terminates immediately:
>
> if (needsync) {
> needsync=false;
>
> for(;it!=items.end();++it)
> if (it->headerhaspts())
> // header carries PTS
> break;
I agree that the for loop seems to terminate immediately and should be
removed. But what about the rest of the code? It used to be executed
whenever there was a frame synchronization problem. With your patch,
it's only executed once at the beginning of the outer (while) loop
because you deleted the assignment "needsync=true" later on (line 182 in
revision 35) and decided to leave the loop instead.
Besides that, your code will never return the length of the last frame
that begins in the buffer because {ac3,mpa}frame() can't access the
following frame header. But what happens if the frames are aligned?
I'm afraid you opened a big can of worms here.
--
Michael "Tired" Riepe <[EMAIL PROTECTED]>
X-Tired: Each morning I get up I die a little
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
DVBCUT-user mailing list
DVBCUT-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dvbcut-user
