darealshinji / delaycut

cuts and corrects delay in ac3 and dts files
GNU General Public License v3.0
33 stars 4 forks source link

Wrong ac3 file detection #4

Closed tebasuna51 closed 7 years ago

tebasuna51 commented 7 years ago

Only loading a test ac3 file we can see differences between DelayCut versions:

= INPUT FILE INFO =================
                   v1.3.0  CORRECT   v1.4.3.7          v1.4.3.8
                   ----------------  ----------------  ------------------
File is            ac3               ac3               ac3
Bitrate  (kbit/s)  192               192               448            (?)
Act rate (kbit/s)  192.000           192.000           448.000        (?)
File size (bytes)  480000            480000            480000
Channels mode      2/0: L+R          2/0: L+R          2/2: L+R+SL+SR (?)
Sampling Frec      48000             48000             48000
Low Frec Effects   LFE: Not present  LFE: Not present  LFE: Present   (?)
Duration           00:00:20.000      00:00:20.000      00:00:08.571   (?)
Frame length (ms)  32.000000         32.000000         32.000000
Frames/second      31.250000         31.250000         31.250000
Num of frames      625               625               267            (?)
Bytes per Frame    768.0000          768.0000          1792.0000      (?)
Size % Framesize   0                 0                 201            (?)
CRC present:       YES               YES               YES
= TARGET FILE INFO ================  =============     ==============
Start Frame        0                 0                 0
End Frame          624               624               266            (?)
Num of Frames      625               625               267            (?)
Duration           00:00:20.000      00:00:19.968 (?)  00:00:08.512   (?)

v1.4.3.7 is buggy because do a wrong calculation of target file duration. Then always than want a cut output one more frame than needed, if I want 3200 ms (100 frames) output 101 frames.

But v1.4.3.8 detect wrong Bitrate, Channels, LFE, etc.

tebasuna51 commented 7 years ago

The problem is in delayac3.cpp, Delayac3::getac3info, lines 1744+:

    syncword=  getbits (16, caracter);
//  crc1=      getbits (16, caracter);
    fscod=     getbits (2,  caracter);
    frmsizecod=getbits (6,  caracter);
//  bsid=      getbits (5,  caracter);
    bsmod=     getbits (3,  caracter);
    acmod=     getbits (3,  caracter);
//  if ((acmod & 0x01) && (acmod != 0x01)) cmixlev=getbits(2, caracter);
//  if (acmod & 0x4)  surmixlev=getbits(2, caracter);
//  if (acmod == 0x2) dsurmod=getbits(2, caracter);
    lfeon=     getbits(1, caracter);
//  dialnorm=  getbits(5, caracter);
//  compre=    getbits(1, caracter);
//  if (compre) compr=getbits(8, caracter);

Even if you don't need some fields in BSI header data, you can't comment the lines than read that data because the next active getbits() read wrong data.

Same problem in Delayac3::getdtsinfo and Delayac3::geteac3info

The problem in v1.4.3.7, also present in v1.4.3.8, is in delayac3.cpp line 2428:

-    estimatedTimeLength = (qint64)((endFrame - startFrame) * frameDuration);
 +   estimatedTimeLength = (qint64)((endFrame - startFrame + 1) * frameDuration);
darealshinji commented 7 years ago

I see. Can you make a pull request?

tebasuna51 commented 7 years ago

First I need learn how compile these sources, for windows, to test changes. By the moment I only can check sources to see problems.

darealshinji commented 7 years ago

Here are test builds for Windows: delaycut_testbuild_win32.zip Can you check them and see if the issues are fixed?

As for building from source, I'm using MXE to cross-compile them on Linux. On Windows you could try to use MSYS2 or Cygwin and install Qt using their bundled package managers. I don't know how to build with MSVC as I've never used that compiler.

The problem in v1.4.3.7, also present in v1.4.3.8, is in delayac3.cpp line 2428:

-    estimatedTimeLength = (qint64)((endFrame - startFrame) * frameDuration);
 +   estimatedTimeLength = (qint64)((endFrame - startFrame + 1) * frameDuration);

The + 1 is not present in the source: https://github.com/darealshinji/delaycut/blob/v1.4.3.8/src/delayac3.cpp#L2428

tebasuna51 commented 7 years ago

Thanks. Now work better.

I was checking DelayCut v1.4 for use it in command line. Then I'm not interested in compile it with Qt (is very dificult to me). I can upload a corrected (I hope) delayac3.cpp. Changes:

1) Old line (v1.4.3.7)

Yep, the +1 is not in v1.4.3.7 but must be corrected, is a v1.4.3.7 bug. The TimeLength include startFrame and endFrame, the difference is one less frame.

2) Uncomment all required reads in headers for getdtsinfo, getac3info and geteac3info

3) Support for any DTS samplerate and bitrate (new conditions for p_silence). Correct calculation of dactrate (real bitrate of any DTS). With: quint32 BytesPerFrame, nblks, fsample; qreal dactrate; I'm not sure if the new line 1989 is correct (real and int operations): dactrate=((qreal)(fsample * BytesPerFrame))/(4000.0 * (nblks+1));

