Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minor issue: granulepos of the last packet of the cut segment (vcut.c) #25

Open
Alpt opened this issue Apr 16, 2020 · 3 comments
Open

Comments

@Alpt
Copy link

Alpt commented Apr 16, 2020

I believe the following patch should be applied in process_audio_packet:

                /* Set it! This 'truncates' the final packet, as needed. */
-               packet->granulepos = rel_cutpoint;
+               packet->granulepos = rel_cutpoint - vs->samples_cut;
                cut_on_eos = packet->e_o_s;

line 426 of vcut.c, commit 70d0d8e

@Alpt Alpt changed the title minor issue: granulepos of the last packet of the cut segment minor issue: granulepos of the last packet of the cut segment (vcut.c) Apr 16, 2020
@petterreinholdtsen
Copy link

petterreinholdtsen commented Apr 16, 2020 via email

@Alpt
Copy link
Author

Alpt commented Apr 16, 2020

In line 456, packet->granule_pos is calculated as follow:

        packet->granulepos = vs->granulepos
                - vs->initial_granpos - vs->samples_cut;

Instead, in line 426, it is rel_cutpoint, which is calculated as

vs->vi.rate * (s->next_segment->cuttime - s->prevstream_time)

(this if segment->cuttime >= 0)
where
s->prevstream_samples += vs->granulepos - vs->initial_granpos

What I am trying to say is that both in 426 and 456 (granulepos - initial_granpos) is considered, but in 456 also vs->samples_cut is considered.

The problem to be observed should be that the last granulepos of the cut segment is wrong if vs->samples_cut is greater than 0.

I am working on a fork of vcut and I was observing this. I solved it with the mentioned patch.
As soon as I have time, I'll try to provide a proof of concept.

Best regards.

@Alpt
Copy link
Author

Alpt commented Apr 17, 2020

Ok, I don't know if it happens in other cases, e.g. chained streams, but it surely happens if you perform more than one cut. To perform more than one cut, set segment->next to another valid vcut_segment. See master...Alpt:pof-ok

Here is the output of running the PoC


# the input file is 10 seconds long

$ soxi ~/Documents/tone-440.ogg  | grep -i duration
Duration       : 00:00:10.00 = 441000 samples = 750 CDDA sectors

# performing a cut each second

$ ./vcut/vcut ~/Documents/tone-440.ogg /tmp/tone-A /tmp/tone-B +1
Processing: Cutting at 1,000000 seconds
/tmp/tone-B-0000
/tmp/tone-B-0001
/tmp/tone-B-0002
/tmp/tone-B-0003
/tmp/tone-B-0004
/tmp/tone-B-0005
/tmp/tone-B-0006
/tmp/tone-B-0007
/tmp/tone-B-0008
/tmp/tone-B-0009
WARNING: found EOS before cutpoint

# each segment has a wrong declared duration. However, if you play it or open with audacity,
   you see 1 second of audio

andrea@convertibile:~/input/software/vorbis-tools-orig$ soxi /tmp/tone-* | grep -i Duration
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:02.00 = 88200 samples = 150 CDDA sectors
Duration       : 00:00:03.00 = 132300 samples = 225 CDDA sectors
Duration       : 00:00:04.00 = 176400 samples = 300 CDDA sectors
Duration       : 00:00:05.00 = 220500 samples = 375 CDDA sectors
Duration       : 00:00:06.00 = 264600 samples = 450 CDDA sectors
Duration       : 00:00:07.00 = 308700 samples = 525 CDDA sectors
Duration       : 00:00:08.00 = 352800 samples = 600 CDDA sectors
Duration       : 00:00:09.00 = 396900 samples = 675 CDDA sectors
Duration       : 00:00:10.00 = 441000 samples = 750 CDDA sectors
Total Duration of 10 files: 00:00:55.00

# indeed, concatenating the segments we get 10 seconds of audio
andrea@convertibile:~/input/software/vorbis-tools-orig$ sox /tmp/tone-* /tmp/concatenated.ogg
andrea@convertibile:~/input/software/vorbis-tools-orig$ soxi /tmp/concatenated.ogg | grep -i Duration
Duration       : 00:00:10.06 = 443664 samples = 754.531 CDDA sectors

Moreover, this issue creates gaps when concatenating the segments. By applying, the patch mentioned above in this issue, all these problems are solved.

$ soxi /tmp/tone-* | grep -i Duration
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Duration       : 00:00:01.00 = 44100 samples = 75 CDDA sectors
Total Duration of 10 files: 00:00:10.00

$ sox /tmp/tone-* /tmp/concatenated.ogg

$ soxi /tmp/concatenated.ogg  | grep -i duration
Duration       : 00:00:10.00 = 441000 samples = 750 CDDA sectors

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants