[FFmpeg-devel] AC3 Format Detection unreliable.

[Michael Niedermayer]

Hi

On Sat, Aug 04, 2007 at 11:32:21PM -0400, Justin Ruggles wrote:
> Michael Niedermayer wrote:
> > Hi
> > 
> > On Sat, Aug 04, 2007 at 08:33:03PM -0400, Justin Ruggles wrote:
> >> Justin Ruggles wrote:
> >>> Dan Sully wrote:
> >>>> * Dan Sully shaped the electrons to say...
> >>>>
> >>>>>> Yes, please make the stream(s) available.
> >>>> Also, I am on an amd64 system.
> >>> Doesn't look like that's an issue.  I'm getting the same thing on my 
> >>> system.  The file is valid.  The issue seems to be the frame size. 
> >>> 640kbps 48kHz is the largest AC3 frame size, and I think it's not 
> >>> fitting enough frames into the probe buffer to give a positive 
> >>> detection.  I'll look into it.
> >> The problem with this sample is two-fold.  The first issue is that the 
> >> probe buffer in this case is only 2048 bytes.  The frame size for this 
> >> sample (and any others which are 640kbps & 48kHz) is 2560.  Therefore, 
> >> only 1 frame is detected.  I changed the threshold for a score of 50 
> >> down to 2 frames, but this doesn't help here.  After I changed to 1 
> >> frame (at the start of the stream) it now gets a score of 50, but then a 
> >> 2nd issue occurs.
> > 
> > so you havent fixed the issue but you sucessfully broke the whole probe
> > system in lavf, great
> > 
> > the ff_ac3_parse_header function checks only something like 18bits
> > a single match at the start IS NOT enough for a score of 50%
> > mpeg-ps checks for somthing like 60bits and does misdetect it so
> > how many misdetections do you think you added by decreasing the
> > bits for ac3 to 18 at the start or 36 at a random pos?
> > 
> > also keep in mind that if no detection happens in 2048 bytes lavf will
> > increase the buffer size until a reasonable detection is achived!
> > 
> > so to summarize do NOT "fix" bugs if you do NOT FULLY understand why
> > it doesnt work and how the code works, this comment is not intended
> > to you but rather to alot of people here
> > when i think of pts/dts issues old ogg-vorbis, flv, swf, ... patches
> > which where applied these all added fantastically broken, huge, stupid and
> > unneeded code to workaround bugs in some other part of the code
> 
> You're right, I apologize.  I should have tested it more with other 
> formats before relaxing the detection criteria.  I'll change it back and 
> try to find another way.
> 
> 2048 bytes is enough for the 1st 5/8 of any AC3 frame, so the 1st CRC 
> code could be used to detect a single frame with pretty close to 100% 
> certainty.  Then you have the issue of AC3 inside a container.  But 
> would it be okay to give a 50% score if the frame is at the start of the 
> probe buffer and is a CRC match?

maybe but why not require more than 1 frame?
if you have any ac3 files which dont have 2 consecutive frames then i
would rather investigate why instead of trying to detect a single frame
ac3
that said checking the CRC too is definitly a good idea

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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-devel/attachments/20070805/0ff1ab3c/attachment.pgp>