delayac3_new.zip

darealshinji commented 7 years ago

Here's a new build with your fixes: delaycut_testbuild.zip And here's the branch with your fixes: https://github.com/darealshinji/delaycut/tree/ac3-patch

tebasuna51 commented 7 years ago

Thanks, near perfect for me. Now support DTS files 44100 Hz (from surround CDA), with other framelength than 2013, is common the framelength 2012 (exact bitrate 1509.00), etc.

But still there are a bug from v1.4.3.7 when calculate the endFrame with a endCut (same problem than before, one more frame is selected), I added the second line:

        endFrame = round((-endDelay + round(endCut * 1000.0)) / frameDuration);
        if (endFrame > startFrame) endFrame = endFrame - 1;

delayac3_new.zip

darealshinji commented 7 years ago
        endFrame = round((-endDelay + round(endCut * 1000.0)) / frameDuration);
        if (endFrame > startFrame) endFrame = endFrame - 1;

Patch applied: delaycut_testbuild.zip

tebasuna51 commented 7 years ago

Solved the Target EndFrame calculation,

But I make many test and I found other errors (even in v1.4.3.7) than make v1.4 unusable for me. Maybe is a Windows only problem (I'm using W 7 and tested 32 and 64 bits versions without differences). Sorry, but I can't help to solve the errors detected.

Some documentation about my test: testbuild_win32_0 with your changes testbuild_patch_1 with my first patch testbuild_patch_2 with my second patch

Problem 1: wrong source Info Problem 2: wrong target Duration (and full DTS support) Problem 3: wrong target EndFrame Problem 4: Destination Output File Name ignored (always output default INPUT_fixed) Problem 5: wrong CRC calculation (log file with error in all frames, see Doom9 forum) Problem 6: Sometimes (I can reproduce it) show a unexpected (without sense, see image) ERROR dialog, but continue to OK Problem 7: APPCRASH after a time doing nothing (see image)

DelayCut  Source   Target   Full DTS   Target    CRC     Output  ERROR
Version    Info   Duration  support   EndFrame  error  name edit dialog APPCRASH
--------  ------  --------  -------   --------  -----    ------  -----  --------
v1.3.0.0    OK      OK        NO        OK       OK       OK      OK      OK
v1.4.3.7    OK     wrong      NO       wrong  sometimes  wrong    yes     yes
v1.4.3.8   wrong   wrong      NO       wrong  sometimes  wrong    yes     yes
win32_0     OK     wrong      NO       wrong  sometimes  wrong    yes     yes
patch_1     OK      OK        YES      wrong    always   wrong    yes     yes
patch_2     OK      OK        YES       OK      always   wrong    yes     yes
patch_3     OK      OK        YES       OK       OK      wrong    yes     yes

error_without_sense appcrash

tebasuna51 commented 7 years ago

About the Output File Name:

Seems the procedure DelayCut::on_outputBrowseButton_clicked() work fine, but don't exist a procedure DelayCut::on_outputFileLineEdit_textChanged() than is the logic way to suply a output name than don't exist before.

Exist DelayCut::on_inputFileLineEdit_textChanged() than work, but is useless (better Browse or Drag&Drop)

One more try please:

delayac3_new3.zip

darealshinji commented 7 years ago

Thanks for your help. I really wish the original author didn't drop this project.

delaycut_testbuild.zip

tebasuna51 commented 7 years ago

Well, seems than CRC errors was solved now and can work now like command line without problems. (I edit my previous table to add patch_3) The rest are related, I think, with Qt GUI:

1) Don't accept edit the File Name Output. With Browse button always search for AC3 (*.ac3) files even if source is DTS or other.

2) Seems there are some async between data in capture fields and internal variables used to calculate Target data or errors. In the previous image is clear than Start cut (3200 ms) is less than File Length (20 sec.) That produce unexpected errors, and always I get a unexpected error after a time (some minutes unused) I get the APPCRASH.

If you make a new release at least is better than v1.4.3.7

darealshinji commented 7 years ago

Do you have the source code of version 1.3.0?

tebasuna51 commented 7 years ago

Yep: http://madshi.net/delaycut.rar

I compared delayac3.cpp from 1.3.0 with the patched version and is ok for me. There are many variables unused, maybe for that you comment some lines, but only can be commented at end (like with mpa), or replaced by readbits() without assing the value to a variable unused, but must follow the order until the last field in header needed.

In my last patch I uncomment some lines in other procedures (for that now work CRC calculations)

darealshinji commented 7 years ago

In my last patch I uncomment some lines in other procedures (for that now work CRC calculations)

Where is that patch?

tebasuna51 commented 7 years ago

The delayac3_new3 already sended. I think than delayac3.cpp is ok now.

darealshinji commented 7 years ago

The output filename can now be edited through the text input field. However, if input and output are the same, the input file will be replaced. I still need to fix that. delaycut.zip

tebasuna51 commented 7 years ago

Thanks.

darealshinji commented 7 years ago

Does it now work as expected?

tebasuna51 commented 7 years ago

Sorry to answer to late.

Yes, please do a new release.

darealshinji commented 7 years ago

New release was published